On Fri, 2 Jul 2021 at 22:36, Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote: > > On Thu, Jul 1, 2021 at 8:38 PM Christian Göttsche > <cgzones@xxxxxxxxxxxxxx> wrote: > > > > Unsigned integer overflow is well-defined and not undefined behavior. > > But it is still useful to enable undefined behavior sanitizer checks on > > unsigned arithmetic to detect possible issues on counters or variables > > with similar purpose. > > > > Use a spaceship operator like comparison instead of subtraction. > > > > Modern compilers will generate a single comparison instruction instead > > of actually perform the subtraction. > > > > policydb.c:851:24: runtime error: unsigned integer overflow: 801 - 929 cannot be represented in type 'unsigned int' > > > > This is similar to 1537ea84. > > While I agree with the change, I still see subtractions in gcc 11.1 > and clang 12. What do you call "modern compilers"? > > More precisely, I copied the function rangetr_cmp in Godbolt and > compiled it with "x86-64 gcc 11.1" and -O2, > https://godbolt.org/z/5c4jG7eeh . The generated assembly code > contains: > > mov eax, DWORD PTR [rsi] > cmp DWORD PTR [rdi], eax > seta al > movzx eax, al > sbb eax, 0 > test eax, eax > jne .L1 > > This really computes a subtraction, with instruction SBB. For people > not familiar with x86_64 assembly language: > > * the first two instructions compare two values, > * SETA sets the 8-bit register AL to 1 if the first value was above > the other one, > * MOVZX ensures that the 32-bit register EAX contains 0 or 1, > * SBB computes the subtraction between EAX and the carry flag, which > is one only if the first value was below the other one. > * TEST checks whether the result of the subtraction is zero. > Since I also checked the assembly results with Godbolt, it seems I had misinterpreted the results regarding the subtract instruction. > For information, the previous code generated the following instructions: > > mov eax, DWORD PTR [rdi] > sub eax, DWORD PTR [rsi] > jne .L1 > > So the previous code generated simpler assembler instructions, but > which could trigger an undefined behavior in C. > It is not undefined behaviour, unsigned integer overflow is well defined to be wrapped. And there are legitimate use cases, like for hashing or pseudo number generation. But outside of those unsigned integer over- and underflow can point to logic errors (when reducing a counter below zero) or missed overflow checks on user input. > TL;DR Anyway, as this change is not about performance but about > undefined behaviour in C, it looks good to me. Nevertheless the > sentence "Modern compilers will generate a single comparison > instruction instead of actually perform the subtraction." seems to be > wrong. Could you rephrase it (for example by giving examples of such > compilers), or remove it? > I'll send a new version with an updated description and also using a comparison macro, to minimize the chance for typos like in the first version.
