[Bug ld/30499] reword "alignment ... is smaller than alignment ..." warning
https://sourceware.org/bugzilla/show_bug.cgi?id=30499 Nick Clifton changed: What|Removed |Added Resolution|--- |FIXED Status|ASSIGNED|RESOLVED --- Comment #11 from Nick Clifton --- Right - I have applied the patch. The PR number is referenced in the sources, which I think should be enough for motivated users to track down. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug ld/30499] reword "alignment ... is smaller than alignment ..." warning
https://sourceware.org/bugzilla/show_bug.cgi?id=30499 --- Comment #10 from cvs-commit at gcc dot gnu.org --- The master branch has been updated by Nick Clifton : https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=064ad3ea5ecbc30de1eb52a22ac73fea1b5dcc5b commit 064ad3ea5ecbc30de1eb52a22ac73fea1b5dcc5b Author: Nick Clifton Date: Wed Jun 7 14:30:13 2023 +0100 Add extra linker warning message about discrepancies between normal and common symbols. PR 30499 bfd * elflink.c (elf_link_add_object_symbols): Add a message indicating that alignment and size discrepancies between the definition of common symbols and normal symbols are serious and should be investigated. ld * testsuite/ld-elfcomm/elfcomm.exp: Update regexps to match new output from the linker. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug ld/30499] reword "alignment ... is smaller than alignment ..." warning
https://sourceware.org/bugzilla/show_bug.cgi?id=30499 --- Comment #9 from Michael Matz --- (In reply to Nick Clifton from comment #7) > Created attachment 14923 [details] > Proposed patch > > Hi Michael, > >OK, what do you think of this patch ? It would change the wording to: > > ld: warning: alignment 32 of normal symbol `com2_' in test2.o is smaller > than 64 used by the common definition in test1.o > ld: warning: NOTE: alignment discrepancies can cause real problems. > Investigation is advised. If you think the verbosity is not a problem I have no problem with it either. But then you could also add "(see PR30499)" somewhere to help with the advised investigation :-) -- You are receiving this mail because: You are on the CC list for the bug.
[Bug ld/30499] reword "alignment ... is smaller than alignment ..." warning
https://sourceware.org/bugzilla/show_bug.cgi?id=30499 Nick Clifton changed: What|Removed |Added Attachment #14923|0 |1 is obsolete|| --- Comment #8 from Nick Clifton --- Created attachment 14924 --> https://sourceware.org/bugzilla/attachment.cgi?id=14924&action=edit Proposed patch Now with a fix for the linker testsuite, and a placement of the new advisory message at the correct place in the code. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug ld/30499] reword "alignment ... is smaller than alignment ..." warning
https://sourceware.org/bugzilla/show_bug.cgi?id=30499 --- Comment #7 from Nick Clifton --- Created attachment 14923 --> https://sourceware.org/bugzilla/attachment.cgi?id=14923&action=edit Proposed patch Hi Michael, OK, what do you think of this patch ? It would change the wording to: ld: warning: alignment 32 of normal symbol `com2_' in test2.o is smaller than 64 used by the common definition in test1.o ld: warning: NOTE: alignment discrepancies can cause real problems. Investigation is advised. Cheers Nick -- You are receiving this mail because: You are on the CC list for the bug.
[Bug ld/30499] reword "alignment ... is smaller than alignment ..." warning
https://sourceware.org/bugzilla/show_bug.cgi?id=30499 Sam James changed: What|Removed |Added CC||sam at gentoo dot org -- You are receiving this mail because: You are on the CC list for the bug.
[Bug ld/30499] reword "alignment ... is smaller than alignment ..." warning
https://sourceware.org/bugzilla/show_bug.cgi?id=30499 Nick Clifton changed: What|Removed |Added Assignee|unassigned at sourceware dot org |nickc at redhat dot com Status|NEW |ASSIGNED --- Comment #5 from Nick Clifton --- (In reply to Michael Matz from comment #4) > % cc ... > ld: warning: alignment 32 of symbol `com2_' in test2.o is smaller than 64 in > test1.o > > Note how it complains only about one, not both, symbols. And further: I think that this is because com2_ is a global symbol, whereas com1_ is local in test2.o and global in test1.o. (If you look you should see two definitions of com1_ in the symbol table...) > So, all in all, ultimately I think: > a) the suggested wording of the warning is wrong, as not achievable in the >general case Presumably you mean the suggested new wording ? Maybe the warning message should explicitly indicate which alignment has been chosen ? > b) the above example shows how the warning might even be regarded as error. >Code that assumes 64-alignment for com1_ and com2_ (as requested by > test1.o) >_will_ break with the generated output and there's no way for the linker >to magically make it work. > > In the interest of backward compatibility and in light of the existence of > -fcommon for C, even though its default changed a couple years back, which > makes mixture of common and data symbols be somewhat common, I'm not actually > suggesting to make this an error, though. Agreed - but it might be worthwhile extending the warning message to emphasize that the discrepancy could cause problems. Cheers Nick -- You are receiving this mail because: You are on the CC list for the bug.
[Bug ld/30499] reword "alignment ... is smaller than alignment ..." warning
https://sourceware.org/bugzilla/show_bug.cgi?id=30499 --- Comment #4 from Michael Matz --- (In reply to Nick Clifton from comment #3) > (In reply to Michael Matz from comment #2) > > Hmm, on reflection this proposed message might not actually be correct. > > Generally one can't just increase the alignment of random data symbols like > > here: they might be part of a larger object with known relative offsets, and > > changing the alignment of such data symbol will then break such knowledge. > > Can this happen ? > > More to the point, if a meta-object does contain sub-objects with their own > symbols, shouldn't the offsets of those sub-objects be computed by > calculating > the difference between the symbol's address and the meta-object's start > address. Rather than just by being pre-calculated to some fixed value ? Sometimes yes. But the more usual case is that the addresses are encoded as section-relative. Especially so if the symbols in question aren't global, but still just so happen to be related (by positioning) to the common symbol that is tried to be moved around. For instance, in this example: % cat test1.s .type com2_,@object .comm com2_,8,64 .quad com2_ .type com1_,@object .comm com1_,8,64 .quad com1_ .section .note.GNU-stack % cat test2.s .globl com2_ .data randomstuff: .quad 1 .align 32 .type com1_, @object .size com1_, 8 com1_: .quad 2 .align 32 .type com2_, @object .size com2_, 8 com2_: .quad 2 foobar: .quad 3 addroffoobar: .quad foobar - randomstuff .section .note.GNU-stack main.c and compile commands as above. Note how I now have two "problematic" common symbols, both constrained to be 64-aligned from test1.s, but actually only getting a 32-alignment in test2.s. Not also that the distance between com1_ and com2_ is basically fixated in test2.s because I encode a distance between sourrounding (non-global) symbols. So, as a whole this .data section from test2.s can move happily around, but of course nothing can be added right _into_ that blob representing test2.o:.data. To wit: % cc ... ld: warning: alignment 32 of symbol `com2_' in test2.o is smaller than 64 in test1.o Note how it complains only about one, not both, symbols. And further: % readelf -sW a.out ... 15: 00404020 0 NOTYPE LOCAL DEFAULT 23 randomstuff ... 17: 00404068 0 NOTYPE LOCAL DEFAULT 23 foobar 18: 00404070 0 NOTYPE LOCAL DEFAULT 23 addroffoobar ... 28: 00404060 8 OBJECT GLOBAL DEFAULT 23 com2_ ... 43: 004040c0 8 OBJECT GLOBAL DEFAULT 25 com1_ % readelf -x .data a.out Hex dump of section '.data': 0x00404000 0x00404010 0x00404020 0100 0x00404030 0x00404040 0200 0x00404050 0x00404060 0200 0300 0x00404070 4800 H... So, it correctly left the distance between foobar and randomstuff at 0x48, both in symbol table and .data content. It achieved the 64-alignment of 'com1_' by letting the begin of test2.o:.data (corresponding also to the symbol 'randomstuff') be at 0x20 within a.out:.data. But that means that com2_ also had to move with it, to .data+0x60. That's _not_ 64-aligned. And especially it can't be made 64-aligned in any way. Either it would break the alignment of com1_ again, or it would change the distance between randomstuff and foobar, which wouldn't be able to rectified as no trace of that is left in the object files (and rightly so!). Curiously it only gave a warning message about com2_, which is the one where it couldn't do any alignment upgrade, but not about com1_ where it actually did change it. So, all in all, ultimately I think: a) the suggested wording of the warning is wrong, as not achievable in the general case b) the above example shows how the warning might even be regarded as error. Code that assumes 64-alignment for com1_ and com2_ (as requested by test1.o) _will_ break with the generated output and there's no way for the linker to magically make it work. In the interest of backward compatibility and in light of the existence of -fcommon for C, even though its default changed a couple years back, which makes mixture of common and data symbols be somewhat common, I'm not actually suggesting to make this an error, though. We could give the suggested wording of the warning in the very specific case where the target alignment is achievable. But the current code doesn't lend itself to that, at the place the warning is given it doesn't really know yet if that symbol can be over-aligned or not. (As shown above b
[Bug ld/30499] reword "alignment ... is smaller than alignment ..." warning
https://sourceware.org/bugzilla/show_bug.cgi?id=30499 Nick Clifton changed: What|Removed |Added CC||nickc at redhat dot com --- Comment #3 from Nick Clifton --- (In reply to Michael Matz from comment #2) > Hmm, on reflection this proposed message might not actually be correct. > Generally one can't just increase the alignment of random data symbols like > here: they might be part of a larger object with known relative offsets, and > changing the alignment of such data symbol will then break such knowledge. Can this happen ? More to the point, if a meta-object does contain sub-objects with their own symbols, shouldn't the offsets of those sub-objects be computed by calculating the difference between the symbol's address and the meta-object's start address. Rather than just by being pre-calculated to some fixed value ? If it is possible however then maybe the message should be: warning: alignment 32 of symbol `com2_' in test2.o changed to 64 to match test1.o warning: beware: this might break code that relies upon the alignment being 32. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug ld/30499] reword "alignment ... is smaller than alignment ..." warning
https://sourceware.org/bugzilla/show_bug.cgi?id=30499 --- Comment #2 from Michael Matz --- Hmm, on reflection this proposed message might not actually be correct. Generally one can't just increase the alignment of random data symbols like here: they might be part of a larger object with known relative offsets, and changing the alignment of such data symbol will then break such knowledge. In _this_ specific case everything works out and the resulting alignment of 'com2_' is 64. But that might not always be possible, which I guess is the reason for the warning in the first place. And changing the wording to suggest that the bigger alignment is actually used always is then misleading. So one point for not changing the warning :) -- You are receiving this mail because: You are on the CC list for the bug.
[Bug ld/30499] reword "alignment ... is smaller than alignment ..." warning
https://sourceware.org/bugzilla/show_bug.cgi?id=30499
--- Comment #1 from Michael Matz ---
Patch would be trivial:
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -5339,7 +5339,7 @@ elf_link_add_object_symbols (bfd *abfd, struct
bfd_link_info *info)
_bfd_error_handler
/* xgettext:c-format */
(_("warning: alignment %u of symbol `%s' in %pB"
-" is smaller than %u in %pB"),
+" changed to %u to match %pB"),
1 << normal_align, name, normal_bfd,
1 << common_align, common_bfd);
}
--
You are receiving this mail because:
You are on the CC list for the bug.
