[PATCH] D154043: [CodeGen] -fsanitize={function, kcfi}: ensure align 4 if +strict-align

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D154043#4459446 <https://reviews.llvm.org/D154043#4459446>, @simon_tatham wrote:

> The details of this approach look good to me, but is this the best place to solve it? Doing it in clang means that //every// language front end that wants to use either of these sanitizers is responsible for doing this same work: tagging every IR function with `align 4` if it also has `!kcfi_type` or `!func_sanitize`, and perhaps also checking the target-features to decide whether to do that.
>
> I'd imagined the problem being solved at a lower level, when converting the IR into actual function prologues, so that all front ends generating IR would benefit from the fix.



In D154043#4460665 <https://reviews.llvm.org/D154043#4460665>, @efriedma wrote:

> I also think it makes sense to fix the alignment when we lower the metadata, not in the frontend, unless I'm missing something.
>
> It's not clear to me how "strict-align" is relevant; if sanitizer lowering is generating "align 4" loads, the relevant pointers need to be appropriately aligned regardless of the cost of unaligned loads.  Misaligned loads are undefined behavior in LLVM IR on all targets.  (32-bit ARM in particular has cases where 32-bit unaligned loads are supported, but certain load instruction variations enforce alignment.)

OK. See D154125 <https://reviews.llvm.org/D154125> for the MachineFunction.cpp approach. If we go that direction, I'll abandon this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154043