Skip to content

Conversation

@jdmpapin
Copy link
Contributor

@jdmpapin jdmpapin commented Jul 8, 2021

A label has a null address until its location is determined when it is encountered in the instruction stream during binary encoding. When the compiler generates a jump to a label whose address has not yet been determined, it also generates a label relocation to correct the jump target once the label's address is known after binary encoding. But if for some reason the label's address is never updated, the relocation would silently continue using zero and generate an incorrect jump.

Because such jumps can be hard to debug, this commit adds assertions that verify that the label has at least had some address set, otherwise failing fast. Note that zero cannot be the true address of a label, since we never generate instructions there.

@jdmpapin jdmpapin requested review from 0xdaryl and mstoodle as code owners July 8, 2021 19:10
@jdmpapin
Copy link
Contributor Author

jdmpapin commented Jul 8, 2021

These assertions would have helped a lot with a crash that I debugged recently

@fjeremic fjeremic self-assigned this Jul 12, 2021
A label has a null address until its location is determined when it is
encountered in the instruction stream during binary encoding. When the
compiler generates a jump to a label whose address has not yet been
determined, it also generates a label relocation to correct the jump
target once the label's address is known after binary encoding. But if
for some reason the label's address is never updated, the relocation
would silently continue using zero and generate an incorrect jump.

Because such jumps can be hard to debug, this commit adds assertions
that verify that the label has at least had some address set, otherwise
failing fast. Note that zero cannot be the true address of a label,
since we never generate instructions there.
@jdmpapin jdmpapin force-pushed the label-relo-assert branch from 1d93fb8 to 43323d2 Compare July 12, 2021 19:19
@fjeremic
Copy link
Contributor

Jenkins build all

@fjeremic fjeremic merged commit 5d21339 into eclipse-omr:master Jul 13, 2021
@jdmpapin jdmpapin deleted the label-relo-assert branch July 15, 2021 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants