RE: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-09 Thread Robinson, Paul via cfe-commits 
| And at runtime, on some targets, we use this:
|
|  
https://llvm.org/svn/llvm-project/compiler-rt/trunk/lib/builtins/floatuntisf.c
|
| ... which gives a NaN in this case.

I copied that function into a test program on Ubuntu, built with gcc, and it 
gives me +Infinity (0x7f80) not NaN (0x7fc0).
--paulr

From: meta...@gmail.com [mailto:meta...@gmail.com] On Behalf Of Richard Smith
Sent: Tuesday, December 08, 2015 11:42 AM
To: Robinson, Paul
Cc: Joerg Sonnenberger; cfe-commits (cfe-commits@lists.llvm.org)
Subject: Re: r254574 - PR17381: Treat undefined behavior during expression 
evaluation as an unmodeled

On Tue, Dec 8, 2015 at 11:18 AM, Richard Smith 
<rich...@metafoo.co.uk<mailto:rich...@metafoo.co.uk>> wrote:
On Tue, Dec 8, 2015 at 10:59 AM, Robinson, Paul 
<paul_robin...@playstation.sony.com<mailto:paul_robin...@playstation.sony.com>> 
wrote:
Okay, I'll bite:  so what *does* UINT128_MAX actually convert to?

$ echo 'unsigned __int128 max = -1; float f = max;' | ~/clang-8/build/bin/clang 
-x c++ - -emit-llvm -S -o - -O3 | grep @f
@f = global float undef, align 4

And at runtime, on some targets, we use this:

  https://llvm.org/svn/llvm-project/compiler-rt/trunk/lib/builtins/floatuntisf.c

... which gives a NaN in this case.

From: cfe-commits 
[mailto:cfe-commits-boun...@lists.llvm.org<mailto:cfe-commits-boun...@lists.llvm.org>]
 On Behalf Of Richard Smith via cfe-commits
Sent: Tuesday, December 08, 2015 10:52 AM
To: Joerg Sonnenberger; cfe-commits
Subject: Re: r254574 - PR17381: Treat undefined behavior during expression 
evaluation as an unmodeled

On Tue, Dec 8, 2015 at 2:13 AM, Joerg Sonnenberger via cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
On Mon, Dec 07, 2015 at 01:32:14PM -0800, Richard Smith via cfe-commits wrote:
> C11 6.3.1.5/1<http://6.3.1.5/1>: "If the value being converted is outside the 
> range of values
> that can be represented, the behavior is undefined."

The value of 1e100 can be represented as +inf, even if not precisely.

Only if +inf is in the range of representable values, which, as already noted, 
is problematic.

This is a bit different from non-IEEE math like VAX, that doesn't have
infinities.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-09 Thread Richard Smith via cfe-commits 
On Wed, Dec 9, 2015 at 10:17 AM, Robinson, Paul via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> | And at runtime, on some targets, we use this:
>
> |
>
> |
> https://llvm.org/svn/llvm-project/compiler-rt/trunk/lib/builtins/floatuntisf.c
>
> |
>
> | ... which gives a NaN in this case.
>
>
>
> I copied that function into a test program on Ubuntu, built with gcc, and
> it gives me +Infinity (0x7f80) not NaN (0x7fc0).
>

Oh right, sorry, off-by-one error when evaluating that by hand; I got
0x7fff (which is also a NaN, 0x7fc0 is not the only NaN).

So, I think the question is, do we want to update LLVM to define the value
of an out-of-range uitofp (and "fix" any targets that don't give +/- Inf
for these conversions)?
http://llvm.org/docs/LangRef.html#uitofp-to-instruction is clear that you
get an undefined result for overflow currently.

In (AFAICS) all supported targets, for integer types supported by clang,
there are only two ways to hit the overflow case:
 1) uint128 -> float
 2) uint64 or larger -> half

Case (1) goes through uitofp (which explicitly says the result is undefined
at the moment), case (2) goes via @llvm.convert.to.fp16 (which says nothing
about what happens in this case, but presumably it is defined). These are
both phenomenally rare conversions, so adding (potential) extra cost to
them to make them handle the out-of-range case correctly doesn't seem
unreasonable.


> --paulr
>
>
>
> *From:* meta...@gmail.com [mailto:meta...@gmail.com] *On Behalf Of *Richard
> Smith
> *Sent:* Tuesday, December 08, 2015 11:42 AM
> *To:* Robinson, Paul
> *Cc:* Joerg Sonnenberger; cfe-commits (cfe-commits@lists.llvm.org)
>
> *Subject:* Re: r254574 - PR17381: Treat undefined behavior during
> expression evaluation as an unmodeled
>
>
>
> On Tue, Dec 8, 2015 at 11:18 AM, Richard Smith <rich...@metafoo.co.uk>
> wrote:
>
> On Tue, Dec 8, 2015 at 10:59 AM, Robinson, Paul <
> paul_robin...@playstation.sony.com> wrote:
>
> Okay, I'll bite:  so what *does* UINT128_MAX actually convert to?
>
>
>
> $ echo 'unsigned __int128 max = -1; float f = max;' |
> ~/clang-8/build/bin/clang -x c++ - -emit-llvm -S -o - -O3 | grep @f
>
> @f = global float undef, align 4
>
>
>
> And at runtime, on some targets, we use this:
>
>
>
>
> https://llvm.org/svn/llvm-project/compiler-rt/trunk/lib/builtins/floatuntisf.c
>
>
>
> ... which gives a NaN in this case.
>
>
>
> *From:* cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] *On
> Behalf Of *Richard Smith via cfe-commits
> *Sent:* Tuesday, December 08, 2015 10:52 AM
> *To:* Joerg Sonnenberger; cfe-commits
> *Subject:* Re: r254574 - PR17381: Treat undefined behavior during
> expression evaluation as an unmodeled
>
>
>
> On Tue, Dec 8, 2015 at 2:13 AM, Joerg Sonnenberger via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> On Mon, Dec 07, 2015 at 01:32:14PM -0800, Richard Smith via cfe-commits
> wrote:
> > C11 6.3.1.5/1: "If the value being converted is outside the range of
> values
> > that can be represented, the behavior is undefined."
>
> The value of 1e100 can be represented as +inf, even if not precisely.
>
>
>
> Only if +inf is in the range of representable values, which, as already
> noted, is problematic.
>
>
>
> This is a bit different from non-IEEE math like VAX, that doesn't have
> infinities.
>
>
> Joerg
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
>
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-09 Thread Robinson, Paul via cfe-commits 
| Oh right, sorry, off-by-one error when evaluating that by hand; I got 
0x7fff (which is also a NaN, 0x7fc0 is not the only NaN).
No worries.

| http://llvm.org/docs/LangRef.html#uitofp-to-instruction is clear that you get 
an undefined result for overflow currently.
Other parts of the LangRef are comfortable talking about infinities, e.g. 
there's a way to write them as constants, and IEEE float is pervasive in this 
era, so it would seem consistent for uitofp to return +infinity for the 
overflow case.  I'm not the one to propose it, though. ☺
--paulr

From: meta...@gmail.com [mailto:meta...@gmail.com] On Behalf Of Richard Smith
Sent: Wednesday, December 09, 2015 12:43 PM
To: Robinson, Paul
Cc: Joerg Sonnenberger; cfe-commits (cfe-commits@lists.llvm.org)
Subject: Re: r254574 - PR17381: Treat undefined behavior during expression 
evaluation as an unmodeled

On Wed, Dec 9, 2015 at 10:17 AM, Robinson, Paul via cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
| And at runtime, on some targets, we use this:
|
|  
https://llvm.org/svn/llvm-project/compiler-rt/trunk/lib/builtins/floatuntisf.c
|
| ... which gives a NaN in this case.

I copied that function into a test program on Ubuntu, built with gcc, and it 
gives me +Infinity (0x7f80) not NaN (0x7fc0).

Oh right, sorry, off-by-one error when evaluating that by hand; I got 
0x7fff (which is also a NaN, 0x7fc0 is not the only NaN).

So, I think the question is, do we want to update LLVM to define the value of 
an out-of-range uitofp (and "fix" any targets that don't give +/- Inf for these 
conversions)? http://llvm.org/docs/LangRef.html#uitofp-to-instruction is clear 
that you get an undefined result for overflow currently.

In (AFAICS) all supported targets, for integer types supported by clang, there 
are only two ways to hit the overflow case:
 1) uint128 -> float
 2) uint64 or larger -> half

