Re: Safe transposition of logical and operands

[Paul Floyd via Gcc]

On 17-09-23 22:51, Jonathan Wakely wrote:



Why would it be trapping? It's loading an int64_t, which might be 
uninitialised but it can't trap.


In this context I think that Valgrind is considering that any memory 
load could trap.


*f on a std::optional is not like dereferencing a pointer, the int64_t 
memory location is always present as part of the object.


For this

movq40(%rbx), %rax

unless you know that what RBX+40 is pointing to is safe to dereference 
it's not different to dereferencing a pointer.


So I think that the problem is that Valgrind is being cautious and not 
allowing any loads but GCC is accepting what it considers safe loads 
from the stack.


A+
Paul

How to enable DCE during late_ipa_passes?

[Hanke Zhang via Gcc]

Hi, I am currently developing a new pass in the late_ipa_passes phase,
located behind the pass_ipa_pta and in front of the
pass_omp_simd_clone. I now hope that after the pass I developed, the
code can be vectorized, that is, the code that can be vectorized
during the pass_omp_simd_clone.

But not yet, I'm guessing that maybe it's because I added some dead
code during the pass I developed, so I want to be followed by a DCE
pass right after my pass, hoping that this will allow
pass_omp_simd_clone to vectorize the code after my pass.

But I've found that adding NEXT_PASS(pass_dce) directly after my pass
results in a runtime error, so I would like to ask if there is any
good way to get pass_dce to work in the late_ipa_passes phase?

Hanke Zhang.

Re: Safe transposition of logical and operands

[Jonathan Wakely via Gcc]

On Mon, 18 Sept 2023, 08:03 Paul Floyd via Gcc,  wrote:

>
>
> On 17-09-23 22:51, Jonathan Wakely wrote:
>
> >
> > Why would it be trapping? It's loading an int64_t, which might be
> > uninitialised but it can't trap.
>
> In this context I think that Valgrind is considering that any memory
> load could trap.
>

There are no padding bits in int64_t and all object representations are
valid values, so it has no trap representations.



> > *f on a std::optional is not like dereferencing a pointer, the int64_t
> > memory location is always present as part of the object.
>
> For this
>
> movq40(%rbx), %rax
>
> unless you know that what RBX+40 is pointing to is safe to dereference
> it's not different to dereferencing a pointer.
>

If it isn't safe to load that integer then it isn't safe to call f.operator
bool() either. GCC knows they are part of the same object.


> So I think that the problem is that Valgrind is being cautious and not
> allowing any loads but GCC is accepting what it considers safe loads
> from the stack.
>

Yes, GCC assumes that the reference is bound to a valid object, because C++
requires that to be true. Of course memcheck can't assume that, because one
of its main reasons to exist is to find undefined behaviour where that
isn't true!

I think what GCC is doing is a valid transformation, in the context of a
valid C++ program. But I'm not sure that helps valgrind, which doesn't have
the liberty of assuming a valid program.

Re: Question on -fwrapv and -fwrapv-pointer

[Richard Biener via Gcc]

On Sat, Sep 16, 2023 at 10:38 AM Martin Uecker via Gcc  wrote:
>
>
>
> (moved to gcc@)
>
> > On Fri, Sep 15, 2023 at 08:18:28AM -0700, Andrew Pinski wrote:
> > > On Fri, Sep 15, 2023 at 8:12 AM Qing Zhao  wrote:
> > > >
> > > >
> > > >
> > > > > On Sep 15, 2023, at 3:43 AM, Xi Ruoyao  wrote:
> > > > >
> > > > > On Thu, 2023-09-14 at 21:41 +, Qing Zhao wrote:
> > > >  CLANG already provided -fsanitize=unsigned-integer-overflow. GCC
> > > >  might need to do the same.
> > > > >>>
> > > > >>> NO. There is no such thing as unsigned integer overflow. That option
> > > > >>> is badly designed and the GCC community has rejected a few times now
> > > > >>> having that sanitizer before. It is bad form to have a sanitizer for
> > > > >>> well defined code.
> > > > >>
> > > > >> Even though unsigned integer overflow is well defined, it might be
> > > > >> unintentional, shall we warn user about this?
> > > > >
> > > > > *Everything* could be unintentional and should be warned then.  GCC 
> > > > > is a
> > > > > compiler, not an advanced AI educating the programmers.
> > > >
> > > > Well, you are right in some sense. -:)
> > > >
> > > > However, overflow is one important source for security flaws, it’s 
> > > > important  for compilers to detect
> > > > overflows in the programs in general.
> > >
> > > Except it is NOT an overflow. Rather it is wrapping. That is a big
> > > point here. unsigned wraps and does NOT overflow. Yes there is a major
> > > difference.
> >
> > Right, yes. I will try to pick my language very carefully. :)
> >
> > The practical problem I am trying to solve in the 30 million lines of
> > Linux kernel code is that of catching arithmetic wrap-around. The
> > problem is one of evolving the code -- I can't just drop -fwrapv and
> > -fwrapv-pointer because it's not possible to fix all the cases at once.
> > (And we really don't want to reintroduce undefined behavior.)
> >
> > So, for signed, pointer, and unsigned types, we need:
> >
> > a) No arithmetic UB -- everything needs to have deterministic behavior.
> >The current solution here is "-fno-strict-overflow", which eliminates
> >the UB and makes sure everything wraps.
> >
> > b) A way to run-time warn/trap on overflow/underflow/wrap-around. This
> >would work with -fsanitize=[signed-integer|pointer]-overflow except
> >due to "a)" we always wrap. And there isn't currently coverage like
> >this for unsigned (in GCC).
> >
> > Our problem is that the kernel is filled with a mix of places where there
> > is intended wrap-around and unintended wrap-around. We can chip away at
> > fixing the intended wrap-around that we can find with static analyzers,
> > etc, but at the end of the day there is a long tail of finding the places
> > where intended wrap-around is hiding. But when the refactoring is
> > sufficiently completely, we can move the wrap-around warning to a trap,
> > and the kernel will not longer have this class of security flaw.
> >
> > As a real-world example, here is a bug where a u8 wraps around causing
> > an under-allocation that allowed for a heap overwrite:
> >
> > https://git.kernel.org/linus/6311071a0562
> > https://elixir.bootlin.com/linux/v6.5/source/net/wireless/nl80211.c#L5422
> >
> > If there were more than 255 elements in a linked list, the allocation
> > would be too small, and the second loop would write past the end of the
> > allocation. This is a pretty classic allocation underflow and linear
> > heap write overflow security flaw. (And it would be trivially stopped by
> > trapping on the u8 wrap around.)
> >
> > So, I want to be able to catch that at run-time. But we also have code
> > doing things like "if (ulong + offset < ulong) { ... }":
> >
> > https://elixir.bootlin.com/linux/v6.5/source/drivers/crypto/axis/artpec6_crypto.c#L1187
> >
> > This is easy for a static analyzer to find and we can replace it with a
> > non-wrapping test (e.g. __builtin_add_overflow()), but we'll not find
> > them all immediately, especially for the signed and pointer cases.
> >
> > So, I need to retain the "everything wraps" behavior while still being
> > able to detect when it happens.
>
>
> Hi Kees,
>
> I have a couple of questions:
>
> Currently, my thinking was that you would use signed integers
> if you want the usual integer arithmetic rules we know from
> elementary school and if you overflow this is clearly a bug
> you can diagnose with UBsan.
>
> There are people who think that signed overflow should be
> defined to wrap, but I think this would be a severe
> mistake because then code would start to rely on it, which
> makes it then difficult to differentiate between bugs and
> intended uses (e.g. the unfortunate situation you have
> with the kernel).
>
> I assume you want to combine UBSan plus wrapping for
> production use?  Or only for testing?   Or in other words:
> why would testing UBSan and production with wrapping
> not be sufficient to find and fix all bugs?
>
> Wrapping would not be correct be

Re: Concerns regarding the -ffp-contract=fast default

[Alexander Monakov]

Hi Florian,

On Thu, 14 Sep 2023, Alexander Monakov wrote:

> 
> On Thu, 14 Sep 2023, Florian Weimer via Gcc wrote:
> 
> > While rebuilding CentOS Stream with -march=x86-64-v3, I rediscovered
> > several packages had test suite failures because x86-64 suddenly gained
> > FMA support.  I say “rediscovered” because these issues were already
> > visible on other architectures with FMA.
> > 
> > So far, our package/architecture maintainers had just disabled test
> > suites or had built the package with -fp-contract=off because the
> > failures did not reproduce on x86-64.  I'm not sure if this is the right
> > course of action.
> > 
> > GCC contraction behavior is rather inconsistent.  It does not contract x
> > + x - x without -ffast-math, for example, although I believe it would be
> > permissible under the rules that enable FMA contraction.  This whole
> > thing looks suspiciously like a quick hack to get a performance
> > improvement from FMA instructions (sorry).
> > 
> > I know that GCC 14 has -fp-contract=standard.  Would it make sense to
> > switch the default to that?  If it fixes those package test suites, it
> > probably has an observable performance impact. 8-/
> 
> Note that with =standard FMA contraction is still allowed within an
> expression: the compiler will transform 'x * y + z' to 'fma(x, y, z)'.
> The difference between =fast and =standard is contraction across
> statement boundaries. So I'd expect some test suite failures you speak of
> to remain with =standard as opposed to =off.
> 
> I think it's better to switch both C and C++ defaults to =standard,
> matching Clang, but it needs a bit of leg work to avoid regressing
> our own testsuite for targets that have FMA in the base ISA.
> 
> (personally I'd be on board with switching to =off even)
> 
> See https://gcc.gnu.org/PR106902 for a worked example where -ffp-contract=fast
> caused a correctness issue in a widely used FOSS image processing application
> that was quite hard to debug.
> 
> Obviously -Ofast and -ffast-math will still imply -ffp-contract=fast if we
> make the change, so SPEC scores won't be affected.

Is this the sort of information you were looking for?

If you're joining the Cauldron and could poll people about changing the default,
I feel that could be helpful.

One of the tricky aspects is what to do under -std=cNN, which implies
-ffp-contract=off; "upgrading" it to =standard would introduce FMAs.

Also, I'm a bit unsure what you were implying here:

> I know that GCC 14 has -fp-contract=standard.  Would it make sense to
> switch the default to that?  If it fixes those package test suites, it
> probably has an observable performance impact. 8-/

The "correctness trumps performance" principle still applies, and
-ffp-contract=fast (the current default outside of -std=cNN) is
known to cause correctness issues and violates the C language standard.
And -ffast[-and-loose]-math for is not going away.

Thanks.
Alexander

Re: How to enable DCE during late_ipa_passes?

[Richard Biener via Gcc]

On Mon, Sep 18, 2023 at 9:17 AM Hanke Zhang via Gcc  wrote:
>
> Hi, I am currently developing a new pass in the late_ipa_passes phase,
> located behind the pass_ipa_pta and in front of the
> pass_omp_simd_clone. I now hope that after the pass I developed, the
> code can be vectorized, that is, the code that can be vectorized
> during the pass_omp_simd_clone.
>
> But not yet, I'm guessing that maybe it's because I added some dead
> code during the pass I developed, so I want to be followed by a DCE
> pass right after my pass, hoping that this will allow
> pass_omp_simd_clone to vectorize the code after my pass.
>
> But I've found that adding NEXT_PASS(pass_dce) directly after my pass
> results in a runtime error, so I would like to ask if there is any
> good way to get pass_dce to work in the late_ipa_passes phase?

You could try doing

  NEXT_PASS (pass_your_ipa_pass)
  PUSH_INSERT_PASSES_WITHIN (pass_your_ipa_pass)
NEXT_PASS (pass_dce);
  POP_INSERT_PASES ()
  NEXT_PASS (pass_omp_simd_clone)


> Hanke Zhang.

Re: Safe transposition of logical and operands

[Richard Biener via Gcc]

On Mon, Sep 18, 2023 at 9:24 AM Jonathan Wakely via Gcc  wrote:
>
> On Mon, 18 Sept 2023, 08:03 Paul Floyd via Gcc,  wrote:
>
> >
> >
> > On 17-09-23 22:51, Jonathan Wakely wrote:
> >
> > >
> > > Why would it be trapping? It's loading an int64_t, which might be
> > > uninitialised but it can't trap.
> >
> > In this context I think that Valgrind is considering that any memory
> > load could trap.
> >
>
> There are no padding bits in int64_t and all object representations are
> valid values, so it has no trap representations.
>
>
>
> > > *f on a std::optional is not like dereferencing a pointer, the int64_t
> > > memory location is always present as part of the object.
> >
> > For this
> >
> > movq40(%rbx), %rax
> >
> > unless you know that what RBX+40 is pointing to is safe to dereference
> > it's not different to dereferencing a pointer.
> >
>
> If it isn't safe to load that integer then it isn't safe to call f.operator
> bool() either. GCC knows they are part of the same object.
>
>
> > So I think that the problem is that Valgrind is being cautious and not
> > allowing any loads but GCC is accepting what it considers safe loads
> > from the stack.
> >
>
> Yes, GCC assumes that the reference is bound to a valid object, because C++
> requires that to be true. Of course memcheck can't assume that, because one
> of its main reasons to exist is to find undefined behaviour where that
> isn't true!
>
> I think what GCC is doing is a valid transformation, in the context of a
> valid C++ program. But I'm not sure that helps valgrind, which doesn't have
> the liberty of assuming a valid program.

More specifically GCC thinks it's fine to speculate loads (given it can prove
doing so doesn't trap)

Richard.

Re: Concerns regarding the -ffp-contract=fast default

[Richard Biener via Gcc]

On Mon, Sep 18, 2023 at 9:51 AM Alexander Monakov  wrote:
>
> Hi Florian,
>
> On Thu, 14 Sep 2023, Alexander Monakov wrote:
>
> >
> > On Thu, 14 Sep 2023, Florian Weimer via Gcc wrote:
> >
> > > While rebuilding CentOS Stream with -march=x86-64-v3, I rediscovered
> > > several packages had test suite failures because x86-64 suddenly gained
> > > FMA support.  I say “rediscovered” because these issues were already
> > > visible on other architectures with FMA.
> > >
> > > So far, our package/architecture maintainers had just disabled test
> > > suites or had built the package with -fp-contract=off because the
> > > failures did not reproduce on x86-64.  I'm not sure if this is the right
> > > course of action.
> > >
> > > GCC contraction behavior is rather inconsistent.  It does not contract x
> > > + x - x without -ffast-math, for example, although I believe it would be
> > > permissible under the rules that enable FMA contraction.  This whole

Is that really just x + x - x?  We currently gate simplifying x - x to zero
on no-signed-zeros and round-to-nearest and have no special
handling for x + x - x.

> > > thing looks suspiciously like a quick hack to get a performance
> > > improvement from FMA instructions (sorry).
> > >
> > > I know that GCC 14 has -fp-contract=standard.  Would it make sense to
> > > switch the default to that?  If it fixes those package test suites, it
> > > probably has an observable performance impact. 8-/
> >
> > Note that with =standard FMA contraction is still allowed within an
> > expression: the compiler will transform 'x * y + z' to 'fma(x, y, z)'.
> > The difference between =fast and =standard is contraction across
> > statement boundaries. So I'd expect some test suite failures you speak of
> > to remain with =standard as opposed to =off.
> >
> > I think it's better to switch both C and C++ defaults to =standard,
> > matching Clang, but it needs a bit of leg work to avoid regressing
> > our own testsuite for targets that have FMA in the base ISA.
> >
> > (personally I'd be on board with switching to =off even)
> >
> > See https://gcc.gnu.org/PR106902 for a worked example where 
> > -ffp-contract=fast
> > caused a correctness issue in a widely used FOSS image processing 
> > application
> > that was quite hard to debug.
> >
> > Obviously -Ofast and -ffast-math will still imply -ffp-contract=fast if we
> > make the change, so SPEC scores won't be affected.
>
> Is this the sort of information you were looking for?
>
> If you're joining the Cauldron and could poll people about changing the 
> default,
> I feel that could be helpful.
>
> One of the tricky aspects is what to do under -std=cNN, which implies
> -ffp-contract=off; "upgrading" it to =standard would introduce FMAs.
>
> Also, I'm a bit unsure what you were implying here:
>
> > I know that GCC 14 has -fp-contract=standard.  Would it make sense to
> > switch the default to that?  If it fixes those package test suites, it
> > probably has an observable performance impact. 8-/
>
> The "correctness trumps performance" principle still applies, and
> -ffp-contract=fast (the current default outside of -std=cNN) is
> known to cause correctness issues and violates the C language standard.
> And -ffast[-and-loose]-math for is not going away.

I think that changing the default to =standard without -ffast-math is
reasonable.
IIRC the standard allows such default if it's indicated, so it doesn't require
=off anywhere.

Richard.

>
> Thanks.
> Alexander

Re: How to enable DCE during late_ipa_passes?

[Hanke Zhang via Gcc]

Thanks! That works!

Richard Biener  于2023年9月18日周一 15:56写道：
>
> On Mon, Sep 18, 2023 at 9:17 AM Hanke Zhang via Gcc  wrote:
> >
> > Hi, I am currently developing a new pass in the late_ipa_passes phase,
> > located behind the pass_ipa_pta and in front of the
> > pass_omp_simd_clone. I now hope that after the pass I developed, the
> > code can be vectorized, that is, the code that can be vectorized
> > during the pass_omp_simd_clone.
> >
> > But not yet, I'm guessing that maybe it's because I added some dead
> > code during the pass I developed, so I want to be followed by a DCE
> > pass right after my pass, hoping that this will allow
> > pass_omp_simd_clone to vectorize the code after my pass.
> >
> > But I've found that adding NEXT_PASS(pass_dce) directly after my pass
> > results in a runtime error, so I would like to ask if there is any
> > good way to get pass_dce to work in the late_ipa_passes phase?
>
> You could try doing
>
>   NEXT_PASS (pass_your_ipa_pass)
>   PUSH_INSERT_PASSES_WITHIN (pass_your_ipa_pass)
> NEXT_PASS (pass_dce);
>   POP_INSERT_PASES ()
>   NEXT_PASS (pass_omp_simd_clone)
>
>
> > Hanke Zhang.

Re: Question on -fwrapv and -fwrapv-pointer

[Martin Uecker via Gcc]

Am Montag, dem 18.09.2023 um 09:31 +0200 schrieb Richard Biener via Gcc:
> On Sat, Sep 16, 2023 at 10:38 AM Martin Uecker via Gcc  
> wrote:
> > 
> > 
> > 
> > (moved to gcc@)
> > 
> > > On Fri, Sep 15, 2023 at 08:18:28AM -0700, Andrew Pinski wrote:
> > > > On Fri, Sep 15, 2023 at 8:12 AM Qing Zhao  wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > > On Sep 15, 2023, at 3:43 AM, Xi Ruoyao  wrote:
> > > > > > 
> > > > > > On Thu, 2023-09-14 at 21:41 +, Qing Zhao wrote:
> > > > > > > > > CLANG already provided -fsanitize=unsigned-integer-overflow. 
> > > > > > > > > GCC
> > > > > > > > > might need to do the same.
> > > > > > > > 
> > > > > > > > NO. There is no such thing as unsigned integer overflow. That 
> > > > > > > > option
> > > > > > > > is badly designed and the GCC community has rejected a few 
> > > > > > > > times now
> > > > > > > > having that sanitizer before. It is bad form to have a 
> > > > > > > > sanitizer for
> > > > > > > > well defined code.
> > > > > > > 
> > > > > > > Even though unsigned integer overflow is well defined, it might be
> > > > > > > unintentional, shall we warn user about this?
> > > > > > 
> > > > > > *Everything* could be unintentional and should be warned then.  GCC 
> > > > > > is a
> > > > > > compiler, not an advanced AI educating the programmers.
> > > > > 
> > > > > Well, you are right in some sense. -:)
> > > > > 
> > > > > However, overflow is one important source for security flaws, it’s 
> > > > > important  for compilers to detect
> > > > > overflows in the programs in general.
> > > > 
> > > > Except it is NOT an overflow. Rather it is wrapping. That is a big
> > > > point here. unsigned wraps and does NOT overflow. Yes there is a major
> > > > difference.
> > > 
> > > Right, yes. I will try to pick my language very carefully. :)
> > > 
> > > The practical problem I am trying to solve in the 30 million lines of
> > > Linux kernel code is that of catching arithmetic wrap-around. The
> > > problem is one of evolving the code -- I can't just drop -fwrapv and
> > > -fwrapv-pointer because it's not possible to fix all the cases at once.
> > > (And we really don't want to reintroduce undefined behavior.)
> > > 
> > > So, for signed, pointer, and unsigned types, we need:
> > > 
> > > a) No arithmetic UB -- everything needs to have deterministic behavior.
> > >The current solution here is "-fno-strict-overflow", which eliminates
> > >the UB and makes sure everything wraps.
> > > 
> > > b) A way to run-time warn/trap on overflow/underflow/wrap-around. This
> > >would work with -fsanitize=[signed-integer|pointer]-overflow except
> > >due to "a)" we always wrap. And there isn't currently coverage like
> > >this for unsigned (in GCC).
> > > 
> > > Our problem is that the kernel is filled with a mix of places where there
> > > is intended wrap-around and unintended wrap-around. We can chip away at
> > > fixing the intended wrap-around that we can find with static analyzers,
> > > etc, but at the end of the day there is a long tail of finding the places
> > > where intended wrap-around is hiding. But when the refactoring is
> > > sufficiently completely, we can move the wrap-around warning to a trap,
> > > and the kernel will not longer have this class of security flaw.
> > > 
> > > As a real-world example, here is a bug where a u8 wraps around causing
> > > an under-allocation that allowed for a heap overwrite:
> > > 
> > > https://git.kernel.org/linus/6311071a0562
> > > https://elixir.bootlin.com/linux/v6.5/source/net/wireless/nl80211.c#L5422
> > > 
> > > If there were more than 255 elements in a linked list, the allocation
> > > would be too small, and the second loop would write past the end of the
> > > allocation. This is a pretty classic allocation underflow and linear
> > > heap write overflow security flaw. (And it would be trivially stopped by
> > > trapping on the u8 wrap around.)
> > > 
> > > So, I want to be able to catch that at run-time. But we also have code
> > > doing things like "if (ulong + offset < ulong) { ... }":
> > > 
> > > https://elixir.bootlin.com/linux/v6.5/source/drivers/crypto/axis/artpec6_crypto.c#L1187
> > > 
> > > This is easy for a static analyzer to find and we can replace it with a
> > > non-wrapping test (e.g. __builtin_add_overflow()), but we'll not find
> > > them all immediately, especially for the signed and pointer cases.
> > > 
> > > So, I need to retain the "everything wraps" behavior while still being
> > > able to detect when it happens.
> > 
> > 
> > Hi Kees,
> > 
> > I have a couple of questions:
> > 
> > Currently, my thinking was that you would use signed integers
> > if you want the usual integer arithmetic rules we know from
> > elementary school and if you overflow this is clearly a bug
> > you can diagnose with UBsan.
> > 
> > There are people who think that signed overflow should be
> > defined to wrap, but I think this would be a severe
> > mistake because then code would start to rely on

