Showing posts with label bug. Show all posts
Showing posts with label bug. Show all posts

Monday, June 16, 2025

C/C++ header mismatch bug

I encountered a problem where a field in a global struct (myStruct) held a valid value before entering a function foo, but turned into garbage after entering it. When I consulted AI tools, they suggested that foo might be allocating very large local arrays, causing a stack overflow that could corrupt the global structure. Another possibility was an out-of-bounds write elsewhere in the code.

After a week of debugging and trying various solutions—such as increasing the thread's stack size—I discovered the root cause: The function foo was defined in a C library with multiple versions. Each version resided in a different folder but had the same file names. Which folder was used depended on a #define. I was including the header from one version of the library, but linking against the implementation from another. If the struct definitions had matched, this wouldn’t have caused an issue, but they differed—evident from the differing sizeof(myStruct). As a result, myStruct was interpreted using the wrong layout, leading to corrupted values from an incorrect memory region.

Sunday, June 15, 2025

C++ pointer bug

This C++ code has a significant bug that will cause undefined behavior:
#include <iostream>
class A {
  public:
    int val;
};
void reset(A *p_a) {
  if (p_a != NULL) {
      delete p_a;
  }
  p_a = new A();
}
int main() {
  A *p_a = new A();
  p_a->val = 5;
  std::cout << "Before reset, p_a->val:" << p_a->val << "\n";
  reset(p_a);
  std::cout << "After reset, p_a->val:" << p_a->val << "\n";
  return 0;
}

The reset function receives a copy of the pointer p_a, not a reference to it. When you modify p_a inside the function (with p_a = new A()), you're only changing the local copy - the original pointer in main() remains unchanged. What actually happens:

  1. p_a in main() points to an A object with val = 5 
  2. reset() receives a copy of this pointer 
  3. reset() deletes the original object (memory is freed) 
  4. reset() creates a new object, but assigns it only to the local copy 
  5. The original p_a in main() still points to the deleted memory 
  6. Accessing p_a->val after reset() is undefined behavior (accessing freed memory) 
The Fix: Pass the pointer by reference using a pointer-to-pointer or reference-to-pointer:
//Reference to pointer
void reset(A *&p_a) {
  if (p_a != nullptr) {
    delete p_a;
  }
  p_a = new A();
  // Call with: reset(p_a);

An even better fix is to use smart pointers, which removes the necessity for the reset function:

auto p_a = std::make_unique<A>();

You can detect such problems by enabling AddressSanitizer (ASAN) in Visual Studio:

  1. Right-click your project → Properties
  2. Go to Configuration Properties → C/C++ → General
  3. Set Enable Address Sanitizer to Yes (/fsanitize=address)
  4. Go to Configuration Properties → C/C++ → Optimization
  5. Set Optimization to Disabled (/Od) for better debugging
  6. Set Whole Program Optimization to No
  7. Go to Configuration Properties → C/C++ → Debug Information Format
  8. Set to Program Database (/Zi) or Program Database for Edit & Continue (/ZI)
In Eclipse CDT:

  1. Open your C/C++ project in Eclipse CDT
  2. Right-click project → Properties
  3. Navigate to C/C++ Build → Settings
  4. Under Tool Settings:
    1. GCC C++ Compiler → Miscellaneous
    2. GCC C Compiler → Miscellaneous
    3. Add to "Other flags": -fsanitize=address -g -O1
  5. Project Properties → C/C++ Build → Settings
  6. GCC C++ Linker → Miscellaneous
  7. Add to "Other objects": -fsanitize=address

Wednesday, February 12, 2025

Longitude convention and octal values

Recently, I encountered a bug in code that handled longitude values, where 33 degrees longitude is typically written as 033. Upon investigation, I discovered that Java was calculating 033 + 1 as 28 instead of 34. This happened because, when we write longitude values like "033" in geographic notation, we mean decimal 33 degrees. However, if we write it directly in Java, C++, or similar languages, it is interpreted as octal 33 (which equals decimal 27) due to the leading zero!

This is a classic example of why it's important to be careful when working with domain-specific number formats - what makes sense in geographic notation can have unexpected behavior when directly translated to programming language syntax!

Thursday, December 28, 2023

The unbearable lightness of C

We have a Simulink project from which I generate C code to use in a Visual Studio C++ project. The Simulink project works fine, I can build the C code without any errors, but when I ran the C executable, I got an access violation error due to trying to write to address 0x0. The C project was working fine for previous versions. 

I initially identified the revision where this error first appeared. I reviewed the changed code and couldn't find anything wrong.

After a week of debugging I found out that it was due to an off-by-one error; An array was defined with size 47 using Simulink function ssSetNumDiscState(S, 47) in mdlInitializeSizes(...), but later in function mdlInitializeConditions(...), a for loop with upper bound of 48 was executed which resulted in writing to the memory adjacent to the allocated section for that array. 
static void mdlInitializeConditions(SimStruct *S) {
    real_T *states = ssGetRealDiscStates(S);
    for (int i=0; i < 48; i++) {
    	*(states + i) = 0;
    }
}
It has nothing to do with the latest change in the sense that it was not related to the logic of the change. Instead, that change altered the memory mapping of the build, putting another array (TUBufferPtr) after the first and the overflow caused the value at TUBufferPtr[0] to become 0x0 (NULL). When the program tried to write to the address represented by TUBufferPtr[0], it naturally caused an access violation because writing to address 0x0 is not allowed.
When I looked at the repository history, I saw that this error was introduced 3 years ago and for all these years, it did not become visible! The access violation only occurred when other unrelated code updates caused the compiler to arrange memory slightly differently.

This is also one of the reasons why sometimes C programs behave differently between debug and release builds or on different versions (service packs) of the same operating system. That discrepancy is an indication of an error hiding somewhere in your program. Another way such an error can become visible is when you have it in your C++ DLL that you call from your Java program. One day you update your JDK and your program crashes because DLL and JVM share the same memory space. Naturally, your first inclination is to blame the JDK update but in reality there is a buffer overflow in your DLL code.

You can never say for sure that modifying a code in module A won't have an effect on an unrelated module B. As long as these modules are in the same process, i.e. use the same memory, the compiler might put them side by side and an error in A can overflow to B. Functionally distant modules can become "close relatives" in memory address space.

In a way, I was fortunate that the overwrite contained zero values. If it had used some value that was a valid address for the application to write to, it would cause seemingly random behavior and would have been a lot more fun (!)

Note that a typical static code analyzer would not able to catch this problem because we are defining the data structure with Simulink specific ssSetNumDiscState and getting it with ssGetRealDiscStates functions.

I solved the problem by adding #define NUMBER_OF_DISCRETE_STATES (47) and using that define in both ssSetNumDiscState() and mdlInitializeConditions(). This case study also serves as a cautionary tale illustrating why you should use defined constants rather than magic numbers.

In my nightly automated tests I was only checking if exe was generated which only proved that it compiled. I added running the exe and checking if it finished successfully because an access violation can only occur at runtime.