Case (1) goes through uitofp (which explicitly says the result is undefined at 
the moment), case (2) goes via @llvm.convert.to.fp16 (which says nothing about 
what happens in this case, but presumably it is defined). These are both 
phenomenally rare conversions, so adding (potential) extra cost to them to make 
them handle the out-of-range case correctly doesn't seem unreasonable.

--paulr

From: meta...@gmail.com<mailto:meta...@gmail.com> 
[mailto:meta...@gmail.com<mailto:meta...@gmail.com>] On Behalf Of Richard Smith
Sent: Tuesday, December 08, 2015 11:42 AM
To: Robinson, Paul
Cc: Joerg Sonnenberger; cfe-commits 
(cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>)

Subject: Re: r254574 - PR17381: Treat undefined behavior during expression 
evaluation as an unmodeled

On Tue, Dec 8, 2015 at 11:18 AM, Richard Smith 
<rich...@metafoo.co.uk<mailto:rich...@metafoo.co.uk>> wrote:
On Tue, Dec 8, 2015 at 10:59 AM, Robinson, Paul 
<paul_robin...@playstation.sony.com<mailto:paul_robin...@playstation.sony.com>> 
wrote:
Okay, I'll bite:  so what *does* UINT128_MAX actually convert to?

$ echo 'unsigned __int128 max = -1; float f = max;' | ~/clang-8/build/bin/clang 
-x c++ - -emit-llvm -S -o - -O3 | grep @f
@f = global float undef, align 4

And at runtime, on some targets, we use this:

  https://llvm.org/svn/llvm-project/compiler-rt/trunk/lib/builtins/floatuntisf.c

... which gives a NaN in this case.

From: cfe-commits 
[mailto:cfe-commits-boun...@lists.llvm.org<mailto:cfe-commits-boun...@lists.llvm.org>]
 On Behalf Of Richard Smith via cfe-commits
Sent: Tuesday, December 08, 2015 10:52 AM
To: Joerg Sonnenberger; cfe-commits
Subject: Re: r254574 - PR17381: Treat undefined behavior during expression 
evaluation as an unmodeled

On Tue, Dec 8, 2015 at 2:13 AM, Joerg Sonnenberger via cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
On Mon, Dec 07, 2015 at 01:32:14PM -0800, Richard Smith via cfe-commits wrote:
> C11 6.3.1.5/1<http://6.3.1.5/1>: "If the value being converted is outside the 
> range of values
> that can be represented, the behavior is undefined."

The value of 1e100 can be represented as +inf, even if not precisely.

Only if +inf is in the range of representable values, which, as already noted, 
is problematic.

This is a bit different from non-IEEE math like VAX, that doesn't have
infinities.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits




___
cfe-commits mailing list
cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-08 Thread Joerg Sonnenberger via cfe-commits 
On Mon, Dec 07, 2015 at 01:32:14PM -0800, Richard Smith via cfe-commits wrote:
> Worse, it seems
> > even using __builtin_nan() for example doesn't work.
> >
> 
> __builtin_nan() works fine for me, can you provide a testcase?

I take this part back, pilot error.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-08 Thread Joerg Sonnenberger via cfe-commits 
On Mon, Dec 07, 2015 at 01:32:14PM -0800, Richard Smith via cfe-commits wrote:
> C11 6.3.1.5/1: "If the value being converted is outside the range of values
> that can be represented, the behavior is undefined."

The value of 1e100 can be represented as +inf, even if not precisely.
This is a bit different from non-IEEE math like VAX, that doesn't have
infinities.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-08 Thread Robinson, Paul via cfe-commits 
| I've amended this change to permit constant-foldable UB in C variable 
initializers again in r254992.
Works for us, thanks!

| | (2) Shouldn't it diagnose each bad expression in an initializer?
| That change would be unrelated to the one at hand -- this is the way we've 
always behaved.
If it's consistent with past practice, that's fine.
Thanks,
--paulr

From: meta...@gmail.com [mailto:meta...@gmail.com] On Behalf Of Richard Smith
Sent: Monday, December 07, 2015 7:25 PM
To: Robinson, Paul
Cc: cfe-commits (cfe-commits@lists.llvm.org)
Subject: Re: r254574 - PR17381: Treat undefined behavior during expression 
evaluation as an unmodeled

I've amended this change to permit constant-foldable UB in C variable 
initializers again in r254992.

On Mon, Dec 7, 2015 at 6:45 PM, Robinson, Paul via cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
Two more questions:

(1) Commit message implied this is only for C, but I see it with C++11
(but not C++03).
$ cat t.cpp
enum { foo = 123456 * 234567 };
$ clang -c t.cpp -std=c++03
$ clang -c t.cpp -std=c++11
t.cpp:1:14: error: expression is not an integral constant expression

That is the behavior required by the C++11 standard, but we used to accept GNU 
constant folding here as an extension.



(2) Shouldn't it diagnose each bad expression in an initializer?
I see the error only for the first such expression.

$ cat t.c
int foo[2] = { 123456 * 234567, 654321 * 765432 };
$ clang -c t.c
t.c:1:23: error: initializer element is not a compile-time constant
int foo[2] = { 123456 * 234567, 654321 * 765432 };
   ~~~^~~~

That change would be unrelated to the one at hand -- this is the way we've 
always behaved. Consider for instance:

  int n;
  int foo[2] = { n, n };

If you wanted to make us report the other ones, the relevant code is in 
Expr::isConstantInitializer and Sema::CheckForConstantInitializer.

1 error generated.

Thanks,
--paulr

