Undefined behavior in setchkundefinedflags and setchk2undefinedflags

[dirkwhoffmann]

Hi Toni,

I am referring to functions setchkundefinedflags and setchk2undefinedflags in newcpu_common.cpp.

Inside both functions, multiple subtractions are performed such as this one:

if (val > lower && upper - val >= 0)

When running real test cases (e.g., the ones created by cputester), integer underflows may occur which is undefined behavior in C. This is not a problem for MSVC (I think), but it is in clang.

I’ve noticed the issue after porting this code to vAmiga and running cputester. The result was:

• Tests CHK and CHK2 pass when compiled with clang and -O0
• Tests CHK and CHK2 fail when compiled with clang and -O3

Thing is that clang sometimes uses aggressive optimizations that exploit undefined behavior.

In my code, I’ve fixed the issue by replacing the line above by

if (val > lower && diff(upper,val) >= 0)

where diff is defined as

          auto diff = [&](i32 arg1, i32 arg2) {
               return i32((i64)arg1 - (i64)arg2);
           };

As stated above, I think it’s not an issue with MSVC, but if you wish to make your code more robust, you might want to change the troublesome lines by something like:

if (… && (uae_s32)((uae_s64)upper - (uae_s64)val) >= 0)

[tonioni]

Looks a bit too ugly, hopefully there is some better way to fix this cleanly.