Re: Question on -fwrapv and -fwrapv-pointer

[Richard Biener via Gcc]

On Mon, Sep 18, 2023 at 10:17 AM Martin Uecker  wrote:
>
> Am Montag, dem 18.09.2023 um 09:31 +0200 schrieb Richard Biener via Gcc:
> > On Sat, Sep 16, 2023 at 10:38 AM Martin Uecker via Gcc  
> > wrote:
> > >
> > >
> > >
> > > (moved to gcc@)
> > >
> > > > On Fri, Sep 15, 2023 at 08:18:28AM -0700, Andrew Pinski wrote:
> > > > > On Fri, Sep 15, 2023 at 8:12 AM Qing Zhao  
> > > > > wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > On Sep 15, 2023, at 3:43 AM, Xi Ruoyao  wrote:
> > > > > > >
> > > > > > > On Thu, 2023-09-14 at 21:41 +, Qing Zhao wrote:
> > > > > > > > > > CLANG already provided 
> > > > > > > > > > -fsanitize=unsigned-integer-overflow. GCC
> > > > > > > > > > might need to do the same.
> > > > > > > > >
> > > > > > > > > NO. There is no such thing as unsigned integer overflow. That 
> > > > > > > > > option
> > > > > > > > > is badly designed and the GCC community has rejected a few 
> > > > > > > > > times now
> > > > > > > > > having that sanitizer before. It is bad form to have a 
> > > > > > > > > sanitizer for
> > > > > > > > > well defined code.
> > > > > > > >
> > > > > > > > Even though unsigned integer overflow is well defined, it might 
> > > > > > > > be
> > > > > > > > unintentional, shall we warn user about this?
> > > > > > >
> > > > > > > *Everything* could be unintentional and should be warned then.  
> > > > > > > GCC is a
> > > > > > > compiler, not an advanced AI educating the programmers.
> > > > > >
> > > > > > Well, you are right in some sense. -:)
> > > > > >
> > > > > > However, overflow is one important source for security flaws, it’s 
> > > > > > important  for compilers to detect
> > > > > > overflows in the programs in general.
> > > > >
> > > > > Except it is NOT an overflow. Rather it is wrapping. That is a big
> > > > > point here. unsigned wraps and does NOT overflow. Yes there is a major
> > > > > difference.
> > > >
> > > > Right, yes. I will try to pick my language very carefully. :)
> > > >
> > > > The practical problem I am trying to solve in the 30 million lines of
> > > > Linux kernel code is that of catching arithmetic wrap-around. The
> > > > problem is one of evolving the code -- I can't just drop -fwrapv and
> > > > -fwrapv-pointer because it's not possible to fix all the cases at once.
> > > > (And we really don't want to reintroduce undefined behavior.)
> > > >
> > > > So, for signed, pointer, and unsigned types, we need:
> > > >
> > > > a) No arithmetic UB -- everything needs to have deterministic behavior.
> > > >The current solution here is "-fno-strict-overflow", which eliminates
> > > >the UB and makes sure everything wraps.
> > > >
> > > > b) A way to run-time warn/trap on overflow/underflow/wrap-around. This
> > > >would work with -fsanitize=[signed-integer|pointer]-overflow except
> > > >due to "a)" we always wrap. And there isn't currently coverage like
> > > >this for unsigned (in GCC).
> > > >
> > > > Our problem is that the kernel is filled with a mix of places where 
> > > > there
> > > > is intended wrap-around and unintended wrap-around. We can chip away at
> > > > fixing the intended wrap-around that we can find with static analyzers,
> > > > etc, but at the end of the day there is a long tail of finding the 
> > > > places
> > > > where intended wrap-around is hiding. But when the refactoring is
> > > > sufficiently completely, we can move the wrap-around warning to a trap,
> > > > and the kernel will not longer have this class of security flaw.
> > > >
> > > > As a real-world example, here is a bug where a u8 wraps around causing
> > > > an under-allocation that allowed for a heap overwrite:
> > > >
> > > > https://git.kernel.org/linus/6311071a0562
> > > > https://elixir.bootlin.com/linux/v6.5/source/net/wireless/nl80211.c#L5422
> > > >
> > > > If there were more than 255 elements in a linked list, the allocation
> > > > would be too small, and the second loop would write past the end of the
> > > > allocation. This is a pretty classic allocation underflow and linear
> > > > heap write overflow security flaw. (And it would be trivially stopped by
> > > > trapping on the u8 wrap around.)
> > > >
> > > > So, I want to be able to catch that at run-time. But we also have code
> > > > doing things like "if (ulong + offset < ulong) { ... }":
> > > >
> > > > https://elixir.bootlin.com/linux/v6.5/source/drivers/crypto/axis/artpec6_crypto.c#L1187
> > > >
> > > > This is easy for a static analyzer to find and we can replace it with a
> > > > non-wrapping test (e.g. __builtin_add_overflow()), but we'll not find
> > > > them all immediately, especially for the signed and pointer cases.
> > > >
> > > > So, I need to retain the "everything wraps" behavior while still being
> > > > able to detect when it happens.
> > >
> > >
> > > Hi Kees,
> > >
> > > I have a couple of questions:
> > >
> > > Currently, my thinking was that you would use signed integers
> > > if you want the usual integer arithmetic rules we know

