Re: r347588 - Revert "[clang][slh] add attribute for speculative load hardening"

2018-12-03 Thread David Blaikie via cfe-commits
Also, including the SVN revision (llvm's util/git-svn/git-svnrevert script
can help with this) is helpful for other folks following along who may not
be using git.

On Mon, Nov 26, 2018 at 12:19 PM Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Mon, Nov 26, 2018 at 3:13 PM Zola Bridges via cfe-commits
>  wrote:
> >
> > Author: zbrid
> > Date: Mon Nov 26 12:11:18 2018
> > New Revision: 347588
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=347588&view=rev
> > Log:
> > Revert "[clang][slh] add attribute for speculative load hardening"
> >
> > This reverts commit 801eaf91221ba6dd6996b29ff82659ad6359e885.
>
> Next time you revert something, can you add an explanation as to why
> it was reverted into the commit message? It helps when doing code
> archaeology.
>
> Thanks!
>
> ~Aaron
>
> >
> > Removed:
> > cfe/trunk/test/CodeGen/attr-speculative-load-hardening.cpp
> > cfe/trunk/test/CodeGen/attr-speculative-load-hardening.m
> > cfe/trunk/test/SemaCXX/attr-speculative-load-hardening.cpp
> > Modified:
> > cfe/trunk/include/clang/Basic/Attr.td
> > cfe/trunk/include/clang/Basic/AttrDocs.td
> > cfe/trunk/lib/CodeGen/CGCall.cpp
> > cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> >
> > Modified: cfe/trunk/include/clang/Basic/Attr.td
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=347588&r1=347587&r2=347588&view=diff
> >
> ==
> > --- cfe/trunk/include/clang/Basic/Attr.td (original)
> > +++ cfe/trunk/include/clang/Basic/Attr.td Mon Nov 26 12:11:18 2018
> > @@ -3091,9 +3091,3 @@ def AlwaysDestroy : InheritableAttr {
> >let Subjects = SubjectList<[Var]>;
> >let Documentation = [AlwaysDestroyDocs];
> >  }
> > -
> > -def SpeculativeLoadHardening : InheritableAttr {
> > -  let Spellings = [Clang<"speculative_load_hardening">];
> > -  let Subjects = SubjectList<[Function, ObjCMethod], ErrorDiag>;
> > -  let Documentation = [SpeculativeLoadHardeningDocs];
> > -}
> >
> > Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=347588&r1=347587&r2=347588&view=diff
> >
> ==
> > --- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
> > +++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon Nov 26 12:11:18 2018
> > @@ -3629,27 +3629,3 @@ GNU inline semantics are the default beh
> >  ``-std=c89``, ``-std=c94``, or ``-fgnu89-inline``.
> >}];
> >  }
> > -
> > -def SpeculativeLoadHardeningDocs : Documentation {
> > -  let Category = DocCatFunction;
> > -  let Content = [{
> > -  This attribute can be applied to a function declaration in order to
> indicate
> > -  that `Speculative Load Hardening <
> https://llvm.org/docs/SpeculativeLoadHardening.html>`_
> > -  should be enabled for the function body. This can also be applied to
> a method
> > -  in Objective C.
> > -
> > -  Speculative Load Hardening is a best-effort mitigation against
> > -  information leak attacks that make use of control flow
> > -  miss-speculation - specifically miss-speculation of whether a branch
> > -  is taken or not. Typically vulnerabilities enabling such attacks are
> > -  classified as "Spectre variant #1". Notably, this does not attempt to
> > -  mitigate against miss-speculation of branch target, classified as
> > -  "Spectre variant #2" vulnerabilities.
> > -
> > -  When inlining, the attribute is sticky. Inlining a function that
> > -  carries this attribute will cause the caller to gain the
> > -  attribute. This is intended to provide a maximally conservative model
> > -  where the code in a function annotated with this attribute will always
> > -  (even after inlining) end up hardened.
> > -  }];
> > -}
> >
> > Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=347588&r1=347587&r2=347588&view=diff
> >
> ==
> > --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CGCall.cpp Mon Nov 26 12:11:18 2018
> > @@ -1791,8 +1791,6 @@ void CodeGenModule::ConstructDefaultFnAt
> >  if (CodeGenOpts.Backchain)
> >FuncAttrs.addAttribute("backchain");
> >
> > -// FIXME: The interaction of this attribute with the SLH command
> line flag
> > -// has not been determined.
> >  if (CodeGenOpts.SpeculativeLoadHardening)
> >FuncAttrs.addAttribute(llvm::Attribute::SpeculativeLoadHardening);
> >}
> > @@ -1856,8 +1854,6 @@ void CodeGenModule::ConstructAttributeLi
> >FuncAttrs.addAttribute(llvm::Attribute::NoDuplicate);
> >  if (TargetDecl->hasAttr())
> >FuncAttrs.addAttribute(llvm::Attribute::Convergent);
> > -if (TargetDecl->hasAttr())
> > -  FuncAttrs.addAttribute(llvm::Attribute::SpeculativeLoadHardening)

