[PATCH] hardened conditionals

[Alexandre Oliva via Gcc-patches]

This patch introduces optional passes to harden conditionals used in
branches, and in computing boolean expressions, by adding redundant
tests of the reversed conditions, and trapping in case of unexpected
results.  Though in abstract machines the redundant tests should never
fail, CPUs may be led to misbehave under certain kinds of attacks,
such as of power deprivation, and these tests reduce the likelihood of
going too far down an unexpected execution path.

This patch was regstrapped on x86_64-linux-gnu.  It was also
bootstrapped along with an extra common.opt that enables both passes
unconditionally.  Ok to install?


for  gcc/ChangeLog

* common.opt (fharden-compares): New.
(fharden-conditional-branches): New.
* doc/invoke.texi: Document new options.
* gimple-harden-conditionals.cc: New.
* passes.def: Add new passes.
* tree-pass.h (make_pass_harden_compares): Declare.
(make_pass_harden_conditional_branches): Declare.

for  gcc/ada/ChangeLog

* doc/gnat_rm/security_hardening_features.rst
(Hardened Conditionals): New.

for  gcc/testsuite/ChangeLog

* c-c++-common/torture/harden-comp.c: New.
* c-c++-common/torture/harden-cond.c: New.
---
 gcc/Makefile.in|1 
 .../doc/gnat_rm/security_hardening_features.rst|   40 ++
 gcc/common.opt |8 
 gcc/doc/invoke.texi|   19 +
 gcc/gimple-harden-conditionals.cc  |  379 
 gcc/passes.def |2 
 gcc/testsuite/c-c++-common/torture/harden-comp.c   |   14 +
 gcc/testsuite/c-c++-common/torture/harden-cond.c   |   18 +
 gcc/tree-pass.h|3 
 9 files changed, 484 insertions(+)
 create mode 100644 gcc/gimple-harden-conditionals.cc
 create mode 100644 gcc/testsuite/c-c++-common/torture/harden-comp.c
 create mode 100644 gcc/testsuite/c-c++-common/torture/harden-cond.c

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 64252697573a7..7209ed117d09d 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1389,6 +1389,7 @@ OBJS = \
gimple-if-to-switch.o \
gimple-iterator.o \
gimple-fold.o \
+   gimple-harden-conditionals.o \
gimple-laddress.o \
gimple-loop-interchange.o \
gimple-loop-jam.o \
diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst 
b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
index 1c46e3a4c7b88..52240d7e3dd54 100644
--- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst
+++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
@@ -87,3 +87,43 @@ types and subtypes, may be silently ignored.  Specifically, 
it is not
 currently recommended to rely on any effects this pragma might be
 expected to have when calling subprograms through access-to-subprogram
 variables.
+
+
+.. Hardened Conditionals:
+
+Hardened Conditionals
+=
+
+GNAT can harden conditionals to protect against control flow attacks.
+
+This is accomplished by two complementary transformations, each
+activated by a separate command-line option.
+
+The option *-fharden-compares* enables hardening of compares that
+compute results stored in variables, adding verification that the
+reversed compare yields the opposite result.
+
+The option *-fharden-conditional-branches* enables hardening of
+compares that guard conditional branches, adding verification of the
+reversed compare to both execution paths.
+
+These transformations are introduced late in the compilation pipeline,
+long after boolean expressions are decomposed into separate compares,
+each one turned into either a conditional branch or a compare whose
+result is stored in a boolean variable or temporary.  Compiler
+optimizations, if enabled, may also turn conditional branches into
+stored compares, and vice-versa.  Conditionals may also be optimized
+out entirely, if their value can be determined at compile time, and
+occasionally multiple compares can be combined into one.
+
+It is thus difficult to predict which of these two options will affect
+a specific compare operation expressed in source code.  Using both
+options ensures that every compare that is not optimized out will be
+hardened.
+
+The addition of reversed compares can be observed by enabling the dump
+files of the corresponding passes, through command line options
+*-fdump-tree-hardcmp* and *-fdump-tree-hardcbr*, respectively.
+
+They are separate options, however, because of the significantly
+different performance impact of the hardening transformations.
diff --git a/gcc/common.opt b/gcc/common.opt
index e867055fc000d..89f2e6da6e56e 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1719,6 +1719,14 @@ fguess-branch-probability
 Common Var(flag_guess_branch_prob) Optimization
 Enable guessing of branch probabilities.
 
+fharden-compares
+Common Var(flag_harden_compares) Optimiza

Re: [PATCH] hardened conditionals

[Richard Biener via Gcc-patches]

On October 9, 2021 5:30:04 AM GMT+02:00, Alexandre Oliva via Gcc-patches 
 wrote:
