[cfe-dev] -fcatch-undefined-behavior testing
daniel at zuster.org
Wed Dec 16 12:08:49 PST 2009
2009/12/16 Török Edwin <edwintorok at gmail.com>:
> On 2009-12-16 20:36, Mike Stump wrote:
>> On Dec 16, 2009, at 3:47 AM, Török Edwin wrote:
>>> This is a very interesting feature!
>>> However I found no way to skip this error, to look for more errors.
>> Yeah, limitation of the current codegen. Trivially, one could imagine logging interface codegen. It could include column and line numbers, filenames and what rule was violated and how (the shift amount was 128 bits). I think filing a PR for a logging system would make a nice addition. Great for large codebases when build test cycle times are in the 1-3 day range.
> Ok. As a short term solution could it generate an int3 though instead of
> an illegal instruction?
>>> Also would it be possible to warn about shift exceeding width only if they can change the outcome?
>> Well, the notion that some undefined behavior is defined and perfectly ok, is an odd notion. Rather, think of it this way, the runtime check is trying to tell you that the `portable' version of ROR doesn't work on ppc and that you need to fix your code to be portable. After it is fixed, there will be no runtime failure. In fact, one of the purposes of the check was to find exactly this type of non-portable code.
> For the case where the shift amount is constant, I agree that it is
> undefined, for (uint32_t x=...; x << 32) the compiler can say that x is
> 42 (or some other random value).
> However when the shift amount is not constant, the compiler has no
> choice but to implement the shift with CPU instructions (it may or may
> not recognize the ror pattern, currently it doesn't).
> In that case the undefined behavior for the shift becomes an
> implementation defined behavior of the CPU.
This isn't the correct understanding of undefined behavior. The
compiler is perfectly free to optimize all subsequent code in your
program as if the shift value is in the range [0,32), since if it
wasn't the behavior would have be undefined. This has larger
implications than just the result of the shift.
OTOH I agree this is mostly a pedantic point, but overall I agree with
Mike -- the flag should catch undefined behavior. If the programmer
wants to say "oh, yeah, but no compiler will *ever* make me regret
that" then one reasonable way out would be to disable it with a pragma
> For the CLI_ROR case, the only scenario where the undefined behavior
> might occur (and I don't consider it a bug) is if shift amount == bitwidth,
> in which case it depends on the CPU what happens
> (X86 appears to shift by 0, PPC shifts all bits out and result is 0). In
> either case the end result of the CLI_ROR macro is the same, since a|a
> == a|0.
> For higher shift amounts the result may not be the same, but I think for
> shift amount == bitwidth the result is still correct.
> Indeed when the rotate amount passes to CLI_ROR is greater than
> bitwidth, that should be fixed, see here:
>>> The shift of 32 could be worked around by checking whether b is 0, but that adds another branch.
>> A branch on a constant is relatively free. A smart optimizer, having a ror instruction on the target, would always eliminate the branch as well.
>> Consider what happens when a is constant, and the emulation for shift at compile time doesn't match the runtime behavior of shift, the wrong value is computed. Might not happen on a native compiler, but odds of this happening under cross compilation go up.
> Yeah the branch for the constant case would be fine, however it would
> mean a penalty for the non-constant shift case.
> Best regards,
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
More information about the cfe-dev