Page MenuHomePhabricator

[MTE] Add AArch64GlobalsTagging Pass
AcceptedPublic

Authored by hctim on Sep 6 2022, 6:05 PM.

Details

Reviewers
fmayer
eugenis
Summary

Adds the IR pass to tag global variables. You can see the official ABI
proposal at https://github.com/ARM-software/abi-aa/pull/166.

This pass very simply goes over the taggable global variables, re-sizes
and re-aligns them so that they're matched up to the tag granule size
(16 bytes), and marks them as tagged. This allows the backend to create
relocations and put them into a special section.

Global merge must be suppressed for tagged globals, as each global
variable must have a unique tag. This can possibly be relaxed in future;
globals that are identical in size, alignment, and content can
possibly be merged. The major problem comes from tail- or head-merging.

In addition, make sure that tagged globals in object files have
referenced symtab entries for relocations. This is necessary for the
linker to know where a tag should come from, as there's some special
dynamic relocation logic that applies
(https://github.com/ARM-software/abi-aa/blob/64458f8608b69536880501caced7c95a10a49d24/memtagabielf64/memtagabielf64.rst#extended-semantics-of-r-aarch64-relative)

Diff Detail

Event Timeline

hctim created this revision.Sep 6 2022, 6:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 6 2022, 6:05 PM
hctim requested review of this revision.Sep 6 2022, 6:05 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptSep 6 2022, 6:05 PM
fmayer added inline comments.Sep 6 2022, 6:19 PM
llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp
61

not necessarily in this CL: do we want some attribute to turn off instrumentation?

99

can we use the SmallVector here?

109

what does this do?

hctim updated this revision to Diff 458472.Sep 7 2022, 9:02 AM
hctim marked 3 inline comments as done.

Update from Florian's comments.

llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp
61

memtag sanitization is not-implicit (i.e. it's opt-in rather than opt-out): https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/IR/GlobalValue.h#L327, so a turn-off attribute should be unnecessary

109

dead code, thanks, removed

tschuett added inline comments.
llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp
94

If the 16 is the size of the granule, then it deserves to be named constant.

hctim updated this revision to Diff 458487.Sep 7 2022, 9:51 AM
hctim marked an inline comment as done.

Update inlined constants to use named constant.

fmayer added inline comments.Sep 7 2022, 11:04 AM
llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp
26

why don't we put this into the llvm namespace rather than using?

tschuett added inline comments.Sep 7 2022, 11:10 AM
llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp
30

I believe you are going too far with the anonymous namespace. There is no need for static functions in anonymous namespaces.

https://llvm.org/docs/CodingStandards.html#anonymous-namespaces

hctim updated this revision to Diff 458536.Sep 7 2022, 12:41 PM
hctim marked 2 inline comments as done.

Some namespace moving and shaking.

llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp
26

common for code in this folder, INITIALIZE_PASS_BEGIN (below) errors out not being in the llvm namespace, but also warns about redundantly using llvm:: in the macro expansion if you explicitly use the namespace...

30

moved everything but the class out of the namespace

94

thanks, done

fmayer accepted this revision.Sep 7 2022, 12:42 PM
This revision is now accepted and ready to land.Sep 7 2022, 12:42 PM

Change description says that the new pass "marks them as tagged". That's not what is happening.

llvm/lib/CodeGen/GlobalMerge.cpp
657

This comment is not convincing. The change description is right that some globals can be merged, copy that here, or extend the comment a little to explain why and under what assumptions.

llvm/lib/MC/ELFObjectWriter.cpp
1344

Isn't a bigger reason that we need to know the address tag to put on the pointer, and that requires a symbol, not a section reference?