>
>This patch introduces optional passes to harden conditionals used in
>branches, and in computing boolean expressions, by adding redundant
>tests of the reversed conditions, and trapping in case of unexpected
>results.  Though in abstract machines the redundant tests should never
>fail, CPUs may be led to misbehave under certain kinds of attacks,
>such as of power deprivation, and these tests reduce the likelihood of
>going too far down an unexpected execution path.
>
>This patch was regstrapped on x86_64-linux-gnu.  It was also
>bootstrapped along with an extra common.opt that enables both passes
>unconditionally.  Ok to install?

Why two passes (and two IL traverses?) 

How do you prevent RTL optimizers (jump threading) from removing the redundant 
tests? I'd have expected such hardening to occur very late in the RTL pipeline. 

Richard. 

>
>for  gcc/ChangeLog
>
>   * common.opt (fharden-compares): New.
>   (fharden-conditional-branches): New.
>   * doc/invoke.texi: Document new options.
>   * gimple-harden-conditionals.cc: New.
>   * passes.def: Add new passes.
>   * tree-pass.h (make_pass_harden_compares): Declare.
>   (make_pass_harden_conditional_branches): Declare.
>
>for  gcc/ada/ChangeLog
>
>   * doc/gnat_rm/security_hardening_features.rst
>   (Hardened Conditionals): New.
>
>for  gcc/testsuite/ChangeLog
>
>   * c-c++-common/torture/harden-comp.c: New.
>   * c-c++-common/torture/harden-cond.c: New.
>---
> gcc/Makefile.in|1 
> .../doc/gnat_rm/security_hardening_features.rst|   40 ++
> gcc/common.opt |8 
> gcc/doc/invoke.texi|   19 +
> gcc/gimple-harden-conditionals.cc  |  379 
> gcc/passes.def |2 
> gcc/testsuite/c-c++-common/torture/harden-comp.c   |   14 +
> gcc/testsuite/c-c++-common/torture/harden-cond.c   |   18 +
> gcc/tree-pass.h|3 
> 9 files changed, 484 insertions(+)
> create mode 100644 gcc/gimple-harden-conditionals.cc
> create mode 100644 gcc/testsuite/c-c++-common/torture/harden-comp.c
> create mode 100644 gcc/testsuite/c-c++-common/torture/harden-cond.c
>
>diff --git a/gcc/Makefile.in b/gcc/Makefile.in
>index 64252697573a7..7209ed117d09d 100644
>--- a/gcc/Makefile.in
>+++ b/gcc/Makefile.in
>@@ -1389,6 +1389,7 @@ OBJS = \
>   gimple-if-to-switch.o \
>   gimple-iterator.o \
>   gimple-fold.o \
>+  gimple-harden-conditionals.o \
>   gimple-laddress.o \
>   gimple-loop-interchange.o \
>   gimple-loop-jam.o \
>diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst 
>b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
>index 1c46e3a4c7b88..52240d7e3dd54 100644
>--- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst
>+++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
>@@ -87,3 +87,43 @@ types and subtypes, may be silently ignored.  Specifically, 
>it is not
> currently recommended to rely on any effects this pragma might be
> expected to have when calling subprograms through access-to-subprogram
> variables.
>+
>+
>+.. Hardened Conditionals:
>+
>+Hardened Conditionals
>+=
>+
>+GNAT can harden conditionals to protect against control flow attacks.
>+
>+This is accomplished by two complementary transformations, each
>+activated by a separate command-line option.
>+
>+The option *-fharden-compares* enables hardening of compares that
>+compute results stored in variables, adding verification that the
>+reversed compare yields the opposite result.
>+
>+The option *-fharden-conditional-branches* enables hardening of
>+compares that guard conditional branches, adding verification of the
>+reversed compare to both execution paths.
>+
>+These transformations are introduced late in the compilation pipeline,
>+long after boolean expressions are decomposed into separate compares,
>+each one turned into either a conditional branch or a compare whose
>+result is stored in a boolean variable or temporary.  Compiler
>+optimizations, if enabled, may also turn conditional branches into
>+stored compares, and vice-versa.  Conditionals may also be optimized
>+out entirely, if their value can be determined at compile time, and
>+occasionally multiple compares can be combined into one.
>+
>+It is thus difficult to predict which of these two options will affect
>+a specific compare operation expressed in source code.  Using both
>+options ensures that every compare that is not optimized out will be
>+hardened.
>+
>+The addition of reversed compares can be observed by enabling the dump
>+files of the corresponding passes, through command line options
>+*-fdump-tree-hardcmp* and *-fdump-tree-hardcbr*, respectively.
>+
>+They are separate options, however, because of the significantly
>+differen

Re: [PATCH] hardened conditionals

[Alexandre Oliva via Gcc-patches]

On Oct  9, 2021, Richard Biener  wrote:

> Why two passes (and two IL traverses?) 

Different traversals, no reason to force them into a single pass.  One
only looks at the last stmt of each block, where cond stmts may be,
while the other has to look at every stmt.

> How do you prevent RTL optimizers (jump threading) from removing the
> redundant tests?