Re: r347588 - Revert "[clang][slh] add attribute for speculative load hardening"

2018-11-26 Thread Aaron Ballman via cfe-commits
On Mon, Nov 26, 2018 at 3:13 PM Zola Bridges via cfe-commits
 wrote:
>
> Author: zbrid
> Date: Mon Nov 26 12:11:18 2018
> New Revision: 347588
>
> URL: http://llvm.org/viewvc/llvm-project?rev=347588&view=rev
> Log:
> Revert "[clang][slh] add attribute for speculative load hardening"
>
> This reverts commit 801eaf91221ba6dd6996b29ff82659ad6359e885.

Next time you revert something, can you add an explanation as to why
it was reverted into the commit message? It helps when doing code
archaeology.

Thanks!

~Aaron

>
> Removed:
> cfe/trunk/test/CodeGen/attr-speculative-load-hardening.cpp
> cfe/trunk/test/CodeGen/attr-speculative-load-hardening.m
> cfe/trunk/test/SemaCXX/attr-speculative-load-hardening.cpp
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/include/clang/Basic/AttrDocs.td
> cfe/trunk/lib/CodeGen/CGCall.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=347588&r1=347587&r2=347588&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Mon Nov 26 12:11:18 2018
> @@ -3091,9 +3091,3 @@ def AlwaysDestroy : InheritableAttr {
>let Subjects = SubjectList<[Var]>;
>let Documentation = [AlwaysDestroyDocs];
>  }
> -
> -def SpeculativeLoadHardening : InheritableAttr {
> -  let Spellings = [Clang<"speculative_load_hardening">];
> -  let Subjects = SubjectList<[Function, ObjCMethod], ErrorDiag>;
> -  let Documentation = [SpeculativeLoadHardeningDocs];
> -}
>
> Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=347588&r1=347587&r2=347588&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
> +++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon Nov 26 12:11:18 2018
> @@ -3629,27 +3629,3 @@ GNU inline semantics are the default beh
>  ``-std=c89``, ``-std=c94``, or ``-fgnu89-inline``.
>}];
>  }
> -
> -def SpeculativeLoadHardeningDocs : Documentation {
> -  let Category = DocCatFunction;
> -  let Content = [{
> -  This attribute can be applied to a function declaration in order to 
> indicate
> -  that `Speculative Load Hardening 
> `_
> -  should be enabled for the function body. This can also be applied to a 
> method
> -  in Objective C.
> -
> -  Speculative Load Hardening is a best-effort mitigation against
> -  information leak attacks that make use of control flow
> -  miss-speculation - specifically miss-speculation of whether a branch
> -  is taken or not. Typically vulnerabilities enabling such attacks are
> -  classified as "Spectre variant #1". Notably, this does not attempt to
> -  mitigate against miss-speculation of branch target, classified as
> -  "Spectre variant #2" vulnerabilities.
> -
> -  When inlining, the attribute is sticky. Inlining a function that
> -  carries this attribute will cause the caller to gain the
> -  attribute. This is intended to provide a maximally conservative model
> -  where the code in a function annotated with this attribute will always
> -  (even after inlining) end up hardened.
> -  }];
> -}
>
> Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=347588&r1=347587&r2=347588&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGCall.cpp Mon Nov 26 12:11:18 2018
> @@ -1791,8 +1791,6 @@ void CodeGenModule::ConstructDefaultFnAt
>  if (CodeGenOpts.Backchain)
>FuncAttrs.addAttribute("backchain");
>
> -// FIXME: The interaction of this attribute with the SLH command line 
> flag
> -// has not been determined.
>  if (CodeGenOpts.SpeculativeLoadHardening)
>FuncAttrs.addAttribute(llvm::Attribute::SpeculativeLoadHardening);
>}
> @@ -1856,8 +1854,6 @@ void CodeGenModule::ConstructAttributeLi
>FuncAttrs.addAttribute(llvm::Attribute::NoDuplicate);
>  if (TargetDecl->hasAttr())
>FuncAttrs.addAttribute(llvm::Attribute::Convergent);
> -if (TargetDecl->hasAttr())
> -  FuncAttrs.addAttribute(llvm::Attribute::SpeculativeLoadHardening);
>
>  if (const FunctionDecl *Fn = dyn_cast(TargetDecl)) {
>AddAttributesFromFunctionProtoType(
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=347588&r1=347587&r2=347588&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon Nov 

r347588 - Revert "[clang][slh] add attribute for speculative load hardening"

2018-11-26 Thread Zola Bridges via cfe-commits
Author: zbrid
Date: Mon Nov 26 12:11:18 2018
New Revision: 347588

URL: http://llvm.org/viewvc/llvm-project?rev=347588&view=rev
Log:
Revert "[clang][slh] add attribute for speculative load hardening"

This reverts commit 801eaf91221ba6dd6996b29ff82659ad6359e885.

Removed:
cfe/trunk/test/CodeGen/attr-speculative-load-hardening.cpp
cfe/trunk/test/CodeGen/attr-speculative-load-hardening.m
cfe/trunk/test/SemaCXX/attr-speculative-load-hardening.cpp
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=347588&r1=347587&r2=347588&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Mon Nov 26 12:11:18 2018
@@ -3091,9 +3091,3 @@ def AlwaysDestroy : InheritableAttr {
   let Subjects = SubjectList<[Var]>;
   let Documentation = [AlwaysDestroyDocs];
 }
-
-def SpeculativeLoadHardening : InheritableAttr {
-  let Spellings = [Clang<"speculative_load_hardening">];
-  let Subjects = SubjectList<[Function, ObjCMethod], ErrorDiag>;
-  let Documentation = [SpeculativeLoadHardeningDocs];
-}

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=347588&r1=347587&r2=347588&view=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon Nov 26 12:11:18 2018
@@ -3629,27 +3629,3 @@ GNU inline semantics are the default beh
 ``-std=c89``, ``-std=c94``, or ``-fgnu89-inline``.
   }];
 }
