[PATCH] D130145: [AArch64] Simplify BTI/PAC-RET module flags

[Fangrui Song via Phabricator via cfe-commits]

MaskRay marked an inline comment as done.
MaskRay added inline comments.


================
Comment at: llvm/lib/IR/AutoUpgrade.cpp:4415-4430
-    // Upgrade branch protection and return address signing module flags. The
-    // module flag behavior for these fields were Error and now they are Min.
-    if (ID->getString() == "branch-target-enforcement" ||
-        ID->getString().startswith("sign-return-address")) {
-      if (auto *Behavior =
-              mdconst::dyn_extract_or_null<ConstantInt>(Op->getOperand(0))) {
-        if (Behavior->getLimitedValue() == Module::Error) {
----------------
danielkiss wrote:
> MaskRay wrote:
> > danielkiss wrote:
> > > In a full LTO build the linker will complain about the mis match between the flags if one of the objects compiled with an older compiler that emitted `Module:Error` for these flags.
> > The new approach doesnot emit a flag for 0 value, so there won't be an error even with old bitcode files using `Error`.
> older compiler could emit `Module:Error` with 0 and with this patch we emit 1 with `min` so the linker will stop with 
> `ld.lld: error: linking module flags 'branch-target-enforcement': IDs have conflicting behaviors in 'foo.o' and 'ld-temp.o'`
> 
> ```
> clang++-13 -flto=full -c foo.cpp -o foo.o
> ../build/bin/clang++ -flto=full -c main.cpp -o main.o -mbranch-protection=standard
> ../build/bin/clang++ -fuse-ld=lld main.o foo.o -o a.out
> ```
> 
Perhaps we should respect the Error for older llvm and not try fixing it for regular LTO? If you feel this is strongly needed, I'll have to restore this chunk. The main purpose is to remove the module flags for 0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130145