The trick I'm using to copy of a value without the compiler's knowing
it's still the same value is 'asm ("" : "=g" (alt) : "0" (src));'

I've pondered introducing __builtin_hidden_copy or somesuch, but it
didn't seem worth it.

> I'd have expected such hardening to occur very late in the RTL
> pipeline.

Yeah, that would be another way to do it, but then it would have to be a
lot trickier, given all the different ways in which compare-and-branch
can be expressed in RTL.

-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about 

Re: [PATCH] hardened conditionals

[Richard Biener via Gcc-patches]

On Tue, Oct 12, 2021 at 8:35 AM Alexandre Oliva  wrote:
>
> On Oct  9, 2021, Richard Biener  wrote:
>
> > Why two passes (and two IL traverses?)
>
> Different traversals, no reason to force them into a single pass.  One
> only looks at the last stmt of each block, where cond stmts may be,
> while the other has to look at every stmt.
>
> > How do you prevent RTL optimizers (jump threading) from removing the
> > redundant tests?
>
> The trick I'm using to copy of a value without the compiler's knowing
> it's still the same value is 'asm ("" : "=g" (alt) : "0" (src));'
>
> I've pondered introducing __builtin_hidden_copy or somesuch, but it
> didn't seem worth it.

I see.  I remember Marc using sth similar initially when trying to
solve the FENV access problem.  Maybe we indeed want to have
some kind of more generic "dataflow (optimization) barrier" ...

Are there any issues with respect to debugging when using such
asm()s?

> > I'd have expected such hardening to occur very late in the RTL
> > pipeline.
>
> Yeah, that would be another way to do it, but then it would have to be a
> lot trickier, given all the different ways in which compare-and-branch
> can be expressed in RTL.

Agreed, though it would be less disturbing to the early RTL pipeline
and RTL expansion.

> --
> Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
>Free Software Activist   GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about 

Re: [PATCH] hardened conditionals

[Alexandre Oliva via Gcc-patches]

On Oct 12, 2021, Richard Biener  wrote:

> Are there any issues with respect to debugging when using such
> asm()s?

Not in this case.  When creating short-lived copies for immediate use,
like I do in the proposed patch, either the original value remains live
in its original location and we use an actual copy, or the original
value was dead, and we'll have a stmt/insn that visibly marks it as
such, though the value actually remains there.  The newly-added compare
statements use these anonymous, temporary copies, so they're not
relevant for debug information.

Using asms could have effects on debug info and on optimizations in case
their outputs *become* the location/value of the variable, i.e., as if
in source code we did:

  asm ("" : "+g" (var));

After this optimization barrier, the compiler wouldn't know any more
anything it might have known before about the value held in the
variable.  And, if the variable is a gimple register, there would have
to be a new debug bind stmt to bind the variable to its "new" value.
(The debug machinery would assume the asm stmt modifies the value, and
the output would thus overwrite the location with a value unrelated to
the variable without the restated debug bind)


The risk for debug info of introducing such asm stmts after conversion
into SSA is that the debug binds wouldn't be added automatically, as
they are when they're present in source code.


>> Yeah, that would be another way to do it, but then it would have to be a
>> lot trickier, given all the different ways in which compare-and-branch
>> can be expressed in RTL.

> Agreed, though it would be less disturbing to the early RTL pipeline
> and RTL expansion.

Is that a concern, though?  It's not like such structures couldn't be
present in source code, after all, so the RTL machinery has to be able
to deal with them one way or another.  For someone who wishes the
compiler to introduce this hardening, the point in which it's added
seems immaterial to me, as long as it remains all the way to the end.


Now, if we had some kind of optimization barrier at the point of use, we
would be able to save a copy in some cases.  E.g., instead of:

  tmp = var;
  asm ("" : "+g" (tmp));
  if (tmp < cst) ...
  [reads from var]

we could have:

  if (__noopt (var) < cst) ...
  [reads from var]

and that would use var's value without taking an extra copy it, but also
without enabling optimizations based on knowledge about the value.

ISTM that introducing this sort of __noopt use would be a very
significant undertaking.  In RTL, a pseudo set to the original value of
var, and that eventually resolves to the same location that holds that
value, but marked in a way that prevents CSE, fwprop and whatnot (make
it volatile?) would likely get 99% of the way there, but making pseudos
to that end (and having such marks remain after reload) seems nontrivial
to me.

-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about 

Re: [PATCH] hardened conditionals

[Richard Biener via Gcc-patches]