Re: Safe transposition of logical and operands

[Jonathan Wakely via Gcc]

On Mon, 18 Sept 2023 at 08:23, Jonathan Wakely  wrote:
>
>
>
> On Mon, 18 Sept 2023, 08:03 Paul Floyd via Gcc,  wrote:
>>
>>
>>
>> On 17-09-23 22:51, Jonathan Wakely wrote:
>>
>> >
>> > Why would it be trapping? It's loading an int64_t, which might be
>> > uninitialised but it can't trap.
>>
>> In this context I think that Valgrind is considering that any memory
>> load could trap.
>
>
> There are no padding bits in int64_t and all object representations are valid 
> values, so it has no trap representations.
>
>
>>
>> > *f on a std::optional is not like dereferencing a pointer, the int64_t
>> > memory location is always present as part of the object.
>>
>> For this
>>
>> movq40(%rbx), %rax
>>
>> unless you know that what RBX+40 is pointing to is safe to dereference
>> it's not different to dereferencing a pointer.
>
>
> If it isn't safe to load that integer then it isn't safe to call f.operator 
> bool() either. GCC knows they are part of the same object.
>
>>
>> So I think that the problem is that Valgrind is being cautious and not
>> allowing any loads but GCC is accepting what it considers safe loads
>> from the stack.
>
>
> Yes, GCC assumes that the reference is bound to a valid object,

Sorry, replying to email too early in the morning. The parameter isn't
optional& it's just optional so there's no
reference, it's on the stack (as you said) and the compiler can assume
that if the function was called then the stack address is valid.

> because C++ requires that to be true. Of course memcheck can't assume that, 
> because one of its main reasons to exist is to find undefined behaviour where 
> that isn't true!
>
> I think what GCC is doing is a valid transformation, in the context of a 
> valid C++ program. But I'm not sure that helps valgrind, which doesn't have 
> the liberty of assuming a valid program.
>
>
>

Re: Concerns regarding the -ffp-contract=fast default

[Florian Weimer via Gcc]

* Richard Biener:

>> > > GCC contraction behavior is rather inconsistent.  It does not contract x
>> > > + x - x without -ffast-math, for example, although I believe it would be
>> > > permissible under the rules that enable FMA contraction.  This whole
>
> Is that really just x + x - x?  We currently gate simplifying x - x to zero
> on no-signed-zeros and round-to-nearest and have no special
> handling for x + x - x.

It's just an example I made up, and checked that GCC wouldn't optimize
it by default (without -ffast-math et al.).  I believe the rule that GCC
invokes to introduce FMA under -ffp-contract=standard (and
-ffp-contract=fast) would apply to these contractions as well.

Or put differently, -ffp-contract= seems to be exclusively about FMA,
while the option name and description is more generic.

x - x is different because replacing it with 0 doesn't seem to be a
valid contraction because it's incorrect for NaNs.  x + x - x seems to
be different in this regard, but in our implementation, there might be a
quirk about sNaNs and qNaNs.

>> The "correctness trumps performance" principle still applies, and
>> -ffp-contract=fast (the current default outside of -std=cNN) is
>> known to cause correctness issues and violates the C language standard.
>> And -ffast[-and-loose]-math for is not going away.
>
> I think that changing the default to =standard without -ffast-math is
> reasonable.  IIRC the standard allows such default if it's indicated,
> so it doesn't require =off anywhere.

How much numerical code is compatible with that?  For example, David
Goldberg's What Every Computer Scientist Should Know About
Floating-Point Arithmetic (revised) contains this sentence:

| A language definition that does not require parentheses to be honored
| is useless for floating-point calculations.



Few people have access to the IEEE 754 standard, and Goldberg's article
is therefore widely quoted as gospel.  If I understand this correctly,
according to Goldberg, contractions make C “useless”.  But I'm not a
floating point person, and I nowadays regret that I looked down upon the
numerical methods people at university.

Thanks,
Florian

Re: Safe transposition of logical and operands

