This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj/elf] - Fix the PREL31 relocation computation used for dumping arm32 unwind info (-u).
ClosedPublic

Authored by grimar on Sep 28 2020, 5:24 AM.

Details

Summary

This is a part of https://bugs.llvm.org/show_bug.cgi?id=47581.

We have the following computation:

(1) uint64_t Location = Address & 0x7fffffff;
(2) if (Location & 0x04000000)
(3)   Location |= (uint64_t) ~0x7fffffff;
(4) return Location + Place;

At line 2 there is a mistype. The constant should be 0x40000000,
not 0x04000000, because the intention here is to sign extend the Location,
which is a 31 bit signed value.

Diff Detail

Event Timeline

grimar created this revision.Sep 28 2020, 5:24 AM
grimar requested review of this revision.Sep 28 2020, 5:24 AM
psmith accepted this revision.Sep 28 2020, 5:47 AM

LGTM. Agree that it should be bit31 that is sign extended. Apologies for not catching this last time.

This revision is now accepted and ready to land.Sep 28 2020, 5:47 AM

LGTM. Agree that it should be bit31 that is sign extended. Apologies for not catching this last time.

No worries. This and few other issues are/were mentioned in comments on the bug page. They are about 6 years old,
I guess nobody tried to heavily use "llvm-readobj -u" before. I am going to try to fix all of them. Thanks for quick review!

MaskRay added a comment.EditedSep 28 2020, 11:46 AM

LGTM

LGTM. Agree that it should be bit31 that is sign extended. Apologies for not catching this last time.

No worries. This and few other issues are/were mentioned in comments on the bug page. They are about 6 years old,
I guess nobody tried to heavily use "llvm-readobj -u" before. I am going to try to fix all of them. Thanks for quick review!

Thanks for fixing this! I tend to agree with nobody tried to heavily use "llvm-readobj -u" before (not nobody but definitely not many (Android folks noticed a crash last year))..

I tend to agree with nobody tried to heavily use "llvm-readobj -u" before (not nobody but definitely not many (Android folks noticed a crash last year))..

There is one more issue that exist in this area: currently llvm-readelf produce the same LLVM-style output as llvm-readobj does.
We probably want to implement the proper GNU style output.