This is an archive of the discontinued LLVM Phabricator instance.

Reland "[lldb] Remove non address bits when looking up memory regions"
ClosedPublic

Authored by DavidSpickett on Dec 10 2021, 3:40 AM.

Details

Summary

This reverts commit 0df522969a7a0128052bd79182c8d58e00556e2f.

Additional checks are added to fix the detection of the last memory region
in GetMemoryRegions or repeating the "memory region" command when the
target has non-address bits.

Normally you keep reading from address 0, looking up each region's end
address until you get LLDB_INVALID_ADDR as the region end address.
(0xffffffffffffffff)

This is what the remote will return once you go beyond the last mapped region:
[0x0000fffffffdf000-0x0001000000000000) rw- [stack]
[0x0001000000000000-0xffffffffffffffff) ---

Problem is that when we "fix" the lookup address, we remove some bits
from it. On an AArch64 system we have 48 bit virtual addresses, so when
we fix the end address of the [stack] region the result is 0.
So we loop back to the start.

[0x0000fffffffdf000-0x0001000000000000) rw- [stack]
[0x0000000000000000-0x0000000000400000) ---

To fix this I added an additional check for the last range.
If the end address of the region is different once you apply
FixDataAddress, we are at the last region.

Since the end of the last region will be the last valid mappable
address, plus 1. That 1 will be removed by the ABI plugin.

The only side effect is that on systems with non-address bits, you
won't get that last catch all unmapped region from the max virtual
address up to 0xf...f.

[0x0000fffff8000000-0x0000fffffffdf000) ---
[0x0000fffffffdf000-0x0001000000000000) rw- [stack]
<ends here>

Though in some way this is more correct because that region is not
just unmapped, it's not mappable at all.

No extra testing is needed because this is already covered by
TestMemoryRegion.py, I simply forgot to run it on system that had
both top byte ignore and pointer authentication.

This change has been tested on a qemu VM with top byte ignore,
memory tagging and pointer authentication enabled.

Diff Detail

Event Timeline

DavidSpickett created this revision.Dec 10 2021, 3:40 AM
DavidSpickett requested review of this revision.Dec 10 2021, 3:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 10 2021, 3:40 AM

The original change was https://reviews.llvm.org/D102757, I'll add comments showing the new code relative to that.

DavidSpickett added inline comments.
lldb/source/Commands/CommandObjectMemory.cpp
1671

This check has been updated.

lldb/source/Target/Process.cpp
5896

This section has been added and converted from do/while to while and break.

Added a release note for the feture of handling non-address bits in input.
Including the change to the last memory region, which could conceiveably
trip up some scripts if they are also using 0xF...F as their end conditions.

Herald added a project: Restricted Project. · View Herald TranscriptDec 16 2021, 2:52 AM
  • Revert back to the do...while structure.
  • Reword the explanatory comments.

@omjavaid ping!

Using fixed address != original address inspired by https://reviews.llvm.org/D115431#change-7Gybl0dcCOwp. I first used < which I think works out the same but != is clearer.

Apologies for late response!!!

lldb/source/Commands/CommandObjectMemory.cpp
1668

Perhaps this condition should only run when argument is not equal to 1. When user supplies an argument which has non-address-bits set, load_addr may not be equal to value returned by FixDataAddress(load_address). This will fire a invalid argument error which according to comment above we do not intend?

lldb/source/Target/Process.cpp
5890

Just to confirm this assumes range base will always be free of any non-address-bits. ?

DavidSpickett added inline comments.Jan 13 2022, 5:39 AM
lldb/source/Commands/CommandObjectMemory.cpp
1668

Took me a while to figure out how to trigger it but you're right.

(lldb) memory region 0
<...>
(lldb) memory region
<...repeat the command a bunch of times...>
(lldb)
[0x0000fffffffdf000-0x0001000000000000) rw- [stack]
<m_prev_end_addr now has non-address bits>
(lldb) memory region 12345
error: 'memory region' takes one argument:
Usage: memory region ADDR
<error despite 12345 not being tagged>

I'll fix this issue.

For the simpler case where you have no previous address, or one with no non-address bits, that works. (and is covered by the included test)

DavidSpickett added inline comments.Jan 13 2022, 5:46 AM
lldb/source/Target/Process.cpp
5890

Not always but the first range base that has non-address bits will be the range *after* the last mappable range. (end of last mappable range == begin of first unmappable range)

Since we only care about mappable regions here anyway, we can ignore the base addresses. (I'll add this in a comment too)

DavidSpickett added inline comments.Jan 13 2022, 5:49 AM
lldb/source/Target/Process.cpp
5890

And for a mappable range to have non-address bits in the base the OS would have to report it that way. Which I can't say is completely out of the question but I don't see Linux doing it at least.

  • Fix bug where the previous end address was checked for non-address bits even when the user gave a new address.
  • Added testing for that situation.
  • Added a comment to explain that non-address bits in the start/base of a memory region are possible but we can ignore them.
DavidSpickett marked 2 inline comments as done.Jan 13 2022, 6:19 AM

Rebase and update release notes format.

omjavaid accepted this revision.Feb 9 2022, 3:35 AM
This revision is now accepted and ready to land.Feb 9 2022, 3:35 AM
This revision was landed with ongoing or failed builds.Feb 10 2022, 2:43 AM
This revision was automatically updated to reflect the committed changes.