Re: [cfe-users] Clang9 UBSan and GMP

2019-10-30 Thread David Blaikie via cfe-users
I ran the test & understand it a bit better now - so the abort is part of
the code, when the test fails, the test harness uses abort to fail.

So this isn't "clang causes abort" (it didn't select a bad instruction,
etc) this is "clang causes test failure" - this could be any number of
things in terms of compiler optimizations due to overly dependent code (or
due to miscompiles, to be sure). It's possible the test relies on specific
numeric results that the C++ programming language doesn't guarantee (either
overly high precision, or overly low precision - C++ allows extra precision
in computations which can break numerical code that's relying on certain
precision, for instance).

So, yeah, really hard to say where the fault lies without further
investigation.

On Fri, Oct 25, 2019 at 4:13 PM Hans Åberg  wrote:

> The GMP developers felt it was a compiler bug, so I think I will leave it
> at that. But thanks for the tips.
>
>
> > On 26 Oct 2019, at 00:32, David Blaikie  wrote:
> >
> > UBSan doesn't catch everything - you could also try ASan and/or
> valgrind, etc. (MSan if you want, but that's a bit fussier/more work to use)
>
>
___
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users


Re: [cfe-users] Clang9 UBSan and GMP

2019-10-30 Thread David Blaikie via cfe-users
On Wed, Oct 30, 2019 at 2:29 PM Hans Åberg  wrote:

> Indeed, very hard to figure out. If it is some hidden undefined behavior
> causing it, the UBSan should have caught it, but it does not.


Right - but especially with numerics (especially floating point) there's
loads of room for valid but different behavior between different compilers
- behavior that isn't UB. How much precision a certain mathematical
equation maintains is really at the whim of the optimizers in a lot of ways.


> The link that Matthew gave says that the GMP developers experienced a
> number of such issues with Clang. One can though turn off the optimizer,
> and the tests pass.
>

Sure - most of the numeric effects would only appear with optimizations.
Without them every numeric operation's just done in registers, then written
right back to memory (so no chance of excess precision leaking in by
storing the value in an 80bit floating point register between multiple
operations, or any risk of fused operations that produces extra precision,
etc).

The only way to know is to trace down/reduce the point where the values
diverge & stare at the code to see who's right.

- Dave


>
>
> > On 30 Oct 2019, at 22:17, David Blaikie  wrote:
> >
> > I ran the test & understand it a bit better now - so the abort is part
> of the code, when the test fails, the test harness uses abort to fail.
> >
> > So this isn't "clang causes abort" (it didn't select a bad instruction,
> etc) this is "clang causes test failure" - this could be any number of
> things in terms of compiler optimizations due to overly dependent code (or
> due to miscompiles, to be sure). It's possible the test relies on specific
> numeric results that the C++ programming language doesn't guarantee (either
> overly high precision, or overly low precision - C++ allows extra precision
> in computations which can break numerical code that's relying on certain
> precision, for instance).
> >
> > So, yeah, really hard to say where the fault lies without further
> investigation.
> >
> > On Fri, Oct 25, 2019 at 4:13 PM Hans Åberg  wrote:
> > The GMP developers felt it was a compiler bug, so I think I will leave
> it at that. But thanks for the tips.
> >
> >
> > > On 26 Oct 2019, at 00:32, David Blaikie  wrote:
> > >
> > > UBSan doesn't catch everything - you could also try ASan and/or
> valgrind, etc. (MSan if you want, but that's a bit fussier/more work to use)
> >
>
>
___
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users


Re: [cfe-users] Clang9 UBSan and GMP

2019-10-30 Thread David Blaikie via cfe-users
On Wed, Oct 30, 2019 at 4:25 PM Hans Åberg  wrote:

>
> > On 30 Oct 2019, at 23:50, David Blaikie  wrote:
> >
> >> On Wed, Oct 30, 2019 at 2:29 PM Hans Åberg  wrote:
> >> Indeed, very hard to figure out. If it is some hidden undefined
> behavior causing it, the UBSan should have caught it, but it does not.
> >>
> > Right - but especially with numerics (especially floating point) there's
> loads of room for valid but different behavior between different compilers
> - behavior that isn't UB. How much precision a certain mathematical
> equation maintains is really at the whim of the optimizers in a lot of ways.
>
> I believe that GMP is just using integer types, and then uses that to make
> multiprecision integers, rational numbers, and floating point numbers. At
> least MPFR uses only the integer and rational number part of GMP, and
> builds multiprecision floating point numbers on top of that, which is
> necessary because of special requirements of the standards they adhere to.
>

Ah, fair enough - that narrows down the points of failure a little.

>> The link that Matthew gave says that the GMP developers experienced a
> number of such issues with Clang. One can though turn off the optimizer,
> and the tests pass.
> >
> > Sure - most of the numeric effects would only appear with optimizations.
> Without them every numeric operation's just done in registers, then written
> right back to memory (so no chance of excess precision leaking in by
> storing the value in an 80bit floating point register between multiple
> operations, or any risk of fused operations that produces extra precision,
> etc).
> >
> > The only way to know is to trace down/reduce the point where the values
> diverge & stare at the code to see who's right.
>
> GMP has been used in three years in a sequenced operation that must be
> exact and without errors to solve the problem [1], and I would think it
> used GCC with optimizations. So that puts Clang in a tough spot. :-)
>