On Wed, Oct 13, 2021 at 8:54 PM Alexandre Oliva  wrote:
>
> On Oct 12, 2021, Richard Biener  wrote:
>
> > Are there any issues with respect to debugging when using such
> > asm()s?
>
> Not in this case.  When creating short-lived copies for immediate use,
> like I do in the proposed patch, either the original value remains live
> in its original location and we use an actual copy, or the original
> value was dead, and we'll have a stmt/insn that visibly marks it as
> such, though the value actually remains there.  The newly-added compare
> statements use these anonymous, temporary copies, so they're not
> relevant for debug information.
>
> Using asms could have effects on debug info and on optimizations in case
> their outputs *become* the location/value of the variable, i.e., as if
> in source code we did:
>
>   asm ("" : "+g" (var));
>
> After this optimization barrier, the compiler wouldn't know any more
> anything it might have known before about the value held in the
> variable.  And, if the variable is a gimple register, there would have
> to be a new debug bind stmt to bind the variable to its "new" value.
> (The debug machinery would assume the asm stmt modifies the value, and
> the output would thus overwrite the location with a value unrelated to
> the variable without the restated debug bind)
>
>
> The risk for debug info of introducing such asm stmts after conversion
> into SSA is that the debug binds wouldn't be added automatically, as
> they are when they're present in source code.
>
>
> >> Yeah, that would be another way to do it, but then it would have to be a
> >> lot trickier, given all the different ways in which compare-and-branch
> >> can be expressed in RTL.
>
> > Agreed, though it would be less disturbing to the early RTL pipeline
> > and RTL expansion.
>
> Is that a concern, though?  It's not like such structures couldn't be
> present in source code, after all, so the RTL machinery has to be able
> to deal with them one way or another.  For someone who wishes the
> compiler to introduce this hardening, the point in which it's added
> seems immaterial to me, as long as it remains all the way to the end.
>
>
> Now, if we had some kind of optimization barrier at the point of use, we
> would be able to save a copy in some cases.  E.g., instead of:
>
>   tmp = var;
>   asm ("" : "+g" (tmp));
>   if (tmp < cst) ...
>   [reads from var]
>
> we could have:
>
>   if (__noopt (var) < cst) ...
>   [reads from var]
>
> and that would use var's value without taking an extra copy it, but also
> without enabling optimizations based on knowledge about the value.
>
> ISTM that introducing this sort of __noopt use would be a very
> significant undertaking.  In RTL, a pseudo set to the original value of
> var, and that eventually resolves to the same location that holds that
> value, but marked in a way that prevents CSE, fwprop and whatnot (make
> it volatile?) would likely get 99% of the way there, but making pseudos
> to that end (and having such marks remain after reload) seems nontrivial
> to me.

Yeah, I think that eventually marking the operation we want to preserve
(with volatile?) would be the best way.  On GIMPLE that's difficult,
it's easier on GENERIC (we can set TREE_THIS_VOLATILE on the
expression at least), and possibly also on RTL (where such flag
might already be a thing?).  So when going that way doing the hardening
on RTL seems easier (if you want to catch all compares, if you want to
only catch compare + jump that has your mentioned issue of all the
different representations).  Of course if volatile non-memory operations
are not a thing already on RTL (well, we _do_ have volatile UNSPECs ...)
then of course you still need to at least look at redundancy elimination
and expression combining passes to honor such flag.

Note that I did not look on the actual patch, I'm trying to see whether there's
some generic usefulness we can extract from the patchs requirement
which to me looks quite specific and given it's "hackish" implementation
way might not be the most important one to carry on trunk (I understand
that AdaCore will carry it in their compiler).

Thanks,
Richard.

>
> --
> Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
>Free Software Activist   GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about 

Re: [PATCH] hardened conditionals

[Alexandre Oliva via Gcc-patches]

On Oct 14, 2021, Richard Biener  wrote:

> Yeah, I think that eventually marking the operation we want to preserve
> (with volatile?) would be the best way.  On GIMPLE that's difficult,
> it's easier on GENERIC (we can set TREE_THIS_VOLATILE on the
> expression at least), and possibly also on RTL (where such flag
> might already be a thing?).

Making the expr volatile would likely get gimple to deal with it like
memory, which would completely defeat the point of trying to avoid a
copy.

RTL has support for volatile MEMs and (user-)REGs indeed, but in order
to avoid the copy, we don't want the pseudo to be volatile, we want
specific users thereof to be.  An unspec_volatile would accomplish that,
but it would require RTL patterns to match it wherever a pseudo might
appear.  Considering all forms of insns involving conditionals on all
relevant targets, that's far too much effort for no measurable beenefit.


> So when going that way doing the hardening on RTL seems easier (if you
> want to catch all compares, if you want to only catch compare + jump
> that has your mentioned issue of all the different representations)

It's not.  RTL has various ways to represent store-flags too.  Even now
that we don't have to worry about implicit CC, a single boolean test may
expand to a compare-and-set-[CC-]reg, and then a
compare-and-store-CC-reg, or a single compare-and-set-[non-CC-]reg, and
IIRC in some cases even more than one (pair of) conditionals.

Compare-and-branches also come in such a multitude of settings.

It all depends on the target, and I don't really see any benefit
whatsoever of implementing such trivial gimple passes with all the
potential complexity of RTL on all the architectures relevant for GCC,
or even for this project alone.

> Note that I did not look on the actual patch, I'm trying to see whether 
> there's
> some generic usefulness we can extract from the patchs requirement
> which to me looks quite specific and given it's "hackish" implementation
> way might not be the most important one to carry on trunk (I understand
> that AdaCore will carry it in their compiler).

It's also simple, no-maintenance, and entirely self-contained.  A good
example of something that could be implemented as a plugin, except for
command-line options.

Maybe we could have a plugin collection in our source tree, to hold
stuff like this and to offer examples of plugins, and means to build
select plugins as such, or as preloaded modules into the compiler for
easier deployment.

-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about 

Re: [PATCH] hardened conditionals

[Richard Biener via Gcc-patches]

On Fri, Oct 15, 2021 at 8:35 PM Alexandre Oliva  wrote:
>
> On Oct 14, 2021, Richard Biener  wrote:
>
> > Yeah, I think that eventually marking the operation we want to preserve
> > (with volatile?) would be the best way.  On GIMPLE that's difficult,
> > it's easier on GENERIC (we can set TREE_THIS_VOLATILE on the
> > expression at least), and possibly also on RTL (where such flag
> > might already be a thing?).
>
> Making the expr volatile would likely get gimple to deal with it like
> memory, which would completely defeat the point of trying to avoid a
> copy.
>
> RTL has support for volatile MEMs and (user-)REGs indeed, but in order
> to avoid the copy, we don't want the pseudo to be volatile, we want
> specific users thereof to be.  An unspec_volatile would accomplish that,
> but it would require RTL patterns to match it wherever a pseudo might
> appear.  Considering all forms of insns involving conditionals on all
> relevant targets, that's far too much effort for no measurable beenefit.
>
>
> > So when going that way doing the hardening on RTL seems easier (if you
> > want to catch all compares, if you want to only catch compare + jump
> > that has your mentioned issue of all the different representations)
>
> It's not.  RTL has various ways to represent store-flags too.  Even now
> that we don't have to worry about implicit CC, a single boolean test may
> expand to a compare-and-set-[CC-]reg, and then a
> compare-and-store-CC-reg, or a single compare-and-set-[non-CC-]reg, and
> IIRC in some cases even more than one (pair of) conditionals.
>
> Compare-and-branches also come in such a multitude of settings.
>
> It all depends on the target, and I don't really see any benefit
> whatsoever of implementing such trivial gimple passes with all the
> potential complexity of RTL on all the architectures relevant for GCC,
> or even for this project alone.
>
> > Note that I did not look on the actual patch, I'm trying to see whether 
> > there's
> > some generic usefulness we can extract from the patchs requirement
> > which to me looks quite specific and given it's "hackish" implementation
> > way might not be the most important one to carry on trunk (I understand
> > that AdaCore will carry it in their compiler).
>
> It's also simple, no-maintenance, and entirely self-contained.

Yes, it is (just having had a quick look most of the functions in the
pass lack function-level comments).

>  A good
> example of something that could be implemented as a plugin, except for
> command-line options.
>
> Maybe we could have a plugin collection in our source tree, to hold
> stuff like this and to offer examples of plugins, and means to build
> select plugins as such, or as preloaded modules into the compiler for
> easier deployment.

I think this has been suggested before, yes.  But if those "plugins"
are as self-contained as yours here there's also no reason to not
simply compile them in as regular passes (unless they are slightly
broken and thus a maintainance burden).

Richard.

> --
> Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
>Free Software Activist   GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about 

Re: [PATCH] hardened conditionals

[Alexandre Oliva via Gcc-patches]

On Oct 18, 2021, Richard Biener  wrote:

> Yes, it is (just having had a quick look most of the functions in the
> pass lack function-level comments).

Oh my, I'm so sorry, please accept my apologies.  I stepped away from
this patch for a few weeks, and when I got back to it, I did not realize
it wasn't quite finished yet.  I misled myself because I had already
written the ChangeLog entry, which I normally do as the last thing
before contributing a patch.  Besides the comments, it was missing
preservation of source location information.  I've implemented the
missing bit, and added comments to all functions and then some.

This patch regstrapped successfully on x86_64-linux-gnu.  Unfortunately,
both this patch and the earlier patch, applied onto recent trunk along
with a patch that enables both passes (both now in aoliva/hardcomp), hit
a bootstrap compare error in insn-opinit.c, confirmed with
-fcompare-debug.

I suppose it's a latent issue exposed by the patch, rather than some
problem introduced by the patch, because the earlier patch had
bootstrapped successfully with both passes enabled back then.

I'm yet to investigate this problem, but I'm a little tied up with
something else ATM, and it's likely an unrelated latent problem to be
fixed in a separate patch, so I'm posting this right away, and even
daring ask: ok to install?


