From: "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
To: binutils@sourceware.org, jbeulich@suse.com
Subject: Non-zero RELA section contents
Date: Sat, 16 Apr 2022 00:44:04 -0400 [thread overview]
Message-ID: <1650070699.hyefqn4i18.none@localhost> (raw)
In-Reply-To: <1650070699.hyefqn4i18.none.ref@localhost>
Hi,
In ELF RELA relocations, the addend is stored in the relocation instead
of the section contents. This was the premise of commit 17c6c3b99156
("x86-64/ELF: clear src_mask for all reloc types"); the value in the
section contents is not used.
I recently encountered a difficult-to-trace bug in 7-Zip, eventually
determined to be caused by JWasm-family assemblers incorrectly putting
the addend in the section contents, and setting the RELA addend to -4.
Previous versions of binutils added the existing value to the relocation
result, producing a working program; new versions of binutils overwrite
it, producing a non-working program.
Now, I want to be clear: while the relevant specifications are not clear
about what the linker should do in this case, they are reasonably clear
that the assembler should not generate this. The assembler is absolutely
wrong.
With that being said, I think it would be a good idea for ld to either
revert to the previous behavior or issue a warning or error when
detecting such malformed object files. I think a warning at the least is
appropriate, because binaries are either silently incorrect on the old
version, or silently incorrect on the new version, and silently
incorrect relocations are extremely hard to diagnose. One argument in
favor of reverting to the previous behavior is it is better to preserve
backwards compatibility in BFD linker when the new behavior is not
clearly superior (e.g. faster or closer to spec). One argument in favor
of the new behavior is that it is consistent with LLD and probably gold.
The following patch adds a warning:
diff --git a/bfd/reloc.c b/bfd/reloc.c
index 5098e0ab09f..aecdb21ec59 100644
--- a/bfd/reloc.c
+++ b/bfd/reloc.c
@@ -1509,6 +1509,11 @@ _bfd_relocate_contents (reloc_howto_type *howto,
relocation >>= (bfd_vma) rightshift;
relocation <<= (bfd_vma) bitpos;
+ if (!howto->src_mask && (x & howto->dst_mask))
+ _bfd_error_handler
+ (_("warning: %pB: existing value for %s relocation is %lx, expected 0"),
+ input_bfd, howto->name, x);
+
/* Add RELOCATION to the right bits of X. */
x = ((x & ~howto->dst_mask)
| (((x & howto->src_mask) + relocation) & howto->dst_mask));
I have tested this patch on a single test file which printed the desired
warning. I believe %lx is probably wrong, but I don't know how to fix
it. I suspect this patch may cause a large number of warnings in certain
cases; however, I think adding a warning here is valid, because the
result is either broken with old binutils or broken with new binutils
(or both), and furthermore, proper assemblers should never generate this
case in the first place.
I think the primary counterargument is that the point of "either broken
with old binutils or broken with new binutils" primarily applies to
x86-64/ELF, and may not apply to other targets. I am insufficiently
familiar with other targets to say whether this is the case. If so, it
may be better to add this warning only for x86-64/ELF.
Thanks,
Alex.
next parent reply other threads:[~2022-04-16 4:44 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1650070699.hyefqn4i18.none.ref@localhost>
2022-04-16 4:44 ` Alex Xu (Hello71) [this message]
2022-04-18 16:01 ` H.J. Lu
2022-04-19 7:18 ` Jan Beulich
2022-04-23 21:45 ` Fangrui Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1650070699.hyefqn4i18.none@localhost \
--to=alex_y_xu@yahoo.ca \
--cc=binutils@sourceware.org \
--cc=jbeulich@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).