Skip to content

Commit

Permalink
[LLD][RISCV] Report error for unsatisfiable RISCV_ALIGN
Browse files Browse the repository at this point in the history
If we have a RISCV_ALIGN relocation which can't be satisified with the
available space provided, report an error rather than silently continuing
with a corrupt state.

For context, llvm#73977 fixes an LLD bug which can cause this effect, but that's not the only source of such cases.

Another is our hard-to-fix set of LTO problems.  We can have a single function
which was compiled without C in an otherwise entirely C module.  Until we have
all of the mapping symbols and related mechanisms implemented, this case can
continue to arise.

I think it's very important from a user interface perspective to have
non-assertion builds report an error in this case. If we don't report an
error here, we can crash the linker (due to the fatal error at the bottom
of the function), or if we're less lucky silently produce a malformed binary.

There's a couple of known defects with this patch.

First, there's no test case.  I don't know how to write a stable test
case for this that doesn't involve hex editting an object file, or abusing
the LTO bug that we hope to fix.

Second, this will report an error on each relax iteration.  I explored
trying to report an error only once after relaxation, but ended up deciding
I didn't have the context to implement it safely.

I would be thrilled if someone more knowledgeable of this code wants to
write a better version of this patch, but in the meantime, I believe we
should land this to address the user experience problem described above.
  • Loading branch information
preames committed Dec 1, 2023
1 parent ae485e6 commit 0524354
Showing 1 changed file with 8 additions and 2 deletions.
10 changes: 8 additions & 2 deletions lld/ELF/Arch/RISCV.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -686,8 +686,14 @@ static bool relax(InputSection &sec) {
const uint64_t align = PowerOf2Ceil(r.addend + 2);
// All bytes beyond the alignment boundary should be removed.
remove = nextLoc - ((loc + align - 1) & -align);
assert(static_cast<int32_t>(remove) >= 0 &&
"R_RISCV_ALIGN needs expanding the content");
// If we can't satisfy this alignment, we've found a bad input.
if (LLVM_UNLIKELY(static_cast<int32_t>(remove) < 0)) {
error(getErrorLocation((const uint8_t*)loc) +
"insufficient padding bytes for " + lld::toString(r.type) +
": " + Twine(r.addend) + " bytes available "
+ "for requested alignment of " + Twine(align) + " bytes");
remove = 0;
}
break;
}
case R_RISCV_CALL:
Expand Down

0 comments on commit 0524354

Please sign in to comment.