MaskRay added inline comments.
================
Comment at: clang/docs/ReleaseNotes.rst:371
+- Support ``-mharden-sls=all`` for X86.
+
----------------
MaskRay wrote:
> nickdesaulniers wrote:
> > pengfei wrote:
> > > nickdesaulniers wrote:
> > > > This should be updated if additional options are supported.
> > > >
> > > > You should also update `clang/docs/ClangCommandLineReference.rst` I
> > > > think.
> > > There's already introduction there.
> > We should expand the introduction there to state explicitly what options
> > are supported for which targets.
> Delete ` for X86`. The section is about X86.
I'd add ` for straight-line speculation hardening`
================
Comment at: clang/include/clang/Driver/Options.td:3525
+ HelpText<"Select straight-line speculation hardening scope (ARM/AArch64/X86 "
+ "only). <arg> must be 'all', 'none', 'retbr'(ARM/AArch64), "
+ "'blr'(ARM/AArch64), 'comdat'(ARM/AArch64),
'nocomdat'(ARM/AArch64),"
----------------
`must be one of: `.
Single quotes can be removed.
================
Comment at: clang/lib/Driver/ToolChains/Arch/X86.cpp:259
+ } else if (Scope == "indirect-jmp") {
+ Features.push_back("+harden-sls-ind");
+ } else if (Scope != "none") {
----------------
The name harden-sls-ind seems a bit different from indirect-jmp. Use a more
descriptive feature name.
================
Comment at: clang/test/Driver/x86-target-features.c:313
+// RUN: %clang --target=i386-unknown-linux-gnu -march=i386
-mharden-sls=return,indirect-jmp %s -### -o %t.o 2>&1 | FileCheck
-check-prefix=BAD-SLS %s
+// NO-SLS-NOT: harden-sls
+// SLS-RET-DAG: "-target-feature" "+harden-sls-ret"
----------------
This can cause false positive if the path components of %s have `harden-sls`,
`...-NOT: "+harden-sls"` should be more robust.
================
Comment at: clang/test/Driver/x86-target-features.c:308-309
+
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mharden-sls=all %s
-### -o %t.o 2>&1 | FileCheck -check-prefix=SLS %s
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mharden-sls=none %s
-### -o %t.o 2>&1 | FileCheck -check-prefix=NO-SLS %s
+// SLS: "-target-feature" "+harden-sls-all"
----------------
MaskRay wrote:
> nickdesaulniers wrote:
> > Please add a test for `-mharden-sls=all -mharden-sls=none` to verify that
> > the last value "wins."
> ` -target ` is deprecated legacy spelling. Better to use `--target=`
If the feature applies to any ELF OSes, use --target=i386 (generic ELF) instead
of --target=i386-unknown-linux-gnu.
================
Comment at: llvm/test/CodeGen/X86/speculation-hardening-sls.ll:1
+; RUN: llc -mattr=harden-sls-ret -verify-machineinstrs
-mtriple=x86_64-unknown-unknown < %s | FileCheck %s -check-prefixes=CHECK,RET
+; RUN: llc -mattr=harden-sls-ind -verify-machineinstrs
-mtriple=x86_64-unknown-unknown < %s | FileCheck %s -check-prefixes=CHECK,IND
----------------
Most `-verify-machineinstrs ` usage is redundant.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126137/new/
https://reviews.llvm.org/D126137
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
