<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Dec 7, 2015 at 7:20 PM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">On Mon, Dec 7, 2015 at 9:51 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Mon, Dec 7, 2015 at 5:26 PM, Reid Kleckner via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">It wasn't Chrome, it was some internal dependency on torch-cephes. I submitted a patch for it upstream:<div><a href="https://github.com/deepmind/torch-cephes/commit/9c4a97c90dc200ecbecb883e7230fe3c847954df" target="_blank">https://github.com/deepmind/torch-cephes/commit/9c4a97c90dc200ecbecb883e7230fe3c847954df</a><br></div><div>It's not a pretty, though.<br></div><div><br></div><div>I know LLVM IR rules are not C++ rules, but LLVM generally believes in NaN. It will speculate things like fdiv above branches, leading to traps when people configure the FPU appropriately and then conditionally divide by zero. It would be more consistent for Clang to follow the LLVM direction here. The fact that C++ says that FP div-by-zero is UB might just be a bug in the standard, or a grey area that couldn't be filled in.</div></div></blockquote><div><br></div></span><div>Right, I'm not suggesting we treat floating-point division by zero as "true UB". But the fact we have a sanitizer for it means that our behavior is not, and cannot be, "just give a NaN or Inf". It's a choice of either "give me a NaN or Inf" or "give me a sanitizer diagnostic, followed by a NaN or Inf". And that means it's not constant in the general case, because evaluating it has runtime side-effects.</div></div></div></div></blockquote><div><br></div></span><div>The mere existence of a sanitizer shouldn't be what determines our behavior in the general case.  We also have a sanitizer for unsigned overflow...</div></div></div></div></blockquote><div><br></div><div>I agree, but the difference is that the unsigned overflow sanitizer is not a conforming mode for our implementation, whereas UBSan is supposed to be one.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Prior to this patch, Clang would determine that such divisions had no side-effects, and speculate them *and the corresponding sanitizer checks*, resulting in sanitizer false positives on correct code.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><div><div><div class="gmail_extra"><div class="gmail_quote">On Mon, Dec 7, 2015 at 2:37 PM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Dec 7, 2015 at 5:25 PM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>On Mon, Dec 7, 2015 at 2:14 PM, David Majnemer <<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>> wrote:<br>
><br>
><br>
> On Mon, Dec 7, 2015 at 4:32 PM, Richard Smith via cfe-commits<br>
> <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>> On Mon, Dec 7, 2015 at 7:25 AM, Joerg Sonnenberger via cfe-commits<br>
>> <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
>>><br>
>>> On Thu, Dec 03, 2015 at 01:36:22AM -0000, Richard Smith via cfe-commits<br>
>>> wrote:<br>
>>> > Author: rsmith<br>
>>> > Date: Wed Dec  2 19:36:22 2015<br>
>>> > New Revision: 254574<br>
>>> ><br>
>>> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=254574&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=254574&view=rev</a><br>
>>> > Log:<br>
>>> > PR17381: Treat undefined behavior during expression evaluation as an<br>
>>> > unmodeled<br>
>>> > side-effect, so that we don't allow speculative evaluation of such<br>
>>> > expressions<br>
>>> > during code generation.<br>
>>> ><br>
>>> > This caused a diagnostic quality regression, so fix constant expression<br>
>>> > diagnostics to prefer either the first "can't be constant folded"<br>
>>> > diagnostic or<br>
>>> > the first "not a constant expression" diagnostic depending on the kind<br>
>>> > of<br>
>>> > evaluation we're doing. This was always the intent, but didn't quite<br>
>>> > work<br>
>>> > correctly before.<br>
>>> ><br>
>>> > This results in certain initializers that used to be constant<br>
>>> > initializers to<br>
>>> > no longer be; in particular, things like:<br>
>>> ><br>
>>> >   float f = 1e100;<br>
>>> ><br>
>>> > are no longer accepted in C. This seems appropriate, as such constructs<br>
>>> > would<br>
>>> > lead to code being executed if sanitizers are enabled.<br>
>>><br>
>>> This leads to some pretty annoying regressions as it now seems to be<br>
>>> impossible to use NaN or infinites as constant initializers. Expressions<br>
>>> like 0.0 / 0.0, 1.0 / 0.0 and -1.0 / 0.0 are perfectly well defined<br>
>>> under normal IEEE rules, so they shouldn't be rejected.<br>
>><br>
>><br>
>> Well, we have a problem. The evaluation semantics of these expressions<br>
>> requires code to execute in some build modes (in particular, with sanitizers<br>
>> enabled), and thus has a side-effect.<br>
>><br>
>> I'm inclined to relax the restriction added in this change for the<br>
>> specific case of global variables in C, since (as you say) there is a fair<br>
>> amount of code using divide-by-zero as a "portable" way of generating an inf<br>
>> or nan.<br>
>><br>
>>> Worse, it seems<br>
>>> even using __builtin_nan() for example doesn't work.<br>
>><br>
>><br>
>> __builtin_nan() works fine for me, can you provide a testcase?<br>
>><br>
>>> I'm not even sure about the example given in the commit message, how<br>
>>> exactly is that undefined behavior?<br>
>><br>
>><br>
>> C11 <a href="http://6.3.1.5/1" rel="noreferrer" target="_blank">6.3.1.5/1</a>: "If the value being converted is outside the range of<br>
>> values that can be represented, the behavior is undefined."<br>
><br>
><br>
> I don't think we want to make the UB here true UB.  It would mean that code<br>
> which expected to get NaN might get undef, even outside of constant<br>
> expression evaluation.  The implementation defined behavior of providing NaN<br>
> seems more friendly...  IIRC, this broke Chrome recently because folks were<br>
> doing this in C++.  Hans, do you remember the details?<br>
<br>
</div></div>Hmm, it doesn't ring a bell, but my memory sometimes fails me. Didn't<br>
you and Reid look at something like this the other day? (But maybe<br>
that was in internal code?)<br>
</blockquote></div><br></div></div></div><div class="gmail_extra">Ah, I think you are right... Reid, do you happen to recall?</div></div>
</blockquote></div><br></div>
</div></div><br></div></div><span>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
<br></span></blockquote></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>