-
-def SpeculativeLoadHardeningDocs : Documentation {
-  let Category = DocCatFunction;
-  let Content = [{
-  This attribute can be applied to a function declaration in order to indicate
-  that `Speculative Load Hardening 
`_
-  should be enabled for the function body. This can also be applied to a method
-  in Objective C.
-
-  Speculative Load Hardening is a best-effort mitigation against
-  information leak attacks that make use of control flow
-  miss-speculation - specifically miss-speculation of whether a branch
-  is taken or not. Typically vulnerabilities enabling such attacks are
-  classified as "Spectre variant #1". Notably, this does not attempt to
-  mitigate against miss-speculation of branch target, classified as
-  "Spectre variant #2" vulnerabilities.
-
-  When inlining, the attribute is sticky. Inlining a function that
-  carries this attribute will cause the caller to gain the
-  attribute. This is intended to provide a maximally conservative model
-  where the code in a function annotated with this attribute will always
-  (even after inlining) end up hardened.
-  }];
-}

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=347588&r1=347587&r2=347588&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Mon Nov 26 12:11:18 2018
@@ -1791,8 +1791,6 @@ void CodeGenModule::ConstructDefaultFnAt
 if (CodeGenOpts.Backchain)
   FuncAttrs.addAttribute("backchain");
 
-// FIXME: The interaction of this attribute with the SLH command line flag
-// has not been determined.
 if (CodeGenOpts.SpeculativeLoadHardening)
   FuncAttrs.addAttribute(llvm::Attribute::SpeculativeLoadHardening);
   }
@@ -1856,8 +1854,6 @@ void CodeGenModule::ConstructAttributeLi
   FuncAttrs.addAttribute(llvm::Attribute::NoDuplicate);
 if (TargetDecl->hasAttr())
   FuncAttrs.addAttribute(llvm::Attribute::Convergent);
-if (TargetDecl->hasAttr())
-  FuncAttrs.addAttribute(llvm::Attribute::SpeculativeLoadHardening);
 
 if (const FunctionDecl *Fn = dyn_cast(TargetDecl)) {
   AddAttributesFromFunctionProtoType(

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=347588&r1=347587&r2=347588&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon Nov 26 12:11:18 2018
@@ -6373,9 +6373,6 @@ static void ProcessDeclAttribute(Sema &S
   case ParsedAttr::AT_Section:
 handleSectionAttr(S, D, AL);
 break;
-  case ParsedAttr::AT_SpeculativeLoadHardening:
-handleSimpleAttribute(S, D, AL);
-break;
   case ParsedAttr::AT_CodeSeg:
 handleCodeSegAttr(S, D, AL);
 break;

Removed: cfe/trunk/test/CodeGen/attr-speculative-load-hardening.cpp
URL: 
http://llvm.org/viewvc/llvm-projec