-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[JITLink] Include target addend in out-of-range error #145423
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.
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
Conversation
When JITLink reports an out-of-range error, the underlying reason could be hidden from the user if it's due to an excessively large target addend. Add non-zero target addend to the message for clarity.
1b8b6f8
to
1417046
Compare
This is a nice improvement. With the proposed message the reader still needs to do the target-symbol + addend arithmetic to arrive at the actual out-of- ErrStream << "In graph " << G.getName() << ", section " << Sec.getName()
<< ": relocation target "
<< formatv("{0:x}", E.getTarget().getAddress() + E.getAddend())
<< "(";
if (E.getTarget().hasName())
ErrStream << E.getTarget().getName();
else
ErrStream << "<anonymous symbol>";
if (E.getAddend()) {
// Target address includes non-zero added, so break down the arithmetic.
ErrStream << formatv(":{0:x}", E.getTarget().getAddress()) << " + addend"
<< formatv(":{0:x}", E.getAddend());
}
ErrStream << ") is out of range of " << G.getEdgeKindName(E.getKind())
<< " at address " << formatv("{0:x}", E.getTarget().getAddress()); So error messages would look like: "In graph foo.o, section __text: relocation target 0x10000000 (main) is out of range..." |
Sounds good. I placed "addend" in "<>" to avoid confusion with regular symbol names.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually now that I look at it I like your initial version where the addend was anonymous:
(main:0x10000000 + 0x1000)
-- it's not meaningful to add symbol addresses, so I automatically read the second part as an addend.
If you want to drop the 'addend' bit go for it, but LGTM either way.
Thanks for working on this!
This reverts commit b03845c.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/186/builds/10137 Here is the relevant piece of the build log for the reference
|
When JITLink reports an out-of-range error, the underlying reason could be hidden from the user if it's due to an excessively large target addend. Add non-zero target addend to the message for clarity.
When JITLink reports an out-of-range error, the underlying reason could be hidden from the user if it's due to an excessively large target addend. Add non-zero target addend to the message for clarity.
When JITLink reports an out-of-range error, the underlying reason could be hidden from the user if it's due to an excessively large target addend. Add non-zero target addend to the message for clarity.