[Andreas Schwab via Gcc]

On Sep 17 2023, Paul Floyd via Gcc wrote:

> However, Valgrind thinks that these transpositions can't be done when
> there are potentially trapping memory loads like *f.

*f is not doing any pointer dereference.  It just reads the _M_payload
member of f.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Concerns regarding the -ffp-contract=fast default

[Richard Biener via Gcc]

On Mon, Sep 18, 2023 at 12:10 PM Florian Weimer  wrote:
>
> * Richard Biener:
>
> >> > > GCC contraction behavior is rather inconsistent.  It does not contract 
> >> > > x
> >> > > + x - x without -ffast-math, for example, although I believe it would 
> >> > > be
> >> > > permissible under the rules that enable FMA contraction.  This whole
> >
> > Is that really just x + x - x?  We currently gate simplifying x - x to zero
> > on no-signed-zeros and round-to-nearest and have no special
> > handling for x + x - x.
>
> It's just an example I made up, and checked that GCC wouldn't optimize
> it by default (without -ffast-math et al.).  I believe the rule that GCC
> invokes to introduce FMA under -ffp-contract=standard (and
> -ffp-contract=fast) would apply to these contractions as well.
>
> Or put differently, -ffp-contract= seems to be exclusively about FMA,
> while the option name and description is more generic.
>
> x - x is different because replacing it with 0 doesn't seem to be a
> valid contraction because it's incorrect for NaNs.  x + x - x seems to
> be different in this regard, but in our implementation, there might be a
> quirk about sNaNs and qNaNs.
>
> >> The "correctness trumps performance" principle still applies, and
> >> -ffp-contract=fast (the current default outside of -std=cNN) is
> >> known to cause correctness issues and violates the C language standard.
> >> And -ffast[-and-loose]-math for is not going away.
> >
> > I think that changing the default to =standard without -ffast-math is
> > reasonable.  IIRC the standard allows such default if it's indicated,
> > so it doesn't require =off anywhere.
>
> How much numerical code is compatible with that?  For example, David
> Goldberg's What Every Computer Scientist Should Know About
> Floating-Point Arithmetic (revised) contains this sentence:
>
> | A language definition that does not require parentheses to be honored
> | is useless for floating-point calculations.
>
> 

I suppose that applies to re-association not honoring parens.  With C
you either have no re-association (fine) or re-association but globally,
at the expense of violating the standard.  FP contraction is _not_
about honoring parens, the 'error' introduced by is subtle.

Fortran for example allows re-association by default, but honors
parens.  GCC can do this just fine.

> Few people have access to the IEEE 754 standard, and Goldberg's article
> is therefore widely quoted as gospel.  If I understand this correctly,
> according to Goldberg, contractions make C “useless”.  But I'm not a
> floating point person, and I nowadays regret that I looked down upon the
> numerical methods people at university.

No, I don't think it says that.  Fortran also allows contraction (but not
across parens).

Richard.

> Thanks,
> Florian
>

Re: Concerns regarding the -ffp-contract=fast default

[Alexander Monakov]

On Mon, 18 Sep 2023, Florian Weimer via Gcc wrote:

> x - x is different because replacing it with 0 doesn't seem to be a
> valid contraction because it's incorrect for NaNs.  x + x - x seems to
> be different in this regard, but in our implementation, there might be a
> quirk about sNaNs and qNaNs.

Sorry, do you mean contracting 'x + x - x' to 'x'? That is not allowed
due to different result and lack of FP exception for infinities.

Contracting 'x + x - x' to fma(x, 2, -x) would be fine.

Alexander

Re: Concerns regarding the -ffp-contract=fast default

[Martin Uecker]