> -Original Message-
> From: cfe-commits 
> [mailto:cfe-commits-boun...@lists.llvm.org<mailto:cfe-commits-boun...@lists.llvm.org>]
>  On Behalf Of
> Richard Smith via cfe-commits
> Sent: Wednesday, December 02, 2015 5:36 PM
> To: cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>
> Subject: r254574 - PR17381: Treat undefined behavior during expression
> evaluation as an unmodeled
>
> Author: rsmith
> Date: Wed Dec  2 19:36:22 2015
> New Revision: 254574
>
> URL: http://llvm.org/viewvc/llvm-project?rev=254574=rev
> Log:
> PR17381: Treat undefined behavior during expression evaluation as an
> unmodeled
> side-effect, so that we don't allow speculative evaluation of such
> expressions
> during code generation.
>
> This caused a diagnostic quality regression, so fix constant expression
> diagnostics to prefer either the first "can't be constant folded"
> diagnostic or
> the first "not a constant expression" diagnostic depending on the kind of
> evaluation we're doing. This was always the intent, but didn't quite work
> correctly before.
>
> This results in certain initializers that used to be constant initializers
> to
> no longer be; in particular, things like:
>
>   float f = 1e100;
>
> are no longer accepted in C. This seems appropriate, as such constructs
> would
> lead to code being executed if sanitizers are enabled.
>
> Added:
> cfe/trunk/test/CodeGen/ubsan-conditional.c
> Modified:
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp
> cfe/trunk/test/CodeGen/complex-init-list.c
> cfe/trunk/test/PCH/floating-literal.c
> cfe/trunk/test/Sema/const-eval.c
> cfe/trunk/test/Sema/integer-overflow.c
> cfe/trunk/test/Sema/switch-1.c
> cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
> cfe/trunk/test/SemaCXX/constexpr-printing.cpp
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=254574=254573=254574&
> view=diff
> ==
> 
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Dec  2 19:36:22 2015
> @@ -473,6 +473,10 @@ namespace {
>  /// notes attached to it will also be stored, otherwise they will not
> be.
>  bool HasActiveDiagnostic;
>
> +/// \brief Have we emitted a diagnostic explaining why we couldn't
> constant
> +/// fold (not just why it's not strictly a constant expression)?
> +bool HasFoldFailureDiagnostic;
> +
>  enum EvaluationMode {
>/// Evaluate as a constant expression. Stop if we find that the
> expression
>/// is not a consta

RE: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-08 Thread Robinson, Paul via cfe-commits 
Okay, I'll bite:  so what *does* UINT128_MAX actually convert to?

From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of 
Richard Smith via cfe-commits
Sent: Tuesday, December 08, 2015 10:52 AM
To: Joerg Sonnenberger; cfe-commits
Subject: Re: r254574 - PR17381: Treat undefined behavior during expression 
evaluation as an unmodeled

On Tue, Dec 8, 2015 at 2:13 AM, Joerg Sonnenberger via cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
On Mon, Dec 07, 2015 at 01:32:14PM -0800, Richard Smith via cfe-commits wrote:
> C11 6.3.1.5/1<http://6.3.1.5/1>: "If the value being converted is outside the 
> range of values
> that can be represented, the behavior is undefined."

The value of 1e100 can be represented as +inf, even if not precisely.

Only if +inf is in the range of representable values, which, as already noted, 
is problematic.

This is a bit different from non-IEEE math like VAX, that doesn't have
infinities.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-08 Thread Richard Smith via cfe-commits 
On Tue, Dec 8, 2015 at 11:18 AM, Richard Smith <rich...@metafoo.co.uk>
wrote:

> On Tue, Dec 8, 2015 at 10:59 AM, Robinson, Paul <
> paul_robin...@playstation.sony.com> wrote:
>
>> Okay, I'll bite:  so what *does* UINT128_MAX actually convert to?
>>
>
> $ echo 'unsigned __int128 max = -1; float f = max;' |
> ~/clang-8/build/bin/clang -x c++ - -emit-llvm -S -o - -O3 | grep @f
> @f = global float undef, align 4
>

And at runtime, on some targets, we use this:


https://llvm.org/svn/llvm-project/compiler-rt/trunk/lib/builtins/floatuntisf.c

... which gives a NaN in this case.

*From:* cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] *On Behalf
>> Of *Richard Smith via cfe-commits
>> *Sent:* Tuesday, December 08, 2015 10:52 AM
>> *To:* Joerg Sonnenberger; cfe-commits
>> *Subject:* Re: r254574 - PR17381: Treat undefined behavior during
>> expression evaluation as an unmodeled
>>
>>
>>
>> On Tue, Dec 8, 2015 at 2:13 AM, Joerg Sonnenberger via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>> On Mon, Dec 07, 2015 at 01:32:14PM -0800, Richard Smith via cfe-commits
>> wrote:
>> > C11 6.3.1.5/1: "If the value being converted is outside the range of
>> values
>> > that can be represented, the behavior is undefined."
>>
>> The value of 1e100 can be represented as +inf, even if not precisely.
>>
>>
>>
>> Only if +inf is in the range of representable values, which, as already
>> noted, is problematic.
>>
>>
>>
>> This is a bit different from non-IEEE math like VAX, that doesn't have
>> infinities.
>>
>>
>> Joerg
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-08 Thread Richard Smith via cfe-commits 
On Tue, Dec 8, 2015 at 10:59 AM, Robinson, Paul <
paul_robin...@playstation.sony.com> wrote:

> Okay, I'll bite:  so what *does* UINT128_MAX actually convert to?
>

$ echo 'unsigned __int128 max = -1; float f = max;' |
~/clang-8/build/bin/clang -x c++ - -emit-llvm -S -o - -O3 | grep @f
@f = global float undef, align 4


> *From:* cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] *On
> Behalf Of *Richard Smith via cfe-commits
> *Sent:* Tuesday, December 08, 2015 10:52 AM
> *To:* Joerg Sonnenberger; cfe-commits
> *Subject:* Re: r254574 - PR17381: Treat undefined behavior during
> expression evaluation as an unmodeled
>
>
>
> On Tue, Dec 8, 2015 at 2:13 AM, Joerg Sonnenberger via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> On Mon, Dec 07, 2015 at 01:32:14PM -0800, Richard Smith via cfe-commits
> wrote:
> > C11 6.3.1.5/1: "If the value being converted is outside the range of
> values
> > that can be represented, the behavior is undefined."
>
> The value of 1e100 can be represented as +inf, even if not precisely.
>
>
>
> Only if +inf is in the range of representable values, which, as already
> noted, is problematic.
>
>
>
> This is a bit different from non-IEEE math like VAX, that doesn't have
> infinities.
>
>
> Joerg
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-07 Thread Joerg Sonnenberger via cfe-commits 
On Thu, Dec 03, 2015 at 01:36:22AM -, Richard Smith via cfe-commits wrote:
> Author: rsmith
> Date: Wed Dec  2 19:36:22 2015
> New Revision: 254574
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=254574=rev
> Log:
> PR17381: Treat undefined behavior during expression evaluation as an unmodeled
> side-effect, so that we don't allow speculative evaluation of such expressions
> during code generation.
> 
> This caused a diagnostic quality regression, so fix constant expression
> diagnostics to prefer either the first "can't be constant folded" diagnostic 
> or
> the first "not a constant expression" diagnostic depending on the kind of
> evaluation we're doing. This was always the intent, but didn't quite work
> correctly before.
> 
> This results in certain initializers that used to be constant initializers to
> no longer be; in particular, things like:
> 
>   float f = 1e100;
> 
> are no longer accepted in C. This seems appropriate, as such constructs would
> lead to code being executed if sanitizers are enabled.

This leads to some pretty annoying regressions as it now seems to be
impossible to use NaN or infinites as constant initializers. Expressions
like 0.0 / 0.0, 1.0 / 0.0 and -1.0 / 0.0 are perfectly well defined
under normal IEEE rules, so they shouldn't be rejected. Worse, it seems
even using __builtin_nan() for example doesn't work.

I'm not even sure about the example given in the commit message, how
exactly is that undefined behavior?

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-07 Thread Robinson, Paul via cfe-commits 
Two more questions:

(1) Commit message implied this is only for C, but I see it with C++11
(but not C++03).

$ cat t.cpp
enum { foo = 123456 * 234567 };
$ clang -c t.cpp -std=c++03
$ clang -c t.cpp -std=c++11
t.cpp:1:14: error: expression is not an integral constant expression


(2) Shouldn't it diagnose each bad expression in an initializer?
I see the error only for the first such expression.

$ cat t.c
int foo[2] = { 123456 * 234567, 654321 * 765432 };
$ clang -c t.c
t.c:1:23: error: initializer element is not a compile-time constant
int foo[2] = { 123456 * 234567, 654321 * 765432 };
   ~~~^~~~
1 error generated.

Thanks,
--paulr

> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of
> Richard Smith via cfe-commits
> Sent: Wednesday, December 02, 2015 5:36 PM
> To: cfe-commits@lists.llvm.org
> Subject: r254574 - PR17381: Treat undefined behavior during expression
> evaluation as an unmodeled
> 
> Author: rsmith
> Date: Wed Dec  2 19:36:22 2015
> New Revision: 254574
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=254574=rev
> Log:
> PR17381: Treat undefined behavior during expression evaluation as an
> unmodeled
> side-effect, so that we don't allow speculative evaluation of such
> expressions
> during code generation.
> 
> This caused a diagnostic quality regression, so fix constant expression
> diagnostics to prefer either the first "can't be constant folded"
> diagnostic or
> the first "not a constant expression" diagnostic depending on the kind of
> evaluation we're doing. This was always the intent, but didn't quite work
> correctly before.
> 
> This results in certain initializers that used to be constant initializers
> to
> no longer be; in particular, things like:
> 
>   float f = 1e100;
> 
> are no longer accepted in C. This seems appropriate, as such constructs
> would
> lead to code being executed if sanitizers are enabled.
> 
> Added:
> cfe/trunk/test/CodeGen/ubsan-conditional.c
> Modified:
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp
> cfe/trunk/test/CodeGen/complex-init-list.c
> cfe/trunk/test/PCH/floating-literal.c
> cfe/trunk/test/Sema/const-eval.c
> cfe/trunk/test/Sema/integer-overflow.c
> cfe/trunk/test/Sema/switch-1.c
> cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
> cfe/trunk/test/SemaCXX/constexpr-printing.cpp
> 
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=254574=254573=254574&
> view=diff
> ==
> 
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Dec  2 19:36:22 2015
> @@ -473,6 +473,10 @@ namespace {
>  /// notes attached to it will also be stored, otherwise they will not
> be.
>  bool HasActiveDiagnostic;
> 
> +/// \brief Have we emitted a diagnostic explaining why we couldn't
> constant
> +/// fold (not just why it's not strictly a constant expression)?
> +bool HasFoldFailureDiagnostic;
> +
>  enum EvaluationMode {
>/// Evaluate as a constant expression. Stop if we find that the
> expression
>/// is not a constant expression.
> @@ -537,7 +541,7 @@ namespace {
>  BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
>  EvaluatingDecl((const ValueDecl *)nullptr),
>  EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
> -EvalMode(Mode) {}
> +HasFoldFailureDiagnostic(false), EvalMode(Mode) {}
> 
>  void setEvaluatingDecl(APValue::LValueBase Base, APValue ) {
>EvaluatingDecl = Base;
> @@ -597,7 +601,7 @@ namespace {
>  /// Diagnose that the evaluation cannot be folded.
>  OptionalDiagnostic Diag(SourceLocation Loc, diag::kind DiagId
>= diag::note_invalid_subexpr_in_const_expr,
> -unsigned ExtraNotes = 0) {
> +unsigned ExtraNotes = 0, bool IsCCEDiag =
> false) {
>if (EvalStatus.Diag) {
>  // If we have a prior diagnostic, it will be noting that the
> expression
>  // isn't a constant expression. This diagnostic is more
> important,
> @@ -610,10 +614,9 @@ namespace {
>case EM_ConstantFold:
>case EM_IgnoreSideEffects:
>case EM_EvaluateForOverflow:
> -if (!EvalStatus.HasSideEffects)
> +if (!HasFoldFailureDiagnostic)
>break;
> -// We've had side-effects; we want the diagnostic from them,
> not
> -// some later problem.
> +// We've already failed to fold something. Keep that
> diagnostic.
>case EM_ConstantExpression:
>case EM_PotentialConstantExpression:
>case EM_ConstantExpressionUnevaluated:
> @@ -632,6

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-07 Thread Richard Smith via cfe-commits 
[Adding back cfe-commits]

On Mon, Dec 7, 2015 at 6:46 PM, Richard Smith <rich...@metafoo.co.uk> wrote:

> On Mon, Dec 7, 2015 at 1:59 PM, Robinson, Paul <
> paul_robin...@playstation.sony.com> wrote:
>
>> | C11 6.3.1.5/1: "If the value being converted is outside the range of
>> values that can be represented, the behavior is undefined."
>>
>>
>>
>> I think 5.2.4.2.2/5 says if infinities are representable, there are no
>> values "outside" the floating-point range?
>>
>
> That interpretation would also require converting UINT128_MAX to float to
> have defined behavior (it must give either FLT_MAX or +inf, as those are
> the two closest represetnable values), which it does not on real hardware.
>
>
>> And 6.3.1.5/1 starts out with "if the value being converted can be
>> represented exactly in the new type, it is unchanged."  I'd think that NaNs
>> could be represented "exactly" (they can certainly be represented:
>> 5.2.4.2.2/3) despite not having an actual numeric value.
>>
>
> In talking about NaNs, you've switched to talking about a different case;
> we were talking about doubles that don't fit in a float. The division by
> zero cases have UB because 6.5.5/5 explicitly says they do.
>
>
>> I don't think 6.3.1.5/1 means to exclude NaN/infinities, given that
>> 6.3.1.4/1 explicitly refers to "finite value" wrt conversion to an
>> integer type.
>>
>> --paulr
>>
>>
>>
>> *From:* cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] *On
>> Behalf Of *Richard Smith via cfe-commits
>> *Sent:* Monday, December 07, 2015 1:32 PM
>> *To:* Joerg Sonnenberger; cfe-commits
>> *Subject:* Re: r254574 - PR17381: Treat undefined behavior during
>> expression evaluation as an unmodeled
>>
>>
>>
>> On Mon, Dec 7, 2015 at 7:25 AM, Joerg Sonnenberger via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>> On Thu, Dec 03, 2015 at 01:36:22AM -, Richard Smith via cfe-commits
>> wrote:
>> > Author: rsmith
>> > Date: Wed Dec  2 19:36:22 2015
>> > New Revision: 254574
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=254574=rev
>> > Log:
>> > PR17381: Treat undefined behavior during expression evaluation as an
>> unmodeled
>> > side-effect, so that we don't allow speculative evaluation of such
>> expressions
>> > during code generation.
>> >
>> > This caused a diagnostic quality regression, so fix constant expression
>> > diagnostics to prefer either the first "can't be constant folded"
>> diagnostic or
>> > the first "not a constant expression" diagnostic depending on the kind
>> of
>> > evaluation we're doing. This was always the intent, but didn't quite
>> work
>> > correctly before.
>> >
>> > This results in certain initializers that used to be constant
>> initializers to
>> > no longer be; in particular, things like:
>> >
>> >   float f = 1e100;
>> >
>> > are no longer accepted in C. This seems appropriate, as such constructs
>> would
>> > lead to code being executed if sanitizers are enabled.
>>
>> This leads to some pretty annoying regressions as it now seems to be
>> impossible to use NaN or infinites as constant initializers. Expressions
>> like 0.0 / 0.0, 1.0 / 0.0 and -1.0 / 0.0 are perfectly well defined
>> under normal IEEE rules, so they shouldn't be rejected.
>>
>>
>>
>> Well, we have a problem. The evaluation semantics of these expressions
>> requires code to execute in some build modes (in particular, with
>> sanitizers enabled), and thus has a side-effect.
>>
>>
>>
>> I'm inclined to relax the restriction added in this change for the
>> specific case of global variables in C, since (as you say) there is a fair
>> amount of code using divide-by-zero as a "portable" way of generating an
>> inf or nan.
>>
>>
>>
>> Worse, it seems
>> even using __builtin_nan() for example doesn't work.
>>
>>
>>
>> __builtin_nan() works fine for me, can you provide a testcase?
>>
>>
>>
>> I'm not even sure about the example given in the commit message, how
>> exactly is that undefined behavior?
>>
>>
>>
>> C11 6.3.1.5/1: "If the value being converted is outside the range of
>> values that can be represented, the behavior is undefined."
>>
>>
>>
>> We also have C11 6.6/4: "Each constant expression shall evaluate to a
>> constant that is in the range of representable values for its type."
>>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-07 Thread Richard Smith via cfe-commits 
On Mon, Dec 7, 2015 at 5:26 PM, Reid Kleckner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> It wasn't Chrome, it was some internal dependency on torch-cephes. I
> submitted a patch for it upstream:
>
> https://github.com/deepmind/torch-cephes/commit/9c4a97c90dc200ecbecb883e7230fe3c847954df
> It's not a pretty, though.
>
> 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.
>

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.

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.


> On Mon, Dec 7, 2015 at 2:37 PM, David Majnemer 
> wrote:
>
>>
>>
>> On Mon, Dec 7, 2015 at 5:25 PM, Hans Wennborg  wrote:
>>
>>> On Mon, Dec 7, 2015 at 2:14 PM, David Majnemer 
>>> wrote:
>>> >
>>> >
>>> > On Mon, Dec 7, 2015 at 4:32 PM, Richard Smith via cfe-commits
>>> >  wrote:
>>> >>
>>> >> On Mon, Dec 7, 2015 at 7:25 AM, Joerg Sonnenberger via cfe-commits
>>> >>  wrote:
>>> >>>
>>> >>> On Thu, Dec 03, 2015 at 01:36:22AM -, Richard Smith via
>>> cfe-commits
>>> >>> wrote:
>>> >>> > Author: rsmith
>>> >>> > Date: Wed Dec  2 19:36:22 2015
>>> >>> > New Revision: 254574
>>> >>> >
>>> >>> > URL: http://llvm.org/viewvc/llvm-project?rev=254574=rev
>>> >>> > Log:
>>> >>> > PR17381: Treat undefined behavior during expression evaluation as
>>> an
>>> >>> > unmodeled
>>> >>> > side-effect, so that we don't allow speculative evaluation of such
>>> >>> > expressions
>>> >>> > during code generation.
>>> >>> >
>>> >>> > This caused a diagnostic quality regression, so fix constant
>>> expression
>>> >>> > diagnostics to prefer either the first "can't be constant folded"
>>> >>> > diagnostic or
>>> >>> > the first "not a constant expression" diagnostic depending on the
>>> kind
>>> >>> > of
>>> >>> > evaluation we're doing. This was always the intent, but didn't
>>> quite
>>> >>> > work
>>> >>> > correctly before.
>>> >>> >
>>> >>> > This results in certain initializers that used to be constant
>>> >>> > initializers to
>>> >>> > no longer be; in particular, things like:
>>> >>> >
>>> >>> >   float f = 1e100;
>>> >>> >
>>> >>> > are no longer accepted in C. This seems appropriate, as such
>>> constructs
>>> >>> > would
>>> >>> > lead to code being executed if sanitizers are enabled.
>>> >>>
>>> >>> This leads to some pretty annoying regressions as it now seems to be
>>> >>> impossible to use NaN or infinites as constant initializers.
>>> Expressions
>>> >>> like 0.0 / 0.0, 1.0 / 0.0 and -1.0 / 0.0 are perfectly well defined
>>> >>> under normal IEEE rules, so they shouldn't be rejected.
>>> >>
>>> >>
>>> >> Well, we have a problem. The evaluation semantics of these expressions
>>> >> requires code to execute in some build modes (in particular, with
>>> sanitizers
>>> >> enabled), and thus has a side-effect.
>>> >>
>>> >> I'm inclined to relax the restriction added in this change for the
>>> >> specific case of global variables in C, since (as you say) there is a
>>> fair
>>> >> amount of code using divide-by-zero as a "portable" way of generating
>>> an inf
>>> >> or nan.
>>> >>
>>> >>> Worse, it seems
>>> >>> even using __builtin_nan() for example doesn't work.
>>> >>
>>> >>
>>> >> __builtin_nan() works fine for me, can you provide a testcase?
>>> >>
>>> >>> I'm not even sure about the example given in the commit message, how
>>> >>> exactly is that undefined behavior?
>>> >>
>>> >>
>>> >> C11 6.3.1.5/1: "If the value being converted is outside the range of
>>> >> values that can be represented, the behavior is undefined."
>>> >
>>> >
>>> > I don't think we want to make the UB here true UB.  It would mean that
>>> code
>>> > which expected to get NaN might get undef, even outside of constant
>>> > expression evaluation.  The implementation defined behavior of
>>> providing NaN
>>> > seems more friendly...  IIRC, this broke Chrome recently because folks
>>> were
>>> > doing this in C++.  Hans, do you remember the details?
>>>
>>> Hmm, it doesn't ring a bell, but my memory sometimes fails me. Didn't
>>> you and Reid look at

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-07 Thread Richard Smith via cfe-commits 
On Mon, Dec 7, 2015 at 7:25 AM, Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Thu, Dec 03, 2015 at 01:36:22AM -, Richard Smith via cfe-commits
> wrote:
> > Author: rsmith
> > Date: Wed Dec  2 19:36:22 2015
> > New Revision: 254574
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=254574=rev
> > Log:
> > PR17381: Treat undefined behavior during expression evaluation as an
> unmodeled
> > side-effect, so that we don't allow speculative evaluation of such
> expressions
> > during code generation.
> >
> > This caused a diagnostic quality regression, so fix constant expression
> > diagnostics to prefer either the first "can't be constant folded"
> diagnostic or
> > the first "not a constant expression" diagnostic depending on the kind of
> > evaluation we're doing. This was always the intent, but didn't quite work
> > correctly before.
> >
> > This results in certain initializers that used to be constant
> initializers to
> > no longer be; in particular, things like:
> >
> >   float f = 1e100;
> >
> > are no longer accepted in C. This seems appropriate, as such constructs
> would
> > lead to code being executed if sanitizers are enabled.
>
> This leads to some pretty annoying regressions as it now seems to be
> impossible to use NaN or infinites as constant initializers. Expressions
> like 0.0 / 0.0, 1.0 / 0.0 and -1.0 / 0.0 are perfectly well defined
> under normal IEEE rules, so they shouldn't be rejected.


Well, we have a problem. The evaluation semantics of these expressions
requires code to execute in some build modes (in particular, with
sanitizers enabled), and thus has a side-effect.

I'm inclined to relax the restriction added in this change for the specific
case of global variables in C, since (as you say) there is a fair amount of
code using divide-by-zero as a "portable" way of generating an inf or nan.

Worse, it seems
> even using __builtin_nan() for example doesn't work.
>

__builtin_nan() works fine for me, can you provide a testcase?

I'm not even sure about the example given in the commit message, how
> exactly is that undefined behavior?


C11 6.3.1.5/1: "If the value being converted is outside the range of values
that can be represented, the behavior is undefined."

We also have C11 6.6/4: "Each constant expression shall evaluate to a
constant that is in the range of representable values for its type."
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-07 Thread Robinson, Paul via cfe-commits 
Explicitly cc: cfe-commits *again*….

| C11 6.3.1.5/1<http://6.3.1.5/1>: "If the value being converted is outside the 
range of values that can be represented, the behavior is undefined."

I think 5.2.4.2.2/5 says if infinities are representable, there are no values 
"outside" the floating-point range?
And 6.3.1.5/1 starts out with "if the value being converted can be represented 
exactly in the new type, it is unchanged."  I'd think that NaNs could be 
represented "exactly" (they can certainly be represented: 5.2.4.2.2/3) despite 
not having an actual numeric value.
I don't think 6.3.1.5/1 means to exclude NaN/infinities, given that 6.3.1.4/1 
explicitly refers to "finite value" wrt conversion to an integer type.
--paulr



From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of 
Richard Smith via cfe-commits
Sent: Monday, December 07, 2015 1:32 PM
To: Joerg Sonnenberger; cfe-commits
Subject: Re: r254574 - PR17381: Treat undefined behavior during expression 
evaluation as an unmodeled

On Mon, Dec 7, 2015 at 7:25 AM, Joerg Sonnenberger via cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
On Thu, Dec 03, 2015 at 01:36:22AM -, Richard Smith via cfe-commits wrote:
> Author: rsmith
> Date: Wed Dec  2 19:36:22 2015
> New Revision: 254574
>
> URL: http://llvm.org/viewvc/llvm-project?rev=254574=rev
> Log:
> PR17381: Treat undefined behavior during expression evaluation as an unmodeled
> side-effect, so that we don't allow speculative evaluation of such expressions
> during code generation.
>
> This caused a diagnostic quality regression, so fix constant expression
> diagnostics to prefer either the first "can't be constant folded" diagnostic 
> or
> the first "not a constant expression" diagnostic depending on the kind of
> evaluation we're doing. This was always the intent, but didn't quite work
> correctly before.
>
> This results in certain initializers that used to be constant initializers to
> no longer be; in particular, things like:
>
>   float f = 1e100;
>
> are no longer accepted in C. This seems appropriate, as such constructs would
> lead to code being executed if sanitizers are enabled.

This leads to some pretty annoying regressions as it now seems to be
impossible to use NaN or infinites as constant initializers. Expressions
like 0.0 / 0.0, 1.0 / 0.0 and -1.0 / 0.0 are perfectly well defined
under normal IEEE rules, so they shouldn't be rejected.

Well, we have a problem. The evaluation semantics of these expressions requires 
code to execute in some build modes (in particular, with sanitizers enabled), 
and thus has a side-effect.

I'm inclined to relax the restriction added in this change for the specific 
case of global variables in C, since (as you say) there is a fair amount of 
code using divide-by-zero as a "portable" way of generating an inf or nan.

Worse, it seems
even using __builtin_nan() for example doesn't work.

__builtin_nan() works fine for me, can you provide a testcase?

I'm not even sure about the example given in the commit message, how
exactly is that undefined behavior?

C11 6.3.1.5/1<http://6.3.1.5/1>: "If the value being converted is outside the 
range of values that can be represented, the behavior is undefined."

We also have C11 6.6/4: "Each constant expression shall evaluate to a constant 
that is in the range of representable values for its type."
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-07 Thread David Majnemer via cfe-commits 
On Mon, Dec 7, 2015 at 4:32 PM, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Mon, Dec 7, 2015 at 7:25 AM, Joerg Sonnenberger via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> On Thu, Dec 03, 2015 at 01:36:22AM -, Richard Smith via cfe-commits
>> wrote:
>> > Author: rsmith
>> > Date: Wed Dec  2 19:36:22 2015
>> > New Revision: 254574
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=254574=rev
>> > Log:
>> > PR17381: Treat undefined behavior during expression evaluation as an
>> unmodeled
>> > side-effect, so that we don't allow speculative evaluation of such
>> expressions
>> > during code generation.
>> >
>> > This caused a diagnostic quality regression, so fix constant expression
>> > diagnostics to prefer either the first "can't be constant folded"
>> diagnostic or
>> > the first "not a constant expression" diagnostic depending on the kind
>> of
>> > evaluation we're doing. This was always the intent, but didn't quite
>> work
>> > correctly before.
>> >
>> > This results in certain initializers that used to be constant
>> initializers to
>> > no longer be; in particular, things like:
>> >
>> >   float f = 1e100;
>> >
>> > are no longer accepted in C. This seems appropriate, as such constructs
>> would
>> > lead to code being executed if sanitizers are enabled.
>>
>> This leads to some pretty annoying regressions as it now seems to be
>> impossible to use NaN or infinites as constant initializers. Expressions
>> like 0.0 / 0.0, 1.0 / 0.0 and -1.0 / 0.0 are perfectly well defined
>> under normal IEEE rules, so they shouldn't be rejected.
>
>
> Well, we have a problem. The evaluation semantics of these expressions
> requires code to execute in some build modes (in particular, with
> sanitizers enabled), and thus has a side-effect.
>
> I'm inclined to relax the restriction added in this change for the
> specific case of global variables in C, since (as you say) there is a fair
> amount of code using divide-by-zero as a "portable" way of generating an
> inf or nan.
>
> Worse, it seems
>> even using __builtin_nan() for example doesn't work.
>>
>
> __builtin_nan() works fine for me, can you provide a testcase?
>
> I'm not even sure about the example given in the commit message, how
>> exactly is that undefined behavior?
>
>
> C11 6.3.1.5/1: "If the value being converted is outside the range of
> values that can be represented, the behavior is undefined."
>

I don't think we want to make the UB here true UB.  It would mean that code
which expected to get NaN might get undef, even outside of constant
expression evaluation.  The implementation defined behavior of providing
NaN seems more friendly...  IIRC, this broke Chrome recently because folks
were doing this in C++.  Hans, do you remember the details?


>
> We also have C11 6.6/4: "Each constant expression shall evaluate to a
> constant that is in the range of representable values for its type."
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-07 Thread Hans Wennborg via cfe-commits 
On Mon, Dec 7, 2015 at 2:14 PM, David Majnemer  wrote:
>
>
> On Mon, Dec 7, 2015 at 4:32 PM, Richard Smith via cfe-commits
>  wrote:
>>
>> On Mon, Dec 7, 2015 at 7:25 AM, Joerg Sonnenberger via cfe-commits
>>  wrote:
>>>
>>> On Thu, Dec 03, 2015 at 01:36:22AM -, Richard Smith via cfe-commits
>>> wrote:
>>> > Author: rsmith
>>> > Date: Wed Dec  2 19:36:22 2015
>>> > New Revision: 254574
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=254574=rev
>>> > Log:
>>> > PR17381: Treat undefined behavior during expression evaluation as an
>>> > unmodeled
>>> > side-effect, so that we don't allow speculative evaluation of such
>>> > expressions
>>> > during code generation.
>>> >
>>> > This caused a diagnostic quality regression, so fix constant expression
>>> > diagnostics to prefer either the first "can't be constant folded"
>>> > diagnostic or
>>> > the first "not a constant expression" diagnostic depending on the kind
>>> > of
>>> > evaluation we're doing. This was always the intent, but didn't quite
>>> > work
>>> > correctly before.
>>> >
>>> > This results in certain initializers that used to be constant
>>> > initializers to
>>> > no longer be; in particular, things like:
>>> >
>>> >   float f = 1e100;
>>> >
>>> > are no longer accepted in C. This seems appropriate, as such constructs
>>> > would
>>> > lead to code being executed if sanitizers are enabled.
>>>
>>> This leads to some pretty annoying regressions as it now seems to be
>>> impossible to use NaN or infinites as constant initializers. Expressions
>>> like 0.0 / 0.0, 1.0 / 0.0 and -1.0 / 0.0 are perfectly well defined
>>> under normal IEEE rules, so they shouldn't be rejected.
>>
>>
>> Well, we have a problem. The evaluation semantics of these expressions
>> requires code to execute in some build modes (in particular, with sanitizers
>> enabled), and thus has a side-effect.
>>
>> I'm inclined to relax the restriction added in this change for the
>> specific case of global variables in C, since (as you say) there is a fair
>> amount of code using divide-by-zero as a "portable" way of generating an inf
>> or nan.
>>
>>> Worse, it seems
>>> even using __builtin_nan() for example doesn't work.
>>
>>
>> __builtin_nan() works fine for me, can you provide a testcase?
>>
>>> I'm not even sure about the example given in the commit message, how
>>> exactly is that undefined behavior?
>>
>>
>> C11 6.3.1.5/1: "If the value being converted is outside the range of
>> values that can be represented, the behavior is undefined."
>
>
> I don't think we want to make the UB here true UB.  It would mean that code
> which expected to get NaN might get undef, even outside of constant
> expression evaluation.  The implementation defined behavior of providing NaN
> seems more friendly...  IIRC, this broke Chrome recently because folks were
> doing this in C++.  Hans, do you remember the details?

Hmm, it doesn't ring a bell, but my memory sometimes fails me. Didn't
you and Reid look at something like this the other day? (But maybe
that was in internal code?)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-07 Thread David Majnemer via cfe-commits 
On Mon, Dec 7, 2015 at 5:25 PM, Hans Wennborg  wrote:

> On Mon, Dec 7, 2015 at 2:14 PM, David Majnemer 
> wrote:
> >
> >
> > On Mon, Dec 7, 2015 at 4:32 PM, Richard Smith via cfe-commits
> >  wrote:
> >>
> >> On Mon, Dec 7, 2015 at 7:25 AM, Joerg Sonnenberger via cfe-commits
> >>  wrote:
> >>>
> >>> On Thu, Dec 03, 2015 at 01:36:22AM -, Richard Smith via cfe-commits
> >>> wrote:
> >>> > Author: rsmith
> >>> > Date: Wed Dec  2 19:36:22 2015
> >>> > New Revision: 254574
> >>> >
> >>> > URL: http://llvm.org/viewvc/llvm-project?rev=254574=rev
> >>> > Log:
> >>> > PR17381: Treat undefined behavior during expression evaluation as an
> >>> > unmodeled
> >>> > side-effect, so that we don't allow speculative evaluation of such
> >>> > expressions
> >>> > during code generation.
> >>> >
> >>> > This caused a diagnostic quality regression, so fix constant
> expression
> >>> > diagnostics to prefer either the first "can't be constant folded"
> >>> > diagnostic or
> >>> > the first "not a constant expression" diagnostic depending on the
> kind
> >>> > of
> >>> > evaluation we're doing. This was always the intent, but didn't quite
> >>> > work
> >>> > correctly before.
> >>> >
> >>> > This results in certain initializers that used to be constant
> >>> > initializers to
> >>> > no longer be; in particular, things like:
> >>> >
> >>> >   float f = 1e100;
> >>> >
> >>> > are no longer accepted in C. This seems appropriate, as such
> constructs
> >>> > would
> >>> > lead to code being executed if sanitizers are enabled.
> >>>
> >>> This leads to some pretty annoying regressions as it now seems to be
> >>> impossible to use NaN or infinites as constant initializers.
> Expressions
> >>> like 0.0 / 0.0, 1.0 / 0.0 and -1.0 / 0.0 are perfectly well defined
> >>> under normal IEEE rules, so they shouldn't be rejected.
> >>
> >>
> >> Well, we have a problem. The evaluation semantics of these expressions
> >> requires code to execute in some build modes (in particular, with
> sanitizers
> >> enabled), and thus has a side-effect.
> >>
> >> I'm inclined to relax the restriction added in this change for the
> >> specific case of global variables in C, since (as you say) there is a
> fair
> >> amount of code using divide-by-zero as a "portable" way of generating
> an inf
> >> or nan.
> >>
> >>> Worse, it seems
> >>> even using __builtin_nan() for example doesn't work.
> >>
> >>
> >> __builtin_nan() works fine for me, can you provide a testcase?
> >>
> >>> I'm not even sure about the example given in the commit message, how
> >>> exactly is that undefined behavior?
> >>
> >>
> >> C11 6.3.1.5/1: "If the value being converted is outside the range of
> >> values that can be represented, the behavior is undefined."
> >
> >
> > I don't think we want to make the UB here true UB.  It would mean that
> code
> > which expected to get NaN might get undef, even outside of constant
> > expression evaluation.  The implementation defined behavior of providing
> NaN
> > seems more friendly...  IIRC, this broke Chrome recently because folks
> were
> > doing this in C++.  Hans, do you remember the details?
>
> Hmm, it doesn't ring a bell, but my memory sometimes fails me. Didn't
> you and Reid look at something like this the other day? (But maybe
> that was in internal code?)
>

Ah, I think you are right... Reid, do you happen to recall?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-07 Thread Richard Smith via cfe-commits 
I've amended this change to permit constant-foldable UB in C variable
initializers again in r254992.

On Mon, Dec 7, 2015 at 6:45 PM, Robinson, Paul via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Two more questions:
>
> (1) Commit message implied this is only for C, but I see it with C++11
> (but not C++03).

$ cat t.cpp
> enum { foo = 123456 * 234567 };
> $ clang -c t.cpp -std=c++03
> $ clang -c t.cpp -std=c++11
> t.cpp:1:14: error: expression is not an integral constant expression
>

That is the behavior required by the C++11 standard, but we used to accept
GNU constant folding here as an extension.


>
> (2) Shouldn't it diagnose each bad expression in an initializer?
> I see the error only for the first such expression.
>
> $ cat t.c
> int foo[2] = { 123456 * 234567, 654321 * 765432 };
> $ clang -c t.c
> t.c:1:23: error: initializer element is not a compile-time constant
> int foo[2] = { 123456 * 234567, 654321 * 765432 };
>~~~^~~~
>

That change would be unrelated to the one at hand -- this is the way we've
always behaved. Consider for instance:

  int n;
  int foo[2] = { n, n };

If you wanted to make us report the other ones, the relevant code is in
Expr::isConstantInitializer and Sema::CheckForConstantInitializer.

1 error generated.
>
> Thanks,
> --paulr
>
> > -Original Message-
> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> Of
> > Richard Smith via cfe-commits
> > Sent: Wednesday, December 02, 2015 5:36 PM
> > To: cfe-commits@lists.llvm.org
> > Subject: r254574 - PR17381: Treat undefined behavior during expression
> > evaluation as an unmodeled
> >
> > Author: rsmith
> > Date: Wed Dec  2 19:36:22 2015
> > New Revision: 254574
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=254574=rev
> > Log:
> > PR17381: Treat undefined behavior during expression evaluation as an
> > unmodeled
> > side-effect, so that we don't allow speculative evaluation of such
> > expressions
> > during code generation.
> >
> > This caused a diagnostic quality regression, so fix constant expression
> > diagnostics to prefer either the first "can't be constant folded"
> > diagnostic or
> > the first "not a constant expression" diagnostic depending on the kind of
> > evaluation we're doing. This was always the intent, but didn't quite work
> > correctly before.
> >
> > This results in certain initializers that used to be constant
> initializers
> > to
> > no longer be; in particular, things like:
> >
> >   float f = 1e100;
> >
> > are no longer accepted in C. This seems appropriate, as such constructs
> > would
> > lead to code being executed if sanitizers are enabled.
> >
> > Added:
> > cfe/trunk/test/CodeGen/ubsan-conditional.c
> > Modified:
> > cfe/trunk/lib/AST/ExprConstant.cpp
> > cfe/trunk/lib/Sema/SemaChecking.cpp
> > cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp
> > cfe/trunk/test/CodeGen/complex-init-list.c
> > cfe/trunk/test/PCH/floating-literal.c
> > cfe/trunk/test/Sema/const-eval.c
> > cfe/trunk/test/Sema/integer-overflow.c
> > cfe/trunk/test/Sema/switch-1.c
> > cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
> > cfe/trunk/test/SemaCXX/constexpr-printing.cpp
> >
> > Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> > URL: http://llvm.org/viewvc/llvm-
> >
> project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=254574=254573=254574&
> > view=diff
> >
> ==
> > 
> > --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> > +++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Dec  2 19:36:22 2015
> > @@ -473,6 +473,10 @@ namespace {
> >  /// notes attached to it will also be stored, otherwise they will
> not
> > be.
> >  bool HasActiveDiagnostic;
> >
> > +/// \brief Have we emitted a diagnostic explaining why we couldn't
> > constant
> > +/// fold (not just why it's not strictly a constant expression)?
> > +bool HasFoldFailureDiagnostic;
> > +
> >  enum EvaluationMode {
> >/// Evaluate as a constant expression. Stop if we find that the
> > expression
> >/// is not a constant expression.
> > @@ -537,7 +541,7 @@ namespace {
> >  BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
> >  EvaluatingDecl((const ValueDecl *)nullptr),
> >  EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
> > -EvalMode(Mode) {}
> > +HasFoldFailureDiagnostic(false), EvalMode(Mode) {}
> >
> >  void setEvaluatingDecl(APValue::LValueBase Base, APValue ) {
> >EvaluatingDecl = Base;
> > @@ -597,7 +601,7 @@ namespace {
> >  /// Diagnose that the evaluation cannot be folded.
> >  OptionalDiagnostic Diag(SourceLocation Loc, diag::kind DiagId
> >=
> diag::note_invalid_subexpr_in_const_expr,
> > -unsigned ExtraNotes = 0) {
> > +unsigned ExtraNotes = 0, bool IsCCEDiag =
> >

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-07 Thread David Majnemer via cfe-commits 
On Mon, Dec 7, 2015 at 9:51 PM, Richard Smith  wrote:

> On Mon, Dec 7, 2015 at 5:26 PM, Reid Kleckner via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> It wasn't Chrome, it was some internal dependency on torch-cephes. I
>> submitted a patch for it upstream:
>>
>> https://github.com/deepmind/torch-cephes/commit/9c4a97c90dc200ecbecb883e7230fe3c847954df
>> It's not a pretty, though.
>>
>> 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.
>>
>
> 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.
>

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...


>
> 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.
>
>
>> On Mon, Dec 7, 2015 at 2:37 PM, David Majnemer 
>> wrote:
>>
>>>
>>>
>>> On Mon, Dec 7, 2015 at 5:25 PM, Hans Wennborg  wrote:
>>>
 On Mon, Dec 7, 2015 at 2:14 PM, David Majnemer <
 david.majne...@gmail.com> wrote:
 >
 >
 > On Mon, Dec 7, 2015 at 4:32 PM, Richard Smith via cfe-commits
 >  wrote:
 >>
 >> On Mon, Dec 7, 2015 at 7:25 AM, Joerg Sonnenberger via cfe-commits
 >>  wrote:
 >>>
 >>> On Thu, Dec 03, 2015 at 01:36:22AM -, Richard Smith via
 cfe-commits
 >>> wrote:
 >>> > Author: rsmith
 >>> > Date: Wed Dec  2 19:36:22 2015
 >>> > New Revision: 254574
 >>> >
 >>> > URL: http://llvm.org/viewvc/llvm-project?rev=254574=rev
 >>> > Log:
 >>> > PR17381: Treat undefined behavior during expression evaluation as
 an
 >>> > unmodeled
 >>> > side-effect, so that we don't allow speculative evaluation of such
 >>> > expressions
 >>> > during code generation.
 >>> >
 >>> > This caused a diagnostic quality regression, so fix constant
 expression
 >>> > diagnostics to prefer either the first "can't be constant folded"
 >>> > diagnostic or
 >>> > the first "not a constant expression" diagnostic depending on the
 kind
 >>> > of
 >>> > evaluation we're doing. This was always the intent, but didn't
 quite
 >>> > work
 >>> > correctly before.
 >>> >
 >>> > This results in certain initializers that used to be constant
 >>> > initializers to
 >>> > no longer be; in particular, things like:
 >>> >
 >>> >   float f = 1e100;
 >>> >
 >>> > are no longer accepted in C. This seems appropriate, as such
 constructs
 >>> > would
 >>> > lead to code being executed if sanitizers are enabled.
 >>>
 >>> This leads to some pretty annoying regressions as it now seems to be
 >>> impossible to use NaN or infinites as constant initializers.
 Expressions
 >>> like 0.0 / 0.0, 1.0 / 0.0 and -1.0 / 0.0 are perfectly well defined
 >>> under normal IEEE rules, so they shouldn't be rejected.
 >>
 >>
 >> Well, we have a problem. The evaluation semantics of these
 expressions
 >> requires code to execute in some build modes (in particular, with
 sanitizers
 >> enabled), and thus has a side-effect.
 >>
 >> I'm inclined to relax the restriction added in this change for the
 >> specific case of global variables in C, since (as you say) there is
 a fair
 >> amount of code using divide-by-zero as a "portable" way of
 generating an inf
 >> or nan.
 >>
 >>> Worse, it seems
 >>> even using __builtin_nan() for example doesn't work.
 >>
 >>
 >> __builtin_nan() works fine for me, can you provide a testcase?
 >>
 >>> I'm not even sure about the example given in the commit message, how
 >>> exactly is that undefined behavior?
 >>
 >>
 >> C11 6.3.1.5/1: "If the value being converted is outside the range of
 >> values that can be represented, the behavior is undefined."
 >
 >
 > I don't think we want to make the UB here true UB.  It would mean
 that code
 > which expected to get NaN might get

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-07 Thread Richard Smith via cfe-commits 
On Mon, Dec 7, 2015 at 7:20 PM, David Majnemer 
wrote:

> On Mon, Dec 7, 2015 at 9:51 PM, Richard Smith 
> wrote:
>
>> On Mon, Dec 7, 2015 at 5:26 PM, Reid Kleckner via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> It wasn't Chrome, it was some internal dependency on torch-cephes. I
>>> submitted a patch for it upstream:
>>>
>>> https://github.com/deepmind/torch-cephes/commit/9c4a97c90dc200ecbecb883e7230fe3c847954df
>>> It's not a pretty, though.
>>>
>>> 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.
>>>
>>
>> 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.
>>
>
> 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...
>

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.

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.
>>
>>
>>> On Mon, Dec 7, 2015 at 2:37 PM, David Majnemer >> > wrote:
>>>


 On Mon, Dec 7, 2015 at 5:25 PM, Hans Wennborg 
 wrote:

> On Mon, Dec 7, 2015 at 2:14 PM, David Majnemer <
> david.majne...@gmail.com> wrote:
> >
> >
> > On Mon, Dec 7, 2015 at 4:32 PM, Richard Smith via cfe-commits
> >  wrote:
> >>
> >> On Mon, Dec 7, 2015 at 7:25 AM, Joerg Sonnenberger via cfe-commits
> >>  wrote:
> >>>
> >>> On Thu, Dec 03, 2015 at 01:36:22AM -, Richard Smith via
> cfe-commits
> >>> wrote:
> >>> > Author: rsmith
> >>> > Date: Wed Dec  2 19:36:22 2015
> >>> > New Revision: 254574
> >>> >
> >>> > URL: http://llvm.org/viewvc/llvm-project?rev=254574=rev
> >>> > Log:
> >>> > PR17381: Treat undefined behavior during expression evaluation
> as an
> >>> > unmodeled
> >>> > side-effect, so that we don't allow speculative evaluation of
> such
> >>> > expressions
> >>> > during code generation.
> >>> >
> >>> > This caused a diagnostic quality regression, so fix constant
> expression
> >>> > diagnostics to prefer either the first "can't be constant folded"
> >>> > diagnostic or
> >>> > the first "not a constant expression" diagnostic depending on
> the kind
> >>> > of
> >>> > evaluation we're doing. This was always the intent, but didn't
> quite
> >>> > work
> >>> > correctly before.
> >>> >
> >>> > This results in certain initializers that used to be constant
> >>> > initializers to
> >>> > no longer be; in particular, things like:
> >>> >
> >>> >   float f = 1e100;
> >>> >
> >>> > are no longer accepted in C. This seems appropriate, as such
> constructs
> >>> > would
> >>> > lead to code being executed if sanitizers are enabled.
> >>>
> >>> This leads to some pretty annoying regressions as it now seems to
> be
> >>> impossible to use NaN or infinites as constant initializers.
> Expressions
> >>> like 0.0 / 0.0, 1.0 / 0.0 and -1.0 / 0.0 are perfectly well defined
> >>> under normal IEEE rules, so they shouldn't be rejected.
> >>
> >>
> >> Well, we have a problem. The evaluation semantics of these
> expressions
> >> requires code to execute in some build modes (in particular, with
> sanitizers
> >> enabled), and thus has a side-effect.
> >>
> >> I'm inclined to relax the restriction added in this change for the
> >> specific case of global variables in C, since (as you say) there is
> a fair
> >> amount of code using divide-by-zero as a "portable" way of
> generating an inf
> >> or nan.
> >>
> >>> Worse, it seems
> >>> even using __builtin_nan() for example doesn't work.
> >>
> >>
> >> __builtin_nan() works fine for me, can you provide a testcase?
> >>
> >>> I'm not even sure about the example given in the

Re: r254574 - PR17381: Treat undefined behavior during expression evaluation as an unmodeled

2015-12-07 Thread Reid Kleckner via cfe-commits 
It wasn't Chrome, it was some internal dependency on torch-cephes. I
submitted a patch for it upstream:
https://github.com/deepmind/torch-cephes/commit/9c4a97c90dc200ecbecb883e7230fe3c847954df
It's not a pretty, though.

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.

On Mon, Dec 7, 2015 at 2:37 PM, David Majnemer 
wrote:

>
>
> On Mon, Dec 7, 2015 at 5:25 PM, Hans Wennborg  wrote:
>
>> On Mon, Dec 7, 2015 at 2:14 PM, David Majnemer 
>> wrote:
>> >
>> >
>> > On Mon, Dec 7, 2015 at 4:32 PM, Richard Smith via cfe-commits
>> >  wrote:
>> >>
>> >> On Mon, Dec 7, 2015 at 7:25 AM, Joerg Sonnenberger via cfe-commits
>> >>  wrote:
>> >>>
>> >>> On Thu, Dec 03, 2015 at 01:36:22AM -, Richard Smith via
>> cfe-commits
>> >>> wrote:
>> >>> > Author: rsmith
>> >>> > Date: Wed Dec  2 19:36:22 2015
>> >>> > New Revision: 254574
>> >>> >
>> >>> > URL: http://llvm.org/viewvc/llvm-project?rev=254574=rev
>> >>> > Log:
>> >>> > PR17381: Treat undefined behavior during expression evaluation as an
>> >>> > unmodeled
>> >>> > side-effect, so that we don't allow speculative evaluation of such
>> >>> > expressions
>> >>> > during code generation.
>> >>> >
>> >>> > This caused a diagnostic quality regression, so fix constant
>> expression
>> >>> > diagnostics to prefer either the first "can't be constant folded"
>> >>> > diagnostic or
>> >>> > the first "not a constant expression" diagnostic depending on the
>> kind
>> >>> > of
>> >>> > evaluation we're doing. This was always the intent, but didn't quite
>> >>> > work
>> >>> > correctly before.
>> >>> >
>> >>> > This results in certain initializers that used to be constant
>> >>> > initializers to
>> >>> > no longer be; in particular, things like:
>> >>> >
>> >>> >   float f = 1e100;
>> >>> >
>> >>> > are no longer accepted in C. This seems appropriate, as such
>> constructs
>> >>> > would
>> >>> > lead to code being executed if sanitizers are enabled.
>> >>>
>> >>> This leads to some pretty annoying regressions as it now seems to be
>> >>> impossible to use NaN or infinites as constant initializers.
>> Expressions
>> >>> like 0.0 / 0.0, 1.0 / 0.0 and -1.0 / 0.0 are perfectly well defined
>> >>> under normal IEEE rules, so they shouldn't be rejected.
>> >>
>> >>
>> >> Well, we have a problem. The evaluation semantics of these expressions
>> >> requires code to execute in some build modes (in particular, with
>> sanitizers
>> >> enabled), and thus has a side-effect.
>> >>
>> >> I'm inclined to relax the restriction added in this change for the
>> >> specific case of global variables in C, since (as you say) there is a
>> fair
>> >> amount of code using divide-by-zero as a "portable" way of generating
>> an inf
>> >> or nan.
>> >>
>> >>> Worse, it seems
>> >>> even using __builtin_nan() for example doesn't work.
>> >>
>> >>
>> >> __builtin_nan() works fine for me, can you provide a testcase?
>> >>
>> >>> I'm not even sure about the example given in the commit message, how
>> >>> exactly is that undefined behavior?
>> >>
>> >>
>> >> C11 6.3.1.5/1: "If the value being converted is outside the range of
>> >> values that can be represented, the behavior is undefined."
>> >
>> >
>> > I don't think we want to make the UB here true UB.  It would mean that
>> code
>> > which expected to get NaN might get undef, even outside of constant
>> > expression evaluation.  The implementation defined behavior of
>> providing NaN
>> > seems more friendly...  IIRC, this broke Chrome recently because folks
>> were
>> > doing this in C++.  Hans, do you remember the details?
>>
>> Hmm, it doesn't ring a bell, but my memory sometimes fails me. Didn't
>> you and Reid look at something like this the other day? (But maybe
>> that was in internal code?)
>>
>
> Ah, I think you are right... Reid, do you happen to recall?
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits