[PATCH] gold: Add --pack-dyn-relocs=relr for arm/aarch64/x86-64 [PR 28601]

Fangrui Song i@maskray.me
Thu Nov 25 02:08:24 GMT 2021


Thanks for review!

On 2021-11-24, Cary Coutant wrote:
>>    void
>>    set_current_data_size_for_child(off_t data_size)
>>    {
>> -    gold_assert(!this->is_data_size_valid_);
>>      this->data_size_ = data_size;
>>    }
>
>Why was it necessary to remove this assert? In each relaxation pass,
>reset_address_and_file_offset() should have been called, which would
>set this flag to false. It shouldn't get set again until the size is
>finalized, so I think you may be computing the section size too late,
>which could lead to other problems.

With the assert, gold/ld-new --pack-dyn-relocs=relr -pie relr_x86_64.o -o relr_x86_64
will fail: internal error in set_current_data_size_for_child, at ../../../gold/output.h:561
set_current_data_size_for_child may be called multiple times.

>Also, beware of spaces before parens in function calls. The C++ style
>is to have no space.

Ah, my Emacs added a space for the "gnu" style.
Spot 3 places with an excess space.
Fixed in https://github.com/MaskRay/binutils-gdb/tree/gold-relr

>I'm still reviewing the patch, and trying to decide whether adding a
>relaxation pass to x86 is really necessary.
>
>-cary

A relaxation pass is needed even for an arch which usually doesn't need
stub groups(veneers, range extension thunks): .relr.dyn size may affect
addresses of following sections. The address change can affect a symbol
value and then affect the address of .relr.dyn (if a linker script
feature like the output section address is used).


More information about the Binutils mailing list