[llvm-bugs] [Bug 31218] Regression(288024): clang miscompiles chromium's safe_math code on arm

[via llvm-bugs]

https://llvm.org/bugs/show_bug.cgi?id=31218

James Molloy <james.molloy at arm.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |INVALID

--- Comment #5 from James Molloy <james.molloy at arm.com> ---
Hi Nico,

Sorry it took me a day to get to this.

After a heavy debugging session, I believe this stems from undefined behaviour
in the source code.

>From the ground up...

1) a CheckedNumeric<uint8_t> is constructed via a cast from a
CheckedNumeric<float> and the .IsValid() function called. This function is
expected to return false as the conversion was out of range (overflow).

2) IsValid() returns a function of __isfinitef() and the ::is_valid member. The
Cast function ends up simply calling a constructor:

   9184  constexpr CheckedNumeric(const CheckedNumeric<Src>& rhs)
   9185      : state_(rhs.state_.value(), rhs.IsValid()) {}

3) CheckedNumeric<uint8_t>'s struct layout is simply {uint8_t value, uint8_t
is_valid_} (via CheckedNumericState). Therefore, it is represented as an i16
with the high bits being is_valid_.

4) The constructor is implemented similarly to:

  i16 constructor(float value) {
  i8 lo = (uint8_t)value;
  i8 hi = 0;
  if (!__is_finite(value)) {
    hi = 1;
  } else {
    hi = IsValueInRangeForNumericType<uint8_t>((float)value) == 0; // This is
the test that is supposed to set _is_valid = false when value is out of range
for the dst type
  }
  return (i16)lo | ((i16)hi << 8);
  }

5) The cast from float to uint8_t in the first line is undefined behaviour if
value is not representable in uint8_t. As the underlying storage is a register
(32 bits), this ends up setting some high bits, which makes the _is_valid
member of the struct nonzero.

6) LLVM's constant folder doesn't model it in this way, modelling it as a
truncation. This is why the expression succeeds or fails depending on the
inlining heuristic.

There should be an explicit bitmask to avoid the undefined behaviour.

Phew! That was actually quite a fun morning...

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20161202/4e0d55f5/attachment.html>