[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

[Matthew Riley via Phabricator via cfe-commits]

mattdr added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:248-249
+  def warn_slh_does_not_support_gcc_asm_goto : Warning<
+    "speculative load hardening does not support use of GCC asm goto. asm goto "
+        "detected with SLH">, InGroup<DiagGroup<"slh-asm-goto">>;
 }
----------------
I think at the UI level this feature is just called "asm goto" with no "GCC". See e.g. https://lists.llvm.org/pipermail/llvm-dev/2018-October/127239.html

Also, since this is a warning (vs. an error), we probably want to hint about the consequences of continuing despite the warning.

My attempt:
"Speculative load hardening may not fully protect functions with 'asm goto'"



================
Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:249
+    "speculative load hardening does not support use of GCC asm goto. asm goto "
+        "detected with SLH">, InGroup<DiagGroup<"slh-asm-goto">>;
 }
----------------
I don't think we need to attach a `DiagGroup` to this. The goal would be to allow users to turn a bunch of different diagnostics on/off at once, which doesn't apply here yet.

If in the future we **do** need to add a `DiagGroup`, I think we would need to add it here before attaching it to specific warnings: https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Basic/DiagnosticGroups.td




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79743