[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

Mon May 23 04:13:05 PDT 2022

pengfei added a comment.

In D126137#3530777 <https://reviews.llvm.org/D126137#3530777>, @kristof.beyls wrote:

> See https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html, documentation for "mharden-sls": For AArch64, the options available on the command line are "retbr", "blr", "none" and "all".
> I don't think the options necessarily have to be the same for x86.
> But assuming I understand this patch correctly, it seems to me that with this patch -mharden-sls=all would mean fundamentally slightly different things for x86 vs arm and aarch64, which could be confusing to users.
> IIUC this patch correctly, this patch implements the equivalent of aarch64/arm's -mharden-sls=retbr (i.e. add a straight-line-speculation mitigation for returns and indirect jumps, but not for indirect function calls).
> Therefore, I wonder if it wouldn't be better to name this -mharden-sls=retbr for more consistency across architectures?
> Or is the indirect function call case not relevant for x86 (sorry - I'm not up to speed on the details on the x86 side)?
>
> Or does `MBB.back().getDesc().isIndirectBranch()` also return True for indirect calls, in which case my whole remark here can probably be ignored?

Thanks for the information! I referred to the AArch64's implementation when I wrote the patch. But the ideas came from GCC. You can find the documentation of X86 on GCC https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html#x86-Options which has already been different from AArch64. The `all` here means 'return' + 'indirect-jmp'. So, yes, there's no indirect function call here. But it matches with GCC X86's options. (A tangential question is do we need these two options expect `all`? @nickdesaulniers)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126137