Friday, September 27, 2024

Ptrdiff_t is sorta signed.

We are using qsort/bsearch on pointers.
The code looked like this:

typedef struct { int64_t a[0xb0 / 8]; } Data;

int compare(void** p1, void** p2)
{
  ptrdiff_t delta = (*(Data**)p1 - *(Data**)p2);
  return (delta < 0) ? -1 : ((delta > 0) ? 1 : 0);
}

This is somewhat idiomatic and sometimes works, but often wrong or undefined.
In the past I'd seen this go wrong due to subtracting a larger unsigned from a smaller unsigned.
You should always write more like:

  return (*p1 < *p2) ? -1 : ((*p1 > *p2) ? 1 : 0);

But I hadn't seen this go wrong with pointers. Well, I hadn't seen with pointers much.
Here is a program using the same behaviors:
#include <stdlib.h>
#define assert(x) ((void)((x) ? 0 : __debugbreak()))
typedef struct { __int64 a[0xb0 / 8]; } Data;
int main()
{
    while (1)
    {
        Data* d1 = (Data*)malloc(sizeof(Data));
        Data* d2 = (Data*)malloc(sizeof(Data));
        assert((d1 - d2) < 0 == (d1 < d2));
        free(d1);
        free(d2);
    }
}

The problem in all these cases, is that comparison and subtraction of pointers is only defined if the pointers are to elements within the same array. So what? What difference might that make? It means the difference of the pointers must be a multiple of the size of the type they point to. It means that unsigned and signed division implied by the subtraction will return the same result. They only vary in rounding. i.e. When the answer is not a whole number. But the difference is not an entire multiple than unsigned and signed division will round to a different value.

And so why did this come up? The code was failing. What changed to make it fail?
I had changed from the build systems default WholeProgramOptimization (/LTCG), which is incremental (link /LTCG:incremental) to regular /LTCG.

/LTCG:incremental divides signed in this case, and the code depended on it.
/LTCG divides unsigned, and that broke the code.

I changed the code to compare with <. I suspect to fully avoid undefined behavior, we should cast to char* or uintptr_t or intptr_t (should pointers be considered signed or not?)