[PATCH] D100546: [ARM][AArch64] SLSHardening: make non-comdat thunks possible

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:837-840
+    if ((EnableRetBr || EnableBlr) && DisableComdat) {
+      CmdArgs.push_back("-mllvm");
+      CmdArgs.push_back("-arm-sls-hardening-comdat=false");
+    }
----------------
I think it would be better if comdat/nocomdat were also communicated through Features.
It keeps the interface on this part of the code base consistent (no need to introduce a CmdArgs feature); and also makes sure that LTO vs non-LTO builds keep on behaving the same way, as the feature is set per function, not per translation unit.

I haven't really thought about how to best specify that feature. Maybe just have an extra 2 features: +harden-sls-comdat and +harden-sls-nocomdat?


================
Comment at: llvm/test/CodeGen/AArch64/speculation-hardening-sls.ll:218-220
+; HARDEN-COMDAT-OFF-NOT:  .section .text.__llvm_slsblr_thunk_x19
+; HARDEN-COMDAT-OFF-NOT:  .hidden __llvm_slsblr_thunk_x19
+; HARDEN-COMDAT-OFF-NOT:  .weak __llvm_slsblr_thunk_x19
----------------
Would it be possible to also have check lines for HARDEN-COMDAT-OFF, i.e. look for lines that HARDEN-COMDAT-OFF should produce?


================
Comment at: llvm/test/CodeGen/ARM/speculation-hardening-sls.ll:249-251
+; HARDEN-COMDAT-OFF-NOT:  .section {{.text.__llvm_slsblr_thunk_(arm|thumb)_r5}}
+; HARDEN-COMDAT-OFF-NOT:  .hidden {{__llvm_slsblr_thunk_(arm|thumb)_r5}}
+; HARDEN-COMDAT-OFF-NOT:  .weak {{__llvm_slsblr_thunk_(arm|thumb)_r5}}
----------------
Would it be possible to also have check lines for HARDEN-COMDAT-OFF, i.e. look for lines that HARDEN-COMDAT-OFF should produce?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100546/new/

https://reviews.llvm.org/D100546