This patch introduces optional passes to harden conditionals used in
branches, and in computing boolean expressions, by adding redundant
tests of the reversed conditions, and trapping in case of unexpected
results.  Though in abstract machines the redundant tests should never
fail, CPUs may be led to misbehave under certain kinds of attacks,
such as of power deprivation, and these tests reduce the likelihood of
going too far down an unexpected execution path.


for  gcc/ChangeLog

* common.opt (fharden-compares): New.
(fharden-conditional-branches): New.
* doc/invoke.texi: Document new options.
* gimple-harden-conditionals.cc: New.
* passes.def: Add new passes.
* tree-pass.h (make_pass_harden_compares): Declare.
(make_pass_harden_conditional_branches): Declare.

for  gcc/ada/ChangeLog

* doc/gnat_rm/security_hardening_features.rst
(Hardened Conditionals): New.

for  gcc/testsuite/ChangeLog

* c-c++-common/torture/harden-comp.c: New.
* c-c++-common/torture/harden-cond.c: New.
---
 gcc/Makefile.in|1 
 .../doc/gnat_rm/security_hardening_features.rst|   40 ++
 gcc/common.opt |8 
 gcc/doc/invoke.texi|   19 +
 gcc/gimple-harden-conditionals.cc  |  435 
 gcc/passes.def |2 
 gcc/testsuite/c-c++-common/torture/harden-comp.c   |   14 +
 gcc/testsuite/c-c++-common/torture/harden-cond.c   |   18 +
 gcc/tree-pass.h|3 
 9 files changed, 540 insertions(+)
 create mode 100644 gcc/gimple-harden-conditionals.cc
 create mode 100644 gcc/testsuite/c-c++-common/torture/harden-comp.c
 create mode 100644 gcc/testsuite/c-c++-common/torture/harden-cond.c

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index f36ffa4740b78..a79ff93dd5999 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1389,6 +1389,7 @@ OBJS = \
gimple-if-to-switch.o \
gimple-iterator.o \
gimple-fold.o \
+   gimple-harden-conditionals.o \
gimple-laddress.o \
gimple-loop-interchange.o \
gimple-loop-jam.o \
diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst 
b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
index 1c46e3a4c7b88..52240d7e3dd54 100644
--- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst
+++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
@@ -87,3 +87,43 @@ types and subtypes, may be silently ignored.  Specifically, 
it is not
 currently recommended to rely on any effects this pragma might be
 expected to have when calling subprograms through access-to-subprogram
 variables.
+
+
+.. Hardened Conditionals:
+
+Hardened Conditionals
+=
+
+GNAT can harden conditionals to protect against control flow attacks.
+
+This is accomplished by two complementary transformations, each
+activated by a separate command-line option.
+
+The option *-fharden-compares* enables hardening of compares that
+compute results stored in variables, adding verification that the
+reversed compare yields the opposite result.
+
+The option *-fharden-conditional-branches* enables hardening of
+compares that guard conditional branches, adding verification of the
+reversed compare to both execution paths.
+
+These transformations are introduced late in the compilation pipeline,
+long after boolean expressions are decomposed into separate compares,
+each one turned into either a conditional branch or a compare whose
+result is stored in a boolean 

Re: [PATCH] hardened conditionals

[Alexandre Oliva via Gcc-patches]

On Oct 20, 2021, Alexandre Oliva  wrote:

> I suppose it's a latent issue exposed by the patch,

I was mistaken.  Though I even had bisected the -fcompare-debug problem
back to a patch from back in May, that added a new sink_code pass before
store_merging, it was actually a bug in my patch, it was just a little
hard to hit with bootstrap-debug, but it came up with -fcompare-debug in
ways that misled me.

split_block remains slightly risky to use unless you know you have or
are going to insert nondebug stmts/insns in both blocks.  I've often
pondered warning in case split_block completes with only debug
stmts/insns in either block, but IIRC there are multiple passes that
split first and insert code afterwards, which have to be rearranged to
aovid the warning.

Anyway, here's the fixed patch.  Regstrapped on x86_64-linux-gnu, and
bootstrapped with an additional patch that enables both new passes.  Ok
to install?


hardened conditionals

From: Alexandre Oliva 

This patch introduces optional passes to harden conditionals used in
branches, and in computing boolean expressions, by adding redundant
tests of the reversed conditions, and trapping in case of unexpected
results.  Though in abstract machines the redundant tests should never
fail, CPUs may be led to misbehave under certain kinds of attacks,
such as of power deprivation, and these tests reduce the likelihood of
going too far down an unexpected execution path.


for  gcc/ChangeLog

* common.opt (fharden-compares): New.
(fharden-conditional-branches): New.
* doc/invoke.texi: Document new options.
* gimple-harden-conditionals.cc: New.
* passes.def: Add new passes.
* tree-pass.h (make_pass_harden_compares): Declare.
(make_pass_harden_conditional_branches): Declare.

