This is an archive of the discontinued LLVM Phabricator instance.

[lldb][AArch64] Add support for memory tags in core files
ClosedPublic

Authored by DavidSpickett on Jul 11 2022, 7:59 AM.

Details

Summary

This teaches ProcessElfCore to recognise the MTE tag segments.

https://www.kernel.org/doc/html/latest/arm64/memory-tagging-extension.html#core-dump-support

These segments contain all the tags for a matching memory segment
which will have the same size in virtual address terms. In real terms
it's 2 tags per byte so the data in the segment is much smaller.

Since MTE is the only tag type supported I have hardcoded some
things to those values. We could and should support more formats
as they appear but doing so now would leave code untested until that
happens.

A few things to note:

  • /proc/pid/smaps is not in the core file, only the details you have in "maps". Meaning we mark a region tagged only if it has a tag segment.
  • A core file supports memory tagging if it has at least 1 memory tag segment, there is no other flag we can check to tell if memory tagging was enabled. (unlike a live process that can support memory tagging even if there are currently no tagged memory regions)

Tests have been added at the commands level for a core file with
mte and without.

There is a lot of overlap between the "memory tag read" tests here and the unit tests for
MemoryTagManagerAArch64MTE::UnpackTagsFromCoreFileSegment, but I think it's
worth keeping to check ProcessElfCore doesn't cause an assert.

Depends on D129487

Diff Detail

Event Timeline

DavidSpickett created this revision.Jul 11 2022, 7:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2022, 7:59 AM
DavidSpickett requested review of this revision.Jul 11 2022, 7:59 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 11 2022, 7:59 AM

This change is purely about reading the tags.

There will be another change to get fault reporting working. Right now lldb only looks at signo so you see a segfault but not what type of segfault.

Add a release note.

I am wondering if we should also test memory reads with correct/wrong tags for corefiles as well? Do you see any value on doing it or have done it already in above test and i skipped it?

lldb/test/API/linux/aarch64/mte_core_file/main.c
2

Do we have a hardware board that supports MTE or we are still using QEMU?

DavidSpickett marked an inline comment as done.Jul 19 2022, 7:13 AM

I am wondering if we should also test memory reads with correct/wrong tags for corefiles as well? Do you see any value on doing it or have done it already in above test and i skipped it?

You mean "memory read <the tagged memory buffer> --show-tags"? The details are tested on live processes already but a simple smoke test wouldn't hurt, I'll do that.

lldb/test/API/linux/aarch64/mte_core_file/main.c
2

Still using QEMU.

DavidSpickett marked an inline comment as done.

rebase

You mean "memory read <the tagged memory buffer> --show-tags"? The details are tested on live processes already but a simple smoke test wouldn't hurt, I'll do that.

I checked and to include the memory contents takes the core file from 20k to 300 something k. This isn't the end of the world but thinking about it, the code that powers "memory read --show-tags" is built on top of the stuff that's already tested here.

And that show tags code is tested via unit tests and live processes elsewhere, so I don't think we need anything for it here.

You mean "memory read <the tagged memory buffer> --show-tags"? The details are tested on live processes already but a simple smoke test wouldn't hurt, I'll do that.

I checked and to include the memory contents takes the core file from 20k to 300 something k. This isn't the end of the world but thinking about it, the code that powers "memory read --show-tags" is built on top of the stuff that's already tested here.

And that show tags code is tested via unit tests and live processes elsewhere, so I don't think we need anything for it here.

Agreed!

omjavaid accepted this revision.Jul 21 2022, 12:32 AM
This revision is now accepted and ready to land.Jul 21 2022, 12:32 AM

@labath (or indeed anyone else) any objections to this or the previous patch?

Happy to make changes but if it's ok as is I'd like to get it landed before the branch next week.