Not as much as it would seem - again, the spec allows for a fair bit of
flexibility in a bunch of ways. (admittedly, within integer arithmetic
without invoking UB (but, again, that's not proven - UBSan isn't guaranteed
to catch everything)) Different compilers optimize code in different ways -
that the code "works"/produces the desired behavior on one compiler/under
some optimizations and not others doesn't give us much idea about whether
the code or the compiler is correct. Different behavior is acceptable in
C++ in a bunch of ways & compilers rely on that flexibility.

- Dave


>
> 1. https://gmplib.org/list-archives/gmp-discuss/2019-April/006319.html
>
>
>
___
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users


Re: [cfe-users] Clang9 UBSan and GMP

2019-10-30 Thread David Blaikie via cfe-users
On Wed, Oct 30, 2019 at 5:36 PM Hans Åberg  wrote:

>
>
> > On 31 Oct 2019, at 00:28, David Blaikie  wrote:
> >
> >
> >
> > On Wed, Oct 30, 2019 at 4:25 PM Hans Åberg  wrote:
> >
> >> I believe that GMP is just using integer types, and then uses that to
> make multiprecision integers, rational numbers, and floating point numbers.
> At least MPFR uses only the integer and rational number part of GMP, and
> builds multiprecision floating point numbers on top of that, which is
> necessary because of special requirements of the standards they adhere to.
> >
> > Ah, fair enough - that narrows down the points of failure a little.
>
> In addition, they use assembly code, but that can be turned off with
> configure --disable-assembly, though I did not 'make check' with that.
>
> >> GMP has been used in three years in a sequenced operation that must be
> exact and without errors to solve the problem [1], and I would think it
> used GCC with optimizations. So that puts Clang in a tough spot. :-)
> >
> > Not as much as it would seem - again, the spec allows for a fair bit of
> flexibility in a bunch of ways. (admittedly, within integer arithmetic
> without invoking UB (but, again, that's not proven - UBSan isn't guaranteed
> to catch everything)) Different compilers optimize code in different ways -
> that the code "works"/produces the desired behavior on one compiler/under
> some optimizations and not others doesn't give us much idea about whether
> the code or the compiler is correct. Different behavior is acceptable in
> C++ in a bunch of ways & compilers rely on that flexibility.
>
> Yes, but assuming that the GMP adheres to the C standard, there should be
> no difference in the arithmetical values produced.
>

Not necessarily - C (well, I don't know the C standard as well as I know
the C++ standard, admittedly) does allow various variations (implementation
and unspecified behavior, for instance). eg: order of evaluation of
function arguments (not that this is likely to vary due to optimizations -
and doesn't with clang to the best of my knowledge, but as an example of
the /sort/ of thing):

  int f() {
static int i = 0;
return i++;
  }
  int f2(int i, int j) {
return i;
  }
  int main() {
return f2(f(), f());
  }