for  gcc/ada/ChangeLog

* doc/gnat_rm/security_hardening_features.rst
(Hardened Conditionals): New.

for  gcc/testsuite/ChangeLog

* c-c++-common/torture/harden-comp.c: New.
* c-c++-common/torture/harden-cond.c: New.
---
 gcc/Makefile.in|1 
 .../doc/gnat_rm/security_hardening_features.rst|   40 ++
 gcc/common.opt |8 
 gcc/doc/invoke.texi|   19 +
 gcc/gimple-harden-conditionals.cc  |  439 
 gcc/passes.def |2 
 gcc/testsuite/c-c++-common/torture/harden-comp.c   |   14 +
 gcc/testsuite/c-c++-common/torture/harden-cond.c   |   18 +
 gcc/tree-pass.h|3 
 9 files changed, 544 insertions(+)
 create mode 100644 gcc/gimple-harden-conditionals.cc
 create mode 100644 gcc/testsuite/c-c++-common/torture/harden-comp.c
 create mode 100644 gcc/testsuite/c-c++-common/torture/harden-cond.c

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index f36ffa4740b78..a79ff93dd5999 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1389,6 +1389,7 @@ OBJS = \
gimple-if-to-switch.o \
gimple-iterator.o \
gimple-fold.o \
+   gimple-harden-conditionals.o \
gimple-laddress.o \
gimple-loop-interchange.o \
gimple-loop-jam.o \
diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst 
b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
index 1c46e3a4c7b88..52240d7e3dd54 100644
--- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst
+++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
@@ -87,3 +87,43 @@ types and subtypes, may be silently ignored.  Specifically, 
it is not
 currently recommended to rely on any effects this pragma might be
 expected to have when calling subprograms through access-to-subprogram
 variables.
+
+
+.. Hardened Conditionals:
+
+Hardened Conditionals
+=
+
+GNAT can harden conditionals to protect against control flow attacks.
+
+This is accomplished by two complementary transformations, each
+activated by a separate command-line option.
+
+The option *-fharden-compares* enables hardening of compares that
+compute results stored in variables, adding verification that the
+reversed compare yields the opposite result.
+
+The option *-fharden-conditional-branches* enables hardening of
+compares that guard conditional branches, adding verification of the
+reversed compare to both execution paths.
+
+These transformations are introduced late in the compilation pipeline,
+long after boolean expressions are decomposed into separate compares,
+each one turned into either a conditional branch or a compare whose
+result is stored in a boolean variable or temporary.  Compiler
+optimizations, if enabled, may also turn conditional branches into
+stored compares, and vice-versa.  Conditionals may also be optimized
+out entirely, if their value can be determined at compile time, and
+occasionally multiple compares can be combined into one.
+
+It is thus difficult to predict which of these two options will affect
+a speci

Re: [PATCH] hardened conditionals

[Richard Biener via Gcc-patches]

On Fri, Oct 22, 2021 at 4:19 AM Alexandre Oliva  wrote:
>
> On Oct 20, 2021, Alexandre Oliva  wrote:
>
> > I suppose it's a latent issue exposed by the patch,
>
> I was mistaken.  Though I even had bisected the -fcompare-debug problem
> back to a patch from back in May, that added a new sink_code pass before
> store_merging, it was actually a bug in my patch, it was just a little
> hard to hit with bootstrap-debug, but it came up with -fcompare-debug in
> ways that misled me.
>
> split_block remains slightly risky to use unless you know you have or
> are going to insert nondebug stmts/insns in both blocks.  I've often
> pondered warning in case split_block completes with only debug
> stmts/insns in either block, but IIRC there are multiple passes that
> split first and insert code afterwards, which have to be rearranged to
> aovid the warning.
>
> Anyway, here's the fixed patch.  Regstrapped on x86_64-linux-gnu, and
> bootstrapped with an additional patch that enables both new passes.  Ok
> to install?

OK.

Thanks,
Richard.

