<html>
<head>
<base href="https://llvm.org/bugs/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:james.molloy@arm.com" title="James Molloy <james.molloy@arm.com>"> <span class="fn">James Molloy</span></a>
</span> changed
<a class="bz_bug_link
bz_status_RESOLVED bz_closed"
title="RESOLVED INVALID - Regression(288024): clang miscompiles chromium's safe_math code on arm"
href="https://llvm.org/bugs/show_bug.cgi?id=31218">bug 31218</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Status</td>
<td>ASSIGNED
</td>
<td>RESOLVED
</td>
</tr>
<tr>
<td style="text-align:right;">Resolution</td>
<td>---
</td>
<td>INVALID
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_RESOLVED bz_closed"
title="RESOLVED INVALID - Regression(288024): clang miscompiles chromium's safe_math code on arm"
href="https://llvm.org/bugs/show_bug.cgi?id=31218#c5">Comment # 5</a>
on <a class="bz_bug_link
bz_status_RESOLVED bz_closed"
title="RESOLVED INVALID - Regression(288024): clang miscompiles chromium's safe_math code on arm"
href="https://llvm.org/bugs/show_bug.cgi?id=31218">bug 31218</a>
from <span class="vcard"><a class="email" href="mailto:james.molloy@arm.com" title="James Molloy <james.molloy@arm.com>"> <span class="fn">James Molloy</span></a>
</span></b>
<pre>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...</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>