Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i#2154 android64: Fix base address issue to get drrun working #7186

Merged
merged 2 commits into from
Jan 16, 2025
+6 −2
Merged

i#2154 android64: Fix base address issue to get drrun working #7186

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9bbe5b2
i#2154 android64: Fix base address issue to get drrun working
felixc-arm Jan 14, 2025
850095c
Merge branch 'master' into i2154-android64-base-address
felixc-arm Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
@@ -426,6 +426,10 @@ if (WIN32 AND DEBUG)
elseif (APPLE AND X64)
# Set to higher than _PAGEZERO which is [0..0x1'00000000).
set(preferred_base "0x171000000" CACHE STRING "Preferred library base address")
elseif (ANDROID64)
# 64-bit Android needs a base of 0x1000 so that libdynamorio.so
# does not get overwritten by the vDSO.
set(preferred_base "0x1000" CACHE STRING "Preferred library base address")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow this is very low: surprising that Android doesn't have this permanently unmapped. Where is vdso at? I guess it doesn't matter where libdynamorio.so goes since there are no reachability constraints on it vs vmheap or vmcode so super low is ok, but it feels like it's more likely to disrupt typically layout patterns when it's low: does the Android kernel normally load anything down at these low addresses? Normally higher ones are more likely to leave the address space layout unchanged.

Copy link
Collaborator Author

@felixc-arm felixc-arm Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it seems that this has a different effect in Android, even though the base is set low the actual addresses are in a more normal area:

With this new change libdynamorio.so is still placed in higher addresses:

Sections for '/data/local/tmp/build/lib64/debug/libdynamorio.so' (aarch64):
  SectID             Type                   Load Address                             Perm File Off.  File Size  Flags      Section Name
  ------------------ ---------------------- ---------------------------------------  ---- ---------- ---------- ---------- ----------------------------
  0xfffffffffffffffe container              [0x0000007ff77fc000-0x0000007ff77fc270)  r--  0x00000000 0x00000270 0x00000000 libdynamorio.so.PT_LOAD[0]
  0xfffffffffffffffd container              [0x0000007ff77fd000-0x0000007ff7ea7cf0)  r-x  0x00001000 0x006aacf0 0x00000000 libdynamorio.so.PT_LOAD[1]
  0x0000000000000001 code                   [0x0000007ff77fd000-0x0000007ff7ea7cf0)  r-x  0x00001000 0x006aacf0 0x00000006 libdynamorio.so.PT_LOAD[1]..text
  0xfffffffffffffffc container              [0x0000007ff7ea8cf0-0x0000007ff7f9fab8)  r--  0x006abcf0 0x000f6dc8 0x00000000 libdynamorio.so.PT_LOAD[2]

Sections for '[vdso](0x0000007ff77fb000)' (aarch64):
  SectID             Type                   Load Address                             Perm File Off.  File Size  Flags      Section Name
  ------------------ ---------------------- ---------------------------------------  ---- ---------- ---------- ---------- ----------------------------
  0xffffffffffffffff container              [0x0000007ff77fb000-0x0000007ff77fb968)  r-x  0x00000000 0x00000968 0x00000000 [vdso].PT_LOAD[0]

ELF headers for both libdynamorio.so and vDSO are at the start of their sections as expected:

(lldb) memory read 0x0000007ff77fb000
0x7ff77fb000: 7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00  .ELF............
0x7ff77fb010: 03 00 b7 00 01 00 00 00 e0 02 00 00 00 00 00 00  ................
(lldb) memory read 0x0000007ff77fc000
0x7ff77fc000: 7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00  .ELF............
0x7ff77fc010: 03 00 b7 00 01 00 00 00 ec cf 63 00 00 00 00 00  ..........c.....

How it was before (base set to 0x0, same as 32-bit):

Sections for '/data/local/tmp/build/lib64/debug/libdynamorio.so' (aarch64):
  SectID             Type                   Load Address                             Perm File Off.  File Size  Flags      Section Name
  ------------------ ---------------------- ---------------------------------------  ---- ---------- ---------- ---------- ----------------------------
  0xfffffffffffffffe container              [0x0000007ff77fd000-0x0000007ff77fd270)  r--  0x00000000 0x00000270 0x00000000 libdynamorio.so.PT_LOAD[0]
  0x0000000000000001 code                   [0x0000007ff77fd000-0x0000007ff77fd270)  r-x  0x00001000 0x006aacf0 0x00000006 libdynamorio.so.PT_LOAD[0]..text
  0xfffffffffffffffc container              [0x0000007ff7ea8cf0-0x0000007ff7f9fab8)  r--  0x006abcf0 0x000f6dc8 0x00000000 libdynamorio.so.PT_LOAD[1]

Sections for '[vdso](0x0000007ff77fc000)' (aarch64):
  SectID             Type                   Load Address                             Perm File Off.  File Size  Flags      Section Name
  ------------------ ---------------------- ---------------------------------------  ---- ---------- ---------- ---------- ----------------------------
  0xffffffffffffffff container              [0x0000007ff77fc000-0x0000007ff77fc968)  r-x  0x00000000 0x00000968 0x00000000 [vdso].PT_LOAD[0]


