[PATCH v3] bfd/{pe,pei}-x86_64: Decrease preferred section alignment from 16 to 4

LIU Hao lh_mouse@126.com
Mon Mar 31 07:34:59 GMT 2025


在 2025-3-31 15:13, Jan Beulich 写道:
> On 30.03.2025 13:43, LIU Hao wrote:
>> The `.refptr` fix has been committed to GCC master
>> (https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=8d42378acdf1a011420ba78685f7a6c79808c881).
> 
> But that means older gcc with a gas patched as suggested here will produce not entirely
> correct code?

x86-64 supports unaligned loads like that. It's read-only memory, so there shouldn't be any issue about 
correctness.

(If it was an atomic variable which could straddle two consecutive cachelines, then it would be incorrect.)


> Looking at that patch (and some of the involved code) I also can't see why its 8-byte
> alignment (and .quad) uniformly. This code is also used for 32-bit, isn't it?

`.refptr.` is not used for x86-32, where (absolute) addresses are representable in 32-bit displacements.

On x86-64 we need that when using `-mcmodel=large`, since RIP-relative addresses only allow 32-bit 
displacements (%rip ± 2GB), which is unable to cover the entire virtual memory.


>> * ld/testsuite/ld-pe/secidx.d
>>
>>      Escape dots in regular expressions.
>>
> 
> This change is entirely unrelated here. Pre-approved if broken out to a
> standalone change.

Yes I think I should move that into a separate patch.


>>    #define COFF_SECTION_ALIGNMENT_ENTRIES \
>>    { COFF_SECTION_NAME_EXACT_MATCH (".bss"), \
>> -  COFF_ALIGNMENT_FIELD_EMPTY, COFF_ALIGNMENT_FIELD_EMPTY, 4 }, \
>> +  COFF_ALIGNMENT_FIELD_EMPTY, COFF_ALIGNMENT_FIELD_EMPTY, 2 }, \
>>    { COFF_SECTION_NAME_PARTIAL_MATCH (".data"), \
>> -  COFF_ALIGNMENT_FIELD_EMPTY, COFF_ALIGNMENT_FIELD_EMPTY, 4 }, \
>> +  COFF_ALIGNMENT_FIELD_EMPTY, COFF_ALIGNMENT_FIELD_EMPTY, 2 }, \
>>    { COFF_SECTION_NAME_PARTIAL_MATCH (".rdata"), \
>> -  COFF_ALIGNMENT_FIELD_EMPTY, COFF_ALIGNMENT_FIELD_EMPTY, 4 }, \
>> +  COFF_ALIGNMENT_FIELD_EMPTY, COFF_ALIGNMENT_FIELD_EMPTY, 2 }, \
>>    { COFF_SECTION_NAME_PARTIAL_MATCH (".text"), \
>> -  COFF_ALIGNMENT_FIELD_EMPTY, COFF_ALIGNMENT_FIELD_EMPTY, 4 }, \
>> +  COFF_ALIGNMENT_FIELD_EMPTY, COFF_ALIGNMENT_FIELD_EMPTY, 2 }, \
>>    { COFF_SECTION_NAME_PARTIAL_MATCH (".idata"), \
>>      COFF_ALIGNMENT_FIELD_EMPTY, COFF_ALIGNMENT_FIELD_EMPTY, 2 }, \
>>    { COFF_SECTION_NAME_EXACT_MATCH (".pdata"), \
> 
> I remain unconvinced that we can do it like this.

LLVM has already been doing that, no?


>> --- a/ld/testsuite/ld-pe/pr26659-weak-undef-sym.d
>> +++ b/ld/testsuite/ld-pe/pr26659-weak-undef-sym.d
>> @@ -13,7 +13,7 @@
>>     *[0-9a-f]+:	48 85 c0             	test   %rax,%rax
>>     *[0-9a-f]+:	74 05                	je     [0-9a-f]+ <foo\+0x1c>
>>     *[0-9a-f]+:	e8 e4 ef [fb]f [fb]f       	call   100000000 <__size_of_stack_reserve__\+0xffe00000>
>> - *[0-9a-f]+:	48 8b 05 ed 0f 00 00 	mov    0xfed\(%rip\),%rax        # [0-9a-f]+ <.refptr.bar2>
>> + *[0-9a-f]+:	48 8b 05 e5 0f 00 00 	mov    0xfe5\(%rip\),%rax        # [0-9a-f]+ <.refptr.bar2>
> 
> Instead of the 0xfe5 imo we also better would use 0x[0-9a-f]+ there. It's
> the <.refptr.bar2> on the far right which is important to match.

This makes sense. `.refptr` can't be referenced outside %rip ± 2GB, otherwise it would be a serious 
trouble; the displacement just doesn't matter.


I request a confirmation about whether we have agreed that this patch with all the changes above is 
acceptable. I wouldn't send a new patch before that, as it just sounds like noise.



-- 
Best regards,
LIU Hao
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://sourceware.org/pipermail/binutils/attachments/20250331/88752269/attachment.sig>


More information about the Binutils mailing list