This is an archive of the discontinued LLVM Phabricator instance.

Support the min of module flags when linking, use for AArch64 BTI/PAC-RET
ClosedPublic

Authored by danielkiss on Apr 11 2022, 3:34 AM.

Details

Summary

LTO objects might compiled with different mbranch-protection flags which will cause an error in the linker.
Such a setup is allowed in the normal build with this change that is possible.

Diff Detail

Event Timeline

danielkiss created this revision.Apr 11 2022, 3:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 11 2022, 3:34 AM
danielkiss requested review of this revision.Apr 11 2022, 3:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 11 2022, 3:34 AM

This would silently reduce protection, when incompatible modules are linked?

This would silently reduce protection, when incompatible modules are linked?

Yes, but that happens also with the normal linker too by default.

pcc added a comment.Apr 11 2022, 10:16 AM

Can we add logic to llvm/lib/IR/AutoUpgrade.cpp to change Error to Min if we see one of these module flags? I think that should allow old bitcode to continue working.

Can we add logic to llvm/lib/IR/AutoUpgrade.cpp to change Error to Min if we see one of these module flags? I think that should allow old bitcode to continue working.

Thanks, I added the support there. Surprisingly the AutoUpgrade did not run in the linker.

pcc accepted this revision.Apr 12 2022, 10:17 AM

LGTM but please add a test for the autoupgrader.

This revision is now accepted and ready to land.Apr 12 2022, 10:17 AM

Thanks, test is added.

This revision was landed with ongoing or failed builds.Apr 13 2022, 12:32 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2022, 12:32 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
MaskRay added a subscriber: MaskRay.EditedJun 8 2022, 9:44 PM

This behavior requires that all participating modules have the module flag. In the absence of the module flag, what should be behavior be?

For module flags like "sign-return-address", I see that you just emit the flag unconditionally (which can be zero).

This behavior requires that all participating modules have the module flag. In the absence of the module flag, what should be behavior be?

For module flags like "sign-return-address", I see that you just emit the flag unconditionally (which can be zero).

Absence of the "branch-target-enforcement" and "sign-return-address"` means features are off. So the flag merger will take an existing flag and the feature could be turned on, which is bad.
AutoUpdater should insert the attributes with zero value if they are missing.