Am Montag, dem 18.09.2023 um 10:06 +0200 schrieb Richard Biener via Gcc:
> On Mon, Sep 18, 2023 at 9:51 AM Alexander Monakov  wrote:
> > 
> > Hi Florian,
> > 
> > On Thu, 14 Sep 2023, Alexander Monakov wrote:
> > 
> > > 
> > > On Thu, 14 Sep 2023, Florian Weimer via Gcc wrote:
> > > 
> > > > While rebuilding CentOS Stream with -march=x86-64-v3, I rediscovered
> > > > several packages had test suite failures because x86-64 suddenly gained
> > > > FMA support.  I say “rediscovered” because these issues were already
> > > > visible on other architectures with FMA.
> > > > 
> > > > So far, our package/architecture maintainers had just disabled test
> > > > suites or had built the package with -fp-contract=off because the
> > > > failures did not reproduce on x86-64.  I'm not sure if this is the right
> > > > course of action.
> > > > 
> > > > GCC contraction behavior is rather inconsistent.  It does not contract x
> > > > + x - x without -ffast-math, for example, although I believe it would be
> > > > permissible under the rules that enable FMA contraction.  This whole
> 
> Is that really just x + x - x?  We currently gate simplifying x - x to zero
> on no-signed-zeros and round-to-nearest and have no special
> handling for x + x - x.
> 
> > > > thing looks suspiciously like a quick hack to get a performance
> > > > improvement from FMA instructions (sorry).
> > > > 
> > > > I know that GCC 14 has -fp-contract=standard.  Would it make sense to
> > > > switch the default to that?  If it fixes those package test suites, it
> > > > probably has an observable performance impact. 8-/
> > > 
> > > Note that with =standard FMA contraction is still allowed within an
> > > expression: the compiler will transform 'x * y + z' to 'fma(x, y, z)'.
> > > The difference between =fast and =standard is contraction across
> > > statement boundaries. So I'd expect some test suite failures you speak of
> > > to remain with =standard as opposed to =off.
> > > 
> > > I think it's better to switch both C and C++ defaults to =standard,
> > > matching Clang, but it needs a bit of leg work to avoid regressing
> > > our own testsuite for targets that have FMA in the base ISA.
> > > 
> > > (personally I'd be on board with switching to =off even)
> > > 
> > > See https://gcc.gnu.org/PR106902 for a worked example where 
> > > -ffp-contract=fast
> > > caused a correctness issue in a widely used FOSS image processing 
> > > application
> > > that was quite hard to debug.
> > > 
> > > Obviously -Ofast and -ffast-math will still imply -ffp-contract=fast if we
> > > make the change, so SPEC scores won't be affected.
> > 
> > Is this the sort of information you were looking for?
> > 
> > If you're joining the Cauldron and could poll people about changing the 
> > default,
> > I feel that could be helpful.
> > 
> > One of the tricky aspects is what to do under -std=cNN, which implies
> > -ffp-contract=off; "upgrading" it to =standard would introduce FMAs.
> > 
> > Also, I'm a bit unsure what you were implying here:
> > 
> > > I know that GCC 14 has -fp-contract=standard.  Would it make sense to
> > > switch the default to that?  If it fixes those package test suites, it
> > > probably has an observable performance impact. 8-/
> > 
> > The "correctness trumps performance" principle still applies, and
> > -ffp-contract=fast (the current default outside of -std=cNN) is
> > known to cause correctness issues and violates the C language standard.
> > And -ffast[-and-loose]-math for is not going away.
> 
> I think that changing the default to =standard without -ffast-math is
> reasonable.
> IIRC the standard allows such default if it's indicated, so it doesn't require
> =off anywhere.

The C standard requires a pragma to turn control it on and off, 
which GCC does not support.  The default is then implementation
defined.

Martin

Re: Concerns regarding the -ffp-contract=fast default

[Jakub Jelinek via Gcc]

On Mon, Sep 18, 2023 at 01:26:54PM +0200, Martin Uecker wrote:
> > I think that changing the default to =standard without -ffast-math is
> > reasonable.
> > IIRC the standard allows such default if it's indicated, so it doesn't 
> > require
> > =off anywhere.
> 
> The C standard requires a pragma to turn control it on and off, 
> which GCC does not support.  The default is then implementation
> defined.

Perhaps we should add some initial hammer approach for the pragma, like
if you ever use the pragma to turn it somewhere off, it is turned off
globally, or ditto per function.  Might be far easier than trying to
make it precise that contraction is allowed in this expression and not in
this subexpression of that etc.  Of course, making it precise is the
ultimate goal.

Jakub

Re: Concerns regarding the -ffp-contract=fast default

[Florian Weimer via Gcc]

* Alexander Monakov:

> On Mon, 18 Sep 2023, Florian Weimer via Gcc wrote:
>
>> x - x is different because replacing it with 0 doesn't seem to be a
>> valid contraction because it's incorrect for NaNs.  x + x - x seems to
>> be different in this regard, but in our implementation, there might be a
>> quirk about sNaNs and qNaNs.
>
> Sorry, do you mean contracting 'x + x - x' to 'x'?

Yes.

> That is not allowed due to different result and lack of FP exception
> for infinities.

FP exceptions do not need to preserved in C11 (it's a recommended
practice in Annex F only, and the main text explicitly allows changes).
But you are right that inf - inf would result in a NaN, so that's a
different result that cannot be explained by infinite intermediate
precision.  Huh, so it's a bad example.  Sorry!

But the contraction would still be valid after an isfinite check
(something that ranger might catch these days), or with with
-ffinite-math-only in general.  Right?

> Contracting 'x + x - x' to fma(x, 2, -x) would be fine.

It still changes the result, doesn't it?

Thanks,
Florian

Re: Concerns regarding the -ffp-contract=fast default

[Florian Weimer via Gcc]

* Richard Biener:

>> How much numerical code is compatible with that?  For example, David
>> Goldberg's What Every Computer Scientist Should Know About
>> Floating-Point Arithmetic (revised) contains this sentence:
>>
>> | A language definition that does not require parentheses to be honored
>> | is useless for floating-point calculations.
>>
>> 
>
> I suppose that applies to re-association not honoring parens.  With C
> you either have no re-association (fine) or re-association but globally,
> at the expense of violating the standard.  FP contraction is _not_
> about honoring parens, the 'error' introduced by is subtle.

But for contracted expressions, + *is* associative.  Maybe this is not
the most relevant part of the article for contractions, though.

> Fortran for example allows re-association by default, but honors
> parens.  GCC can do this just fine.

I think it's still a divergence from the model promoted by the article.
For further problems see “Pitfalls in Computations on Extended-Based
Systems”; that covers the matter of excess precision (which, as far as I
understand, is very relevant to contractions).  It quotes this code
snippet:

| if (1.0 + x .eq. 1.0) then

The C11 standard does not define what “floating expressions” are (as far
as I can see), so it's a bit unclear whether the whole thing can be
contracted (assuming x is not a NaN).

Thanks,
Florian

Re: Concerns regarding the -ffp-contract=fast default

[Alexander Monakov]

On Mon, 18 Sep 2023, Jakub Jelinek wrote:

> Perhaps we should add some initial hammer approach for the pragma, like
> if you ever use the pragma to turn it somewhere off, it is turned off
> globally, or ditto per function.  Might be far easier than trying to
> make it precise that contraction is allowed in this expression and not in
> this subexpression of that etc.  Of course, making it precise is the
> ultimate goal.

When implementing -ffp-contract=standard I looked into implementing the
pragma (in a precise manner) and I didn't get the impression it would be
hard to implement for C. But I was somewhat discouraged by the lack of
front-end maintainers reaction to the patch implementing the =standard,
so I didn't pursue that.

The hardest part would be popping the pragma state when leaving a block,
which didn't seem difficult (at least for C).

Alexander

Re: Concerns regarding the -ffp-contract=fast default

[Alexander Monakov]

On Mon, 18 Sep 2023, Florian Weimer via Gcc wrote:

> But the contraction would still be valid after an isfinite check
> (something that ranger might catch these days), or with with
> -ffinite-math-only in general.  Right?

Nope, still not valid for negative zero ('x + x - x' would yield
positive zero in the default rounding mode).

> > Contracting 'x + x - x' to fma(x, 2, -x) would be fine.
> 
> It still changes the result, doesn't it?

I don't follow. I doesn't change the result for infinities (produces
a NaN). It changes the result when x is so large that 'x + x' is
not representable (exponent would overflow), but that's exactly what
contraction is about?

Alexander

Re: Concerns regarding the -ffp-contract=fast default

[Martin Uecker]

Am Montag, dem 18.09.2023 um 14:43 +0300 schrieb Alexander Monakov:
> On Mon, 18 Sep 2023, Jakub Jelinek wrote:
> 
> > Perhaps we should add some initial hammer approach for the pragma, like
> > if you ever use the pragma to turn it somewhere off, it is turned off
> > globally, or ditto per function.

I think the default should be off - at least for code that does
not contain the pragma anywhere. 

> > Might be far easier than trying to
> > make it precise that contraction is allowed in this expression and not in
> > this subexpression of that etc.  Of course, making it precise is the
> > ultimate goal.
> 
> When implementing -ffp-contract=standard I looked into implementing the
> pragma (in a precise manner) and I didn't get the impression it would be
> hard to implement for C. But I was somewhat discouraged by the lack of
> front-end maintainers reaction to the patch implementing the =standard,
> so I didn't pursue that.
> 
> The hardest part would be popping the pragma state when leaving a block,
> which didn't seem difficult (at least for C).

It is fairly restricted where it can appear, it essentially operates
on the level of compound statements (!= blocks). 

Martin

Re: Safe transposition of logical and operands

[Floyd, Paul via Gcc]

Hi Richard and Jonathan

On 18/09/2023 10:00, Richard Biener wrote:

On Mon, Sep 18, 2023 at 9:24 AM Jonathan Wakely via Gcc  wrote:

Yes, GCC assumes that the reference is bound to a valid object, because C++
requires that to be true. Of course memcheck can't assume that, because one
of its main reasons to exist is to find undefined behaviour where that
isn't true!


It's even worse than that. This transformation is being done in VEX 
(which unfortunately
is also the bit I know the least). Not normally where we'd do 
accessibility checks.



I think what GCC is doing is a valid transformation, in the context of a
valid C++ program. But I'm not sure that helps valgrind, which doesn't have
the liberty of assuming a valid program.

More specifically GCC thinks it's fine to speculate loads (given it can prove
doing so doesn't trap)


I don't think that will be easy for us to prove. We just don't know 
enough about stack variables.


A+

Paui

Re: Safe transposition of logical and operands

[Richard Biener via Gcc]

On Mon, Sep 18, 2023 at 4:49 PM Floyd, Paul via Gcc  wrote:
>
> Hi Richard and Jonathan
>
> On 18/09/2023 10:00, Richard Biener wrote:
> > On Mon, Sep 18, 2023 at 9:24 AM Jonathan Wakely via Gcc  
> > wrote:
> >> Yes, GCC assumes that the reference is bound to a valid object, because C++
> >> requires that to be true. Of course memcheck can't assume that, because one
> >> of its main reasons to exist is to find undefined behaviour where that
> >> isn't true!
>
> It's even worse than that. This transformation is being done in VEX
> (which unfortunately
> is also the bit I know the least). Not normally where we'd do
> accessibility checks.
>
> >> I think what GCC is doing is a valid transformation, in the context of a
> >> valid C++ program. But I'm not sure that helps valgrind, which doesn't have
> >> the liberty of assuming a valid program.
> > More specifically GCC thinks it's fine to speculate loads (given it can 
> > prove
> > doing so doesn't trap)
>
> I don't think that will be easy for us to prove. We just don't know
> enough about stack variables.

What you could do is report the access only on the point of use of
the accessed value?  (and disregard when the register holding the
value is re-used)

Richard.

> A+
>
> Paui

Re: Question on -fwrapv and -fwrapv-pointer

[Martin Uecker via Gcc]

Am Montag, dem 18.09.2023 um 10:47 +0200 schrieb Richard Biener via Gcc:
> On Mon, Sep 18, 2023 at 10:17 AM Martin Uecker  wrote:
> > 
> > Am Montag, dem 18.09.2023 um 09:31 +0200 schrieb Richard Biener via Gcc:
> > > On Sat, Sep 16, 2023 at 10:38 AM Martin Uecker via Gcc  
> > > wrote:
> > > > 
> > > > 
> > > > 
> > > > (moved to gcc@)
> > > > 
> > > > > On Fri, Sep 15, 2023 at 08:18:28AM -0700, Andrew Pinski wrote:
> > > > > > On Fri, Sep 15, 2023 at 8:12 AM Qing Zhao  
> > > > > > wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > > On Sep 15, 2023, at 3:43 AM, Xi Ruoyao  
> > > > > > > > wrote:
> > > > > > > > 
> > > > > > > > On Thu, 2023-09-14 at 21:41 +, Qing Zhao wrote:
> > > > > > > > > > > CLANG already provided 
> > > > > > > > > > > -fsanitize=unsigned-integer-overflow. GCC
> > > > > > > > > > > might need to do the same.
> > > > > > > > > > 
> > > > > > > > > > NO. There is no such thing as unsigned integer overflow. 
> > > > > > > > > > That option
> > > > > > > > > > is badly designed and the GCC community has rejected a few 
> > > > > > > > > > times now
> > > > > > > > > > having that sanitizer before. It is bad form to have a 
> > > > > > > > > > sanitizer for
> > > > > > > > > > well defined code.
> > > > > > > > > 
> > > > > > > > > Even though unsigned integer overflow is well defined, it 
> > > > > > > > > might be
> > > > > > > > > unintentional, shall we warn user about this?
> > > > > > > > 
> > > > > > > > *Everything* could be unintentional and should be warned then.  
> > > > > > > > GCC is a
> > > > > > > > compiler, not an advanced AI educating the programmers.
> > > > > > > 
> > > > > > > Well, you are right in some sense. -:)
> > > > > > > 
> > > > > > > However, overflow is one important source for security flaws, 
> > > > > > > it’s important  for compilers to detect
> > > > > > > overflows in the programs in general.
> > > > > > 
> > > > > > Except it is NOT an overflow. Rather it is wrapping. That is a big
> > > > > > point here. unsigned wraps and does NOT overflow. Yes there is a 
> > > > > > major
> > > > > > difference.
> > > > > 
> > > > > Right, yes. I will try to pick my language very carefully. :)
> > > > > 
> > > > > The practical problem I am trying to solve in the 30 million lines of
> > > > > Linux kernel code is that of catching arithmetic wrap-around. The
> > > > > problem is one of evolving the code -- I can't just drop -fwrapv and
> > > > > -fwrapv-pointer because it's not possible to fix all the cases at 
> > > > > once.
> > > > > (And we really don't want to reintroduce undefined behavior.)
> > > > > 
> > > > > So, for signed, pointer, and unsigned types, we need:
> > > > > 
> > > > > a) No arithmetic UB -- everything needs to have deterministic 
> > > > > behavior.
> > > > >The current solution here is "-fno-strict-overflow", which 
> > > > > eliminates
> > > > >the UB and makes sure everything wraps.
> > > > > 
> > > > > b) A way to run-time warn/trap on overflow/underflow/wrap-around. This
> > > > >would work with -fsanitize=[signed-integer|pointer]-overflow except
> > > > >due to "a)" we always wrap. And there isn't currently coverage like
> > > > >this for unsigned (in GCC).
> > > > > 
> > > > > Our problem is that the kernel is filled with a mix of places where 
> > > > > there
> > > > > is intended wrap-around and unintended wrap-around. We can chip away 
> > > > > at
> > > > > fixing the intended wrap-around that we can find with static 
> > > > > analyzers,
> > > > > etc, but at the end of the day there is a long tail of finding the 
> > > > > places
> > > > > where intended wrap-around is hiding. But when the refactoring is
> > > > > sufficiently completely, we can move the wrap-around warning to a 
> > > > > trap,
> > > > > and the kernel will not longer have this class of security flaw.
> > > > > 
> > > > > As a real-world example, here is a bug where a u8 wraps around causing
> > > > > an under-allocation that allowed for a heap overwrite:
> > > > > 
> > > > > https://git.kernel.org/linus/6311071a0562
> > > > > https://elixir.bootlin.com/linux/v6.5/source/net/wireless/nl80211.c#L5422
> > > > > 
> > > > > If there were more than 255 elements in a linked list, the allocation
> > > > > would be too small, and the second loop would write past the end of 
> > > > > the
> > > > > allocation. This is a pretty classic allocation underflow and linear
> > > > > heap write overflow security flaw. (And it would be trivially stopped 
> > > > > by
> > > > > trapping on the u8 wrap around.)
> > > > > 
> > > > > So, I want to be able to catch that at run-time. But we also have code
> > > > > doing things like "if (ulong + offset < ulong) { ... }":
> > > > > 
> > > > > https://elixir.bootlin.com/linux/v6.5/source/drivers/crypto/axis/artpec6_crypto.c#L1187
> > > > > 
> > > > > This is easy for a static analyzer to find and we can replace it with 
> > > > > a
> > > > > non-wrapping test (e.g. __builtin_add_overflow()), b

