On 08.03.2022 13:11, Roger Pau Monné wrote:
> On Tue, Mar 08, 2022 at 12:15:04PM +0100, Jan Beulich wrote:
>> On 08.03.2022 11:12, Roger Pau Monné wrote:
>>> On Mon, Mar 07, 2022 at 02:53:32PM +0100, Jan Beulich wrote:
>>>> @@ -179,6 +188,13 @@ SECTIONS
>>>> #endif
>>>> #endif
>>>>
>>>> +#ifndef EFI
>>>> + /* Retain these just for the purpose of possible analysis tools. */
>>>> + DECL_SECTION(.note) {
>>>> + *(.note.*)
>>>> + } PHDR(note) PHDR(text)
>>>
>>> Wouldn't it be enough to place it in the note program header?
>>>
>>> The buildid note is already placed in .rodata, so any remaining notes
>>> don't need to be in a LOAD section?
>>
>> All the notes will be covered by the NOTE phdr. I had this much later
>> in the script originally, but then the NOTE phdr covered large parts of
>> .init.*. Clearly that yields invalid notes, which analysis (or simple
>> dumping) tools wouldn't be happy about. We might be able to add 2nd
>> NOTE phdr, but mkelf32 assumes exactly 2 phdrs if it finds more than
>> one, so changes there would likely be needed then (which I'd like to
>> avoid for the moment). I'm also not sure in how far tools can be
>> expected to look for multiple NOTE phdrs ...
>
> But if we are adding a .note section now we might as well merge it
> with .note.gnu.build-id:
>
> DECL_SECTION(.note) {
> __note_gnu_build_id_start = .;
> *(.note.gnu.build-id)
> __note_gnu_build_id_end = .;
> *(.note.*)
> } PHDR(note) PHDR(text)
>
> And drop the .note.Xen section?
In an ideal world we likely could, yes. But do we know for sure that
nothing recognizes the Xen notes by section name? .note.gnu.build-id
cannot be folded in any event - see the rule for generating note.o,
to be used by xen.efi linking in certain cases.
>>>> +#endif
>>>> +
>>>> _erodata = .;
>>>>
>>>> . = ALIGN(SECTION_ALIGN);
>>>> @@ -266,6 +282,32 @@ SECTIONS
>>>> __ctors_end = .;
>>>> } PHDR(text)
>>>>
>>>> +#ifndef EFI
>>>> + /*
>>>> + * With --orphan-sections=warn (or =error) we need to handle certain
>>>> linker
>>>> + * generated sections. These are all expected to be empty; respective
>>>> + * ASSERT()s can be found towards the end of this file.
>>>> + */
>>>> + DECL_SECTION(.got) {
>>>> + *(.got)
>>>> + } PHDR(text)
>>>> + DECL_SECTION(.got.plt) {
>>>> + *(.got.plt)
>>>> + } PHDR(text)
>>>> + DECL_SECTION(.igot.plt) {
>>>> + *(.igot.plt)
>>>> + } PHDR(text)
>>>> + DECL_SECTION(.iplt) {
>>>> + *(.iplt)
>>>> + } PHDR(text)
>>>> + DECL_SECTION(.plt) {
>>>> + *(.plt)
>>>> + } PHDR(text)
>>>> + DECL_SECTION(.rela) {
>>>> + *(.rela.*)
>>>> + } PHDR(text)
>>>
>>> Why do you need to explicitly place those in the text program header?
>>
>> I guess that's largely for consistency with all other directives. With the
>> assertions that these need to be empty, we might get away without, as long
>> as no linker would decide to set up another zero-size phdr for them.
>
> We already set the debug sections to not be part of any program header
> and seem to get away with it. I'm not sure how different the sections
> handled below would be, linkers might indeed want to place them
> regardless?
Simply because I don't know I'd like to be on the safe side. Debug sections
can't really be taken as reference: At least GNU ld heavily special-cases
them anyway.
> If so it might be good to add a comment that while those should be
> empty (and thus don't end up in any program header) we assign them to
> the text one in order to avoid the linker from creating a new program
> header for them.
I'll add a sentence to the comment I'm already adding here.
Jan