>
> hardened conditionals
>
> From: Alexandre Oliva 
>
> This patch introduces optional passes to harden conditionals used in
> branches, and in computing boolean expressions, by adding redundant
> tests of the reversed conditions, and trapping in case of unexpected
> results.  Though in abstract machines the redundant tests should never
> fail, CPUs may be led to misbehave under certain kinds of attacks,
> such as of power deprivation, and these tests reduce the likelihood of
> going too far down an unexpected execution path.
>
>
> for  gcc/ChangeLog
>
> * common.opt (fharden-compares): New.
> (fharden-conditional-branches): New.
> * doc/invoke.texi: Document new options.
> * gimple-harden-conditionals.cc: New.
> * passes.def: Add new passes.
> * tree-pass.h (make_pass_harden_compares): Declare.
> (make_pass_harden_conditional_branches): Declare.
>
> for  gcc/ada/ChangeLog
>
> * doc/gnat_rm/security_hardening_features.rst
> (Hardened Conditionals): New.
>
> for  gcc/testsuite/ChangeLog
>
> * c-c++-common/torture/harden-comp.c: New.
> * c-c++-common/torture/harden-cond.c: New.
> ---
>  gcc/Makefile.in|1
>  .../doc/gnat_rm/security_hardening_features.rst|   40 ++
>  gcc/common.opt |8
>  gcc/doc/invoke.texi|   19 +
>  gcc/gimple-harden-conditionals.cc  |  439 
> 
>  gcc/passes.def |2
>  gcc/testsuite/c-c++-common/torture/harden-comp.c   |   14 +
>  gcc/testsuite/c-c++-common/torture/harden-cond.c   |   18 +
>  gcc/tree-pass.h|3
>  9 files changed, 544 insertions(+)
>  create mode 100644 gcc/gimple-harden-conditionals.cc
>  create mode 100644 gcc/testsuite/c-c++-common/torture/harden-comp.c
>  create mode 100644 gcc/testsuite/c-c++-common/torture/harden-cond.c
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index f36ffa4740b78..a79ff93dd5999 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1389,6 +1389,7 @@ OBJS = \
> gimple-if-to-switch.o \
> gimple-iterator.o \
> gimple-fold.o \
> +   gimple-harden-conditionals.o \
> gimple-laddress.o \
> gimple-loop-interchange.o \
> gimple-loop-jam.o \
> diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst 
> b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
> index 1c46e3a4c7b88..52240d7e3dd54 100644
> --- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst
> +++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
> @@ -87,3 +87,43 @@ types and subtypes, may be silently ignored.  
> Specifically, it is not
>  currently recommended to rely on any effects this pragma might be
>  expected to have when calling subprograms through access-to-subprogram
>  variables.
> +
> +
> +.. Hardened Conditionals:
> +
> +Hardened Conditionals
> +=
> +
> +GNAT can harden conditionals to protect against control flow attacks.
> +
> +This is accomplished by two complementary transformations, each
> +activated by a separate command-line option.
> +
> +The option *-fharden-compares* enables hardening of compares that
> +compute results stored in variables, adding verification that the
> +reversed compare yields the opposite result.
> +
> +The option *-fharden-conditional-branches* enables hardening of
> +compares that guard conditional branches, adding verification of the
> +reversed compare to both execution paths.
> +
> +These transformations are introduced late in the compilation pipeline,
> +long after boolean expressions are decomposed into separate compares,
> +each one turned into either a conditional branch or a compare whose
> +result is stored in a boolean variable or temporary.  Compiler
> +optimizations, if enabled, may also turn conditiona

Re: [PATCH] hardened conditionals

[Alexandre Oliva via Gcc-patches]

On Oct 26, 2021, Richard Biener  wrote:

> OK.

Thanks.  I've just fixed the ChangeLog entry and pushed it:

>> * common.opt (fharden-compares): New.
>> (fharden-conditional-branches): New.
>> * doc/invoke.texi: Document new options.
>> * gimple-harden-conditionals.cc: New.

 + * Makefile.in (OBJS): Build it.

>> * passes.def: Add new passes.
>> * tree-pass.h (make_pass_harden_compares): Declare.
>> (make_pass_harden_conditional_branches): Declare.

-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about 

Re: [PATCH] hardened conditionals

[Alexandre Oliva via Gcc-patches]

FYI, I'm putting in this follow-up tweak to the GNAT manual.


Implied compares in Ada Harded Conditionals documentation

From: Alexandre Oliva 

Improve the wording on optimizations that prevent compare hardening,
so as to also cover cases in which explicit compares get combined into
operations with implied compares.


for  gcc/ada/ChangeLog

* doc/gnat_rm/security_hardening_features.rst: Mention
optimization to operations with implied compares.
---
 .../doc/gnat_rm/security_hardening_features.rst|7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst 
b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
index 52240d7e3dd54..cf76938d91d13 100644
--- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst
+++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
@@ -112,14 +112,15 @@ long after boolean expressions are decomposed into 
separate compares,
 each one turned into either a conditional branch or a compare whose
 result is stored in a boolean variable or temporary.  Compiler
 optimizations, if enabled, may also turn conditional branches into
-stored compares, and vice-versa.  Conditionals may also be optimized
+stored compares, and vice-versa, or into operations with implied
+conditionals (e.g. MIN and MAX).  Conditionals may also be optimized
 out entirely, if their value can be determined at compile time, and
 occasionally multiple compares can be combined into one.
 
 It is thus difficult to predict which of these two options will affect
 a specific compare operation expressed in source code.  Using both
-options ensures that every compare that is not optimized out will be
-hardened.
+options ensures that every compare that is neither optimized out nor
+optimized into implied conditionals will be hardened.
 
 The addition of reversed compares can be observed by enabling the dump
 files of the corresponding passes, through command line options


-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about