Re: Question on -fwrapv and -fwrapv-pointer

[Andrew Pinski via Gcc]

On Mon, Sep 18, 2023 at 12:33 AM Richard Biener via Gcc  wrote:
>
> On Sat, Sep 16, 2023 at 10:38 AM Martin Uecker via Gcc  
> wrote:
> >
> >
> >
> > (moved to gcc@)
> >
> > > On Fri, Sep 15, 2023 at 08:18:28AM -0700, Andrew Pinski wrote:
> > > > On Fri, Sep 15, 2023 at 8:12 AM Qing Zhao  wrote:
> > > > >
> > > > >
> > > > >
> > > > > > On Sep 15, 2023, at 3:43 AM, Xi Ruoyao  wrote:
> > > > > >
> > > > > > On Thu, 2023-09-14 at 21:41 +, Qing Zhao wrote:
> > > > >  CLANG already provided -fsanitize=unsigned-integer-overflow. GCC
> > > > >  might need to do the same.
> > > > > >>>
> > > > > >>> NO. There is no such thing as unsigned integer overflow. That 
> > > > > >>> option
> > > > > >>> is badly designed and the GCC community has rejected a few times 
> > > > > >>> now
> > > > > >>> having that sanitizer before. It is bad form to have a sanitizer 
> > > > > >>> for
> > > > > >>> well defined code.
> > > > > >>
> > > > > >> Even though unsigned integer overflow is well defined, it might be
> > > > > >> unintentional, shall we warn user about this?
> > > > > >
> > > > > > *Everything* could be unintentional and should be warned then.  GCC 
> > > > > > is a
> > > > > > compiler, not an advanced AI educating the programmers.
> > > > >
> > > > > Well, you are right in some sense. -:)
> > > > >
> > > > > However, overflow is one important source for security flaws, it’s 
> > > > > important  for compilers to detect
> > > > > overflows in the programs in general.
> > > >
> > > > Except it is NOT an overflow. Rather it is wrapping. That is a big
> > > > point here. unsigned wraps and does NOT overflow. Yes there is a major
> > > > difference.
> > >
> > > Right, yes. I will try to pick my language very carefully. :)
> > >
> > > The practical problem I am trying to solve in the 30 million lines of
> > > Linux kernel code is that of catching arithmetic wrap-around. The
> > > problem is one of evolving the code -- I can't just drop -fwrapv and
> > > -fwrapv-pointer because it's not possible to fix all the cases at once.
> > > (And we really don't want to reintroduce undefined behavior.)
> > >
> > > So, for signed, pointer, and unsigned types, we need:
> > >
> > > a) No arithmetic UB -- everything needs to have deterministic behavior.
> > >The current solution here is "-fno-strict-overflow", which eliminates
> > >the UB and makes sure everything wraps.
> > >
> > > b) A way to run-time warn/trap on overflow/underflow/wrap-around. This
> > >would work with -fsanitize=[signed-integer|pointer]-overflow except
> > >due to "a)" we always wrap. And there isn't currently coverage like
> > >this for unsigned (in GCC).
> > >
> > > Our problem is that the kernel is filled with a mix of places where there
> > > is intended wrap-around and unintended wrap-around. We can chip away at
> > > fixing the intended wrap-around that we can find with static analyzers,
> > > etc, but at the end of the day there is a long tail of finding the places
> > > where intended wrap-around is hiding. But when the refactoring is
> > > sufficiently completely, we can move the wrap-around warning to a trap,
> > > and the kernel will not longer have this class of security flaw.
> > >
> > > As a real-world example, here is a bug where a u8 wraps around causing
> > > an under-allocation that allowed for a heap overwrite:
> > >
> > > https://git.kernel.org/linus/6311071a0562
> > > https://elixir.bootlin.com/linux/v6.5/source/net/wireless/nl80211.c#L5422
> > >
> > > If there were more than 255 elements in a linked list, the allocation
> > > would be too small, and the second loop would write past the end of the
> > > allocation. This is a pretty classic allocation underflow and linear
> > > heap write overflow security flaw. (And it would be trivially stopped by
> > > trapping on the u8 wrap around.)
> > >
> > > So, I want to be able to catch that at run-time. But we also have code
> > > doing things like "if (ulong + offset < ulong) { ... }":
> > >
> > > https://elixir.bootlin.com/linux/v6.5/source/drivers/crypto/axis/artpec6_crypto.c#L1187
> > >
> > > This is easy for a static analyzer to find and we can replace it with a
> > > non-wrapping test (e.g. __builtin_add_overflow()), but we'll not find
> > > them all immediately, especially for the signed and pointer cases.
> > >
> > > So, I need to retain the "everything wraps" behavior while still being
> > > able to detect when it happens.
> >
> >
> > Hi Kees,
> >
> > I have a couple of questions:
> >
> > Currently, my thinking was that you would use signed integers
> > if you want the usual integer arithmetic rules we know from
> > elementary school and if you overflow this is clearly a bug
> > you can diagnose with UBsan.
> >
> > There are people who think that signed overflow should be
> > defined to wrap, but I think this would be a severe
> > mistake because then code would start to rely on it, which
> > makes it then difficult to differentiate between bugs and
> > intend

