UB optimization leads to really bad code gen

[bwendling]

Linux has some code that may result in dividing by zero or shifting by negative numbers. For instance, in one of the ARMGPU drivers, a "divide by zero" results in a fall through into the following function.

drivers/gpu/drm/amd/amdgpu/../display/dc/basics/fixpt31_32.o:
     warning: objtool: dc_fixpt_recip() falls through to next function dc_fixpt_sinc()

drivers/gpu/drm/amd/amdgpu/../display/dc/sspl/spl_fixpt31_32.o:
     warning: objtool: spl_fixpt_recip() falls through to next function spl_fixpt_sinc()

It is undefined behavior, but in these cases it will silently generate incorrect code. It's far better to generate the bad code so that it signals/traps/whatever.

[pinskia]

Related to #137693 .

[thesamesam]

https://lore.kernel.org/llvm/20250426200513.GA427956@ax162/

[dzaima]

(random unsolicited opinions / notes on things in the mailing list:)

Calling this generating "incorrect code" isn't doing you any favors. It's a situation that you, Linus, others (even me! I think it'd absolutely make sense to insert a ud2 or equivalent in paths that the compiler has explicitly made but decided shall not happen, and pinskia's referenced issue includes a case where this actually results in incorrect code) think could be handled better, but that by no means makes it incorrect.

llvm shouldn't ever be explicitly trying to "make" worse results; presumably what's happening for the function in question is that some probably-unrelated optimization (perhaps unnecessarily) splits the function on divisor!=0 and divisor==0 and then the latter gets "simplified".
(edit: apparently up the thread Linus did notice that - having two warnings in the path is certainly a good reason to switch to checking the divisor==0 just once, the divide afterwards is just a collateral; and, as such, in a build with the warnings disabled, I'd think the falling-off-the-function would go away too, given no reason for specializing divisor==0; so you always either get the warning(s), or non-"really bad" behavior)

From llvm/clang's perspective, a x/y assumes the user guarantees y!=0 as strongly as if they did if (y==0) __builtin_unreachable();, or in such a case made a noreturn function return, or any number of other UB cases. As such, there could easily be arbitrarily-many other things that'd trigger the objtool warning, this isn't at all specific to division.

I do think it would be very good to have a bunch more flags for picking saner behaviors for a bunch of classes of operations (I even have code that "relies" on UB for cases where it's impossible to do the desired thing in a UB-less way without adding unreasonable amounts of overhead, which I'd quite like to avoid, but oh well).

But it's very hard to make meaningful guarantees at all without potentially needing to actually add otherwise-unnecessary code - e.g. on RISC-V this simple code, even on -O0, where clang doesn't do anything unexpected, results in rather unexpected behavior - the uint8_t index into the 256-element buf indexes out of bounds! This happens because RISC-V on division-by-zero returns 0xffffffffffffffff, but, as for all C-defined cases the result is in 0..255, it's perfectly reasonable to not emit extraneous zero-extension, which I think should be fully in-line with what UB in the C standard was meant for under any interpretation (i.e. allowing mapping an operation directly to an instruction in the target architecture without fixups). So wanting the compiler to not "abuse" UB isn't sufficient - you want it to actually make some guarantees that actually mean something.