(lldb) memory read 0x0000007ff77fc000
0x7ff77fc000: 7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00  .ELF............
0x7ff77fc010: 03 00 b7 00 01 00 00 00 e0 02 00 00 00 00 00 00  ................
(lldb) memory read 0x0000007ff77fd000
0x7ff77fd000: ff c3 00 d1 fd 7b 02 a9 fd 83 00 91 a0 83 1f f8  .....{..........
0x7ff77fd010: e1 0b 00 f9 e2 07 00 f9 e3 07 00 b9 a0 83 5f f8  .............._.

So here DR identifies 7ff77fc000 as the base, which is where the vDSO is, and so it finds an ELF header, but the wrong one... It's worth noting that 7ff77fd000, where libynamorio.so is supposed to start, contains the same data as in address 0x1000 in the hexdump of libdynamorio.so, implying libdynamorio.so should start at the address 0x1000 before 7ff77fd000, which is where vDSO is.

Setting the base to something else (e.g. 0x71000000 as in the main CMakeLists.txt):

Sections for '/data/local/tmp/build/lib64/debug/libdynamorio.so' (aarch64):
  SectID             Type                   Load Address                             Perm File Off.  File Size  Flags      Section Name
  ------------------ ---------------------- ---------------------------------------  ---- ---------- ---------- ---------- ----------------------------
  0xfffffffffffffffe container              [0x0000007f867fd000-0x0000007f867fd270)  r--  0x00000000 0x00000270 0x00000000 libdynamorio.so.PT_LOAD[0]
  0xfffffffffffffffd container              [0x0000007ff77fd000-0x0000007ff7ea7cf0)  r-x  0x00001000 0x006aacf0 0x00000000 libdynamorio.so.PT_LOAD[1]
  0x0000000000000001 code                   [0x0000007ff77fd000-0x0000007ff7ea7cf0)  r-x  0x00001000 0x006aacf0 0x00000006 libdynamorio.so.PT_LOAD[1]..text
  0xfffffffffffffffc container              [0x0000007ff7ea8cf0-0x0000007ff7f9fab8)  r--  0x006abcf0 0x000f6dc8 0x00000000 libdynamorio.so.PT_LOAD[2]

Sections for '[vdso](0x0000007ff77fc000)' (aarch64):
  SectID             Type                   Load Address                             Perm File Off.  File Size  Flags      Section Name
  ------------------ ---------------------- ---------------------------------------  ---- ---------- ---------- ---------- ----------------------------
  0xffffffffffffffff container              [0x0000007ff77fc000-0x0000007ff77fc968)  r-x  0x00000000 0x00000968 0x00000000 [vdso].PT_LOAD[0]

(lldb) memory read 0x0000007f867fd000
0x7f867fd000: 7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00  .ELF............
0x7f867fd010: 03 00 b7 00 01 00 00 00 ec bf 63 71 00 00 00 00  ..........cq....
(lldb) memory read 0x0000007ff77fc000
0x7ff77fc000: 7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00  .ELF............
0x7ff77fc010: 03 00 b7 00 01 00 00 00 e0 02 00 00 00 00 00 00  ................

So now vDSO and libdynamorio.so don't overlap, but libdynamorio.so finds the ELF header at 7ff77fc000, which is vDSO's

This is under LLDB so the addresses are the same each time, but this fix works without LLDB. It looks like base/Ttext becomes an offset from 7ff77fd000 rather than the actual address, and this only moves the ELF header - the .text stays at 7ff77fd000. So perhaps with a base of 0x0, libdynamorio.so's ELF header is being overwritten by the .text portion of itself...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the code merged here is misleading as it implies libdynamorio is loaded at 0x1000. This needs a comment explaining why 0x1000, and IMHO it should not be set to 0x1000 and instead these other issues resolved: 0x1000 doesn't make sense as it's presumably an invalid address that is always unmapped.

It sounds like the real issue is that the DR code looking for its own ELF header gets it wrong when VDSO is nearby. A way to solve that should be found if possible.

Please file a new issue on the wrong header problem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #7195

else ()
set(preferred_base "0x71000000" CACHE STRING "Preferred library base address")
endif ()
4 changes: 2 additions & 2 deletions make/utils.cmake
View file
Original file line number Diff line number Diff line change
@@ -266,8 +266,8 @@ if (UNIX)
# XXX: this is duplicated in DynamoRIOConfig.cmake

function (set_preferred_base_start_and_end target base set_bounds)
if (ANDROID)
# i#1863: Android's loader doesn't support a non-zero base.
if (ANDROID32)
# i#1863: 32-bit Android's loader doesn't support a non-zero base.
# Turning off SET_PREFERRED_BASE is not sufficient as we get
# a 0x10000 base.
set(base 0)