This is an archive of the discontinued LLVM Phabricator instance.

Add a relocation to ObjectFileELF::ApplyRelocations and a test
ClosedPublic

Authored by lanza on Aug 31 2018, 9:14 PM.

Details

Summary

pcm files can end up being processed by lldb with relocations to be
made for the .debug_info section. When a R_AARCH64_ABS64 relocation
was required lldb would hit an assert(false) and die.

Add R_AARCH64_ABS64 relocations to the S+A 64 bit width code path. Add
a test for R_AARCH64_ABS64 and R_AARCH64_ABS32 .rela.debug_info
relocations in a pcm file.

Event Timeline

lanza created this revision.Aug 31 2018, 9:14 PM
lanza updated this revision to Diff 163617.Aug 31 2018, 9:18 PM

move macros

lanza edited reviewers, added: sas, xiaobai, davide; removed: javed.absar, espindola.Aug 31 2018, 9:18 PM

@davide Hey Davide, could you take a look at this?

davide accepted this revision.EditedSep 14 2018, 8:34 AM
davide added subscribers: zturner, labath.

LGTM. @zturner / @labath might have opinions.

This revision is now accepted and ready to land.Sep 14 2018, 8:34 AM

Also, thanks for taking the time to do this right :)

I am not sure which thread it was, but I think I mentioned somewhere that lldb-test would be better suited to write this kind of a test. bin/lldb-test object-file -contents should print the contents of the sections, which should reflect the relocations that have been applied to them. Then you could write the test as:
yaml2obj | lldb-test object-file -contents | FileCheck

Have you tried this approach?

Also, thanks for taking the time to do this right :)

No problem, it was a great learning experience, too.

I am not sure which thread it was, but I think I mentioned somewhere that lldb-test would be better suited to write this kind of a test. bin/lldb-test object-file -contents should print the contents of the sections, which should reflect the relocations that have been applied to them. Then you could write the test as:
yaml2obj | lldb-test object-file -contents | FileCheck

Have you tried this approach?

Just from a quick scan that seems like it would work. I'll make sure look there first from now on.

This revision was automatically updated to reflect the committed changes.
davide added a comment.EditedNov 6 2018, 8:09 AM

@lanza Hey Nathan, it looks like your test exposed an UB in ELFObjectFileReader. May I ask you to take a look?

******************** TEST 'lldb-Unit :: ObjectFile/ELF/./ObjectFileELFTests/ObjectFileELFTest.TestAARCH64Relocations' FAILED ********************
Note: Google Test filter = ObjectFileELFTest.TestAARCH64Relocations
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ObjectFileELFTest
[ RUN      ] ObjectFileELFTest.TestAARCH64Relocations
/Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11: runtime error: store to misaligned address 0x61f000001526 for type 'uint32_t' (aka 'unsigned int'), which requires 4 byte alignment
0x61f000001526: note: pointer points here
 00 00 04 00 00 00  00 00 08 01 00 00 00 00  0c 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00
             ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11 in
davide added a comment.Nov 6 2018, 9:15 AM

@lanza Hey Nathan, it looks like your test exposed an UB in ELFObjectFileReader. May I ask you to take a look?

  • TEST 'lldb-Unit :: ObjectFile/ELF/./ObjectFileELFTests/ObjectFileELFTest.TestAARCH64Relocations' FAILED ****

Note: Google Test filter = ObjectFileELFTest.TestAARCH64Relocations
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ObjectFileELFTest
[ RUN ] ObjectFileELFTest.TestAARCH64Relocations
/Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11: runtime error: store to misaligned address 0x61f000001526 for type 'uint32_t' (aka 'unsigned int'), which requires 4 byte alignment
0x61f000001526: note: pointer points here
00 00 04 00 00 00 00 00 08 01 00 00 00 00 0c 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

^

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11 in

@lanza Hey Nathan, it looks like your test exposed an UB in ELFObjectFileReader. May I ask you to take a look?

******************** TEST 'lldb-Unit :: ObjectFile/ELF/./ObjectFileELFTests/ObjectFileELFTest.TestAARCH64Relocations' FAILED ********************
Note: Google Test filter = ObjectFileELFTest.TestAARCH64Relocations
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ObjectFileELFTest
[ RUN      ] ObjectFileELFTest.TestAARCH64Relocations
/Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11: runtime error: store to misaligned address 0x61f000001526 for type 'uint32_t' (aka 'unsigned int'), which requires 4 byte alignment
0x61f000001526: note: pointer points here
 00 00 04 00 00 00  00 00 08 01 00 00 00 00  0c 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00
             ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11 in

Went ahead and fixed this in r346244. A post commit review would be appreciated.

Davide

lanza added a comment.Nov 6 2018, 12:44 PM

LGTM and the test case passes. Thanks a lot, Davide!