This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Add MemoryTagMap class
ClosedPublic

Authored by DavidSpickett on Oct 29 2021, 8:39 AM.

Details

Summary

The tag map holds a sparse set of memory tags and allows
you to query ranges for tags.

Granules that do not have tags will be set to llvm::None.
to keep the ordering intact. If there are no tags for the
requested range we'll just return an empty result so that
callers don't need to check that all values are llvm::None.

This will be combined with MemoryTagManager's MakeTaggedRanges:

  • MakeTaggedRanges
  • Read from all those ranges
  • Insert the results into the tag map
  • Give the tag map to whatever needs to print tags

Which in this case will be "memory read"/DumpDataExtractor.

Diff Detail

Event Timeline

DavidSpickett created this revision.Oct 29 2021, 8:39 AM
DavidSpickett requested review of this revision.Oct 29 2021, 8:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 29 2021, 8:39 AM
JDevlieghere requested changes to this revision.Oct 29 2021, 11:03 AM
JDevlieghere added inline comments.
lldb/include/lldb/Utility/MemoryTagMap.h
12 ↗(On Diff #383373)

I found Utility an odd place for this class, but it certainly cannot depend on Target. This will introduce a cyclic dependency.

19–22 ↗(On Diff #383373)

These should be doxygen-style comments (which will be attached to the class below if you remove the newline).

This revision now requires changes to proceed.Oct 29 2021, 11:03 AM
  • Move to Target/
  • Doxygen style comments at top of class
JDevlieghere added inline comments.Jan 12 2022, 12:56 PM
lldb/include/lldb/Target/MemoryTagMap.h
30

If the pointer should be non-null, then should this take a reference instead?

47

I'm on the fence about this one. I personally prefer empty() for consistency with llvm and the standard library, but technically this should be Empty() to match the rest of LLDB.

DavidSpickett added inline comments.Jan 13 2022, 12:59 AM
lldb/include/lldb/Target/MemoryTagMap.h
30

I would like to do that but in a later patch I have:

static llvm::Optional<MemoryTagMap>
GetMemoryTags(lldb::addr_t addr, size_t length,
              ExecutionContextScope *exe_scope) {

Can't have references in an optional.

However looking again, I think I could check .empty() instead of using an Optional. I'll try that.

47

I see 7 Empty and 3 emptyso I'll go with the former. Not going to put it through any standard algorithms in any case.

This revision is now accepted and ready to land.Jan 19 2022, 3:39 PM

Change MemoryTagMap::empty() to Empty() to match lldb style.

Attempted to use a const& for the tag manager and remembered
why that won't work. Made the explanatory comment more clear.

DavidSpickett added inline comments.Jan 20 2022, 4:12 AM
lldb/include/lldb/Target/MemoryTagMap.h
30

So the reason this doesn't work:

  • Later we have a function that wants to return a memorytagmap.
  • The current arch may not have tagging at all, so it wouldn't have any tag manager to reference.
  • Hence Optional<MemoryTagManager> makes sense for the return type.
  • Optionals cannot include references, so pointer is the best we can do.

On the plus side, this class only has a single user for now so ensuring the non-null part isn't difficult.

This revision was automatically updated to reflect the committed changes.