This program could exit with 0 or 1 - both results are valid
interpretations of the program per the standard. (again, I don't know the C
spec quite as well, so I'm not sure exactly what it says about this code)

And the assumption that GMP adheres to the C standard isn't one, as a
compiler developer, I'd be willing to make without more data - (heck, even
the compiler itself has some "intentional" undefined behavior in it... )

- Dave
___
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users


Re: [cfe-users] clang-tidy bug?

2019-10-30 Thread David Blaikie via cfe-users
Two separate issues here

1) the fixit hint, as one of a set of alternatives, isn't likely to be
removed/changed - the (albeit quirky) convention of using extra () to
indicate an intentional assignment in a condition has been around for a
while. So if you use the extra parens without writing an assignment, the
compiler's going to suggest you resolve this "conflict" with the style -
either you didn't intend the extra (), or you intended to use assignment.
Those are the two alternative suggestions being made.

2) automatically applying one fixit hint of several ambiguous ones seems
like a bug to me - Aaron - do you know anything about this? Is this
accurate/intended/etc?

On Tue, Oct 29, 2019 at 10:13 AM Robert Ankeney  wrote:

> This was just a example of what I ran into when I used run-clang-tidy.py
> across a compilation database with a -export-fixes=fix.yaml and then ra
>  clang-apply-replacements. Mainly I object to the suggestion+fixit to
> switch to an assignment. Most coding standards would disallow assignments
> in if conditionals. If anything, I would think a suggestion of "if (true
> == isValid)" would be more appropriate.
>
> Thanks for the feedback!
> Robert
>
> On Mon, Oct 28, 2019 at 2:17 PM David Blaikie  wrote:
>
>> clang-tidy in the command line you gave didn't seem to modify the file
>> for me, did it modify the file for you?
>>
>> Are you objecting to the suggestion, or that it was automatically
>> applied? I would think it'd be a bug to apply any fixit/hint if there are
>> multiple possible suggestions.
>>
>> But the existence of the suggestion (without the application of it) to
>> the user seems right to me. The use of extra () to suppress the
>> assignment-in-conditional warning (-Wparentheses) has been around for quite
>> a while, so it's possible that the user intended assignment rather than
>> comparison when they added the extra parentheses.
>>
>> - Dave
>>
>> On Sun, Oct 27, 2019 at 11:32 AM Robert Ankeney via cfe-users <
>> cfe-users@lists.llvm.org> wrote:
>>
>>> For the following code (wrong.cpp):
>>>
>>> bool check(bool isValid)
>>> {
>>> bool retVal = false;
>>>
>>> if (( isValid == true ))
>>> {
>>> retVal = true;
>>> }
>>>
>>> return retVal;
>>> }
>>>
>>> when I run:
>>> clang-tidy -checks=modernize-use-default-member-init wrong.cpp
>>>
>>> I get:
>>> 4 warnings and 1 error generated.
>>> Error while processing /llvm/match/ctBad/wrong.cpp.
>>> /llvm/match/ctBad/wrong.cpp:5:19: error: equality comparison with
>>> extraneous parentheses [clang-diagnostic-parentheses-equality]
>>> if (( isValid == true ))
>>> ~ ^~  ~
>>>   =
>>> /llvm/match/ctBad/wrong.cpp:5:19: note: remove extraneous parentheses
>>> around the comparison to silence this warning
>>> /llvm/match/ctBad/wrong.cpp:5:19: note: use '=' to turn this equality
>>> comparison into an assignment
>>>
>>> Note it turns the if into:
>>> if ( isValid = true )
>>>
>>> Seems like a very bad idea. Removing the redundant parentheses seems
>>> fine, but changing the comparison to an assignment does not. Is this a bug?
>>>
>>> Thanks,
>>> Robert
>>>
>>> ___
>>> cfe-users mailing list
>>> cfe-users@lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
>>>
>>
___
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users