Re: Concerns regarding the -ffp-contract=fast default

[Alexander Monakov]

On Mon, 18 Sep 2023, Martin Uecker wrote:

> > The hardest part would be popping the pragma state when leaving a block,
> > which didn't seem difficult (at least for C).
> 
> It is fairly restricted where it can appear, it essentially operates
> on the level of compound statements (!= blocks). 

Thanks for the clarification. Let me note that the standard somehow
decided to make out-of-place pragma a (compile-time) UB rather than
a constraint violation, leaving us free to make it work for blocks
(or launch Nethack).

Alexander

Re: Concerns regarding the -ffp-contract=fast default

[Florian Weimer via Gcc]

* Alexander Monakov:

>> > Contracting 'x + x - x' to fma(x, 2, -x) would be fine.
>> 
>> It still changes the result, doesn't it?
>
> I don't follow. I doesn't change the result for infinities (produces
> a NaN). It changes the result when x is so large that 'x + x' is
> not representable (exponent would overflow), but that's exactly what
> contraction is about?

Okay, you meant “changing the result” as in “changing the result in a
permitted way”.  Sorry, was confused.  So this is a bad example all
around.  Are there better ones (that don't involve FMA)?

Thanks,
Florian

Re: Concerns regarding the -ffp-contract=fast default

[Alexander Monakov]

On Mon, 18 Sep 2023, Florian Weimer wrote:

> Okay, you meant “changing the result” as in “changing the result in a
> permitted way”.  Sorry, was confused.  So this is a bad example all
> around.  Are there better ones (that don't involve FMA)?

If you're looking for something similar to your original example, I can
suggest 'x + x + x + x' -> '4 * x' under -frounding-math (i.e. not assuming
default rounding mode) or 'x + x + x + x + x + x' -> '6 * x' for default
rounding.

I don't have a "somewhat practical" example off-hand, since apart from FMA
fused operations are rarely available in instruction sets.

Alexander

Re: Concerns regarding the -ffp-contract=fast default

[Joseph Myers]

On Mon, 18 Sep 2023, Alexander Monakov wrote:

> 
> On Mon, 18 Sep 2023, Florian Weimer wrote:
> 
> > Okay, you meant “changing the result” as in “changing the result in a
> > permitted way”.  Sorry, was confused.  So this is a bad example all
> > around.  Are there better ones (that don't involve FMA)?
> 
> If you're looking for something similar to your original example, I can
> suggest 'x + x + x + x' -> '4 * x' under -frounding-math (i.e. not assuming
> default rounding mode) or 'x + x + x + x + x + x' -> '6 * x' for default
> rounding.
> 
> I don't have a "somewhat practical" example off-hand, since apart from FMA
> fused operations are rarely available in instruction sets.

Apart from FMA, there are the formatOf operations such as fadd (add two 
doubles, rounding the result just once to float).  Fewer instruction sets 
have those than have FMA, however (ia64 did; newer versions of Power 
support such operations as well to some extent, though with limitations 
when underflow or overflow occur).

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: Safe transposition of logical and operands

[Paul Floyd via Gcc]

On 18-09-23 16:55, Richard Biener wrote:


What you could do is report the access only on the point of use of
the accessed value?  (and disregard when the register holding the
value is re-used)


Hi Richard

Not sure that I follow what you're saying.

memcheck triggers errors like this at conditional branching opcodes 
where it determines that the condition is undefined.


There are mechanisms to de-duplicate errors, so once an error is emitted 
it won't be printed again.


However, one of our goals is no false positives. This is one example 
that is slipping through the cracks.


Regards
Paul

Re: Safe transposition of logical and operands

[Martin Uecker]

Am Montag, dem 18.09.2023 um 19:56 +0200 schrieb Paul Floyd via Gcc:
> 
> On 18-09-23 16:55, Richard Biener wrote:
> 
> > What you could do is report the access only on the point of use of
> > the accessed value?  (and disregard when the register holding the
> > value is re-used)
> 
> Hi Richard
> 
> Not sure that I follow what you're saying.
> 
> memcheck triggers errors like this at conditional branching opcodes 
> where it determines that the condition is undefined.
> 
> There are mechanisms to de-duplicate errors, so once an error is emitted 
> it won't be printed again.
> 
> However, one of our goals is no false positives. This is one example 
> that is slipping through the cracks.
> 
I do not understand why memcheck cares about the potential trap when
deciding to do the backwards transformation that combines the two
comparisons?  Can't you just remove this condition?  I assume it
is meant as a filter to only transform cases which really come
from an '&&' condition in the source, but as this example show, this
is too strict. Or am I missing something?


Martin

Re: Safe transposition of logical and operands

[Paul Floyd via Gcc]

On 18-09-23 21:09, Martin Uecker wrote:


I do not understand why memcheck cares about the potential trap when
deciding to do the backwards transformation that combines the two
comparisons?  Can't you just remove this condition?  I assume it
is meant as a filter to only transform cases which really come
from an '&&' condition in the source, but as this example show, this
is too strict. Or am I missing something?


My understanding is that this is a generic transformation of

if (a && b) [which might have been transposed from if (b && a)]

into

if (a & b) [with appropriate extension to the right size].

That means both get evaluated and we can't take that risk that one of 
them traps.


A+
Paul

Re: Safe transposition of logical and operands

[Martin Uecker]

Am Montag, dem 18.09.2023 um 22:15 +0200 schrieb Paul Floyd via Gcc:
> 
> On 18-09-23 21:09, Martin Uecker wrote:
> 
> > I do not understand why memcheck cares about the potential trap when
> > deciding to do the backwards transformation that combines the two
> > comparisons?  Can't you just remove this condition?  I assume it
> > is meant as a filter to only transform cases which really come
> > from an '&&' condition in the source, but as this example show, this
> > is too strict. Or am I missing something?
> 
> My understanding is that this is a generic transformation of
> 
> if (a && b) [which might have been transposed from if (b && a)]
> 
> into
> 
> if (a & b) [with appropriate extension to the right size].
> 
> That means both get evaluated and we can't take that risk that one of 
> them traps.

Is the problem that valgrind transforms the code before it then
emulates it and the problem is that during emulation the code
could trap?

Martin

Re: Safe transposition of logical and operands

[Paul Floyd via Gcc]

On 18-09-23 22:52, Martin Uecker wrote:


Is the problem that valgrind transforms the code before it then
emulates it and the problem is that during emulation the code
could trap?


Yes, roughly the process is
guest ISA -> IR -> IR transformations -> IR optimizations -> execution

The && "idiom recovery" is getting filtered out during the IR 
transformations.


A+
Paul