Re: Fwd: [PATCH] COFF Compliant .ctors and .dtors sections

[Martell Malone <martellmalone at gmail dot com>]

Hey Nick,

>Ah - OK - in which case the original section names are OK.  But it would be
>nice if you included a comment in the crtexe.c source file explaining why
>those particular names are chosen.
>Never trust users :-)
I will update the mingw-w64 patch to reflect this.

I wanted to follow up with an issue.
After some extra testing between mingw-w64 lld and ld it seems only
the following works.

    PROVIDE(___CTOR_LIST__ = .);
    PROVIDE(__CTOR_LIST__ = .);
    LONG (-1);
    KEEP(*(.ctor));
    KEEP(*(SORT_BY_NAME(.ctors.*)));
    LONG (0);
    PROVIDE(___DTOR_LIST__ = .);
    PROVIDE(__DTOR_LIST__ = .);
    LONG (-1);
    KEEP(*(.dtor));
    KEEP(*(SORT_BY_NAME(.dtors.*)));
    LONG (0);

Which would mean removing KEEP (*(.ctors)); and KEEP(*(.dtors));
I don't quite understand the original intention of those two lines.
Wouldn't they be covered by KEEP(*(SORT_BY_NAME(.ctors.*))); and
KEEP(*(SORT_BY_NAME(.dtors.*))); respectively?

With those 2 lines included strangely the dtors section appears before
the ctors section and only the closing LONG(0) ends up in the section.
The following dump is from a simple program that has one constructor
and one destructor attributed function
Bug maybe?

Disassembly of section .dtors:
__DTOR_LIST__:
  404000:       ff ff  <unknown>
  404002:       ff ff  <unknown>
  404004:       ff ff  <unknown>
  404006:       ff ff  <unknown>
.dtors:
  404008:       5b      popq    %rbx
  404009:       15 40 00 00 00  adcl    $64, %eax
  40400e:       00 00   addb    %al, (%rax)
__DTOR_END__:
  404010:       00 00   addb    %al, (%rax)
  404012:       00 00   addb    %al, (%rax)
  404014:       00 00   addb    %al, (%rax)
  404016:       00 00   addb    %al, (%rax)
Disassembly of section .ctors:
__CTOR_END__:
  405000:       00 00   addb    %al, (%rax)
  405002:       00 00   addb    %al, (%rax)
  405004:       00 00   addb    %al, (%rax)
  405006:       00 00   addb    %al, (%rax)

Without those 2 lines the following appears

Disassembly of section .ctors:
__CTOR_LIST__:
  404000:       ff ff  <unknown>
  404002:       ff ff  <unknown>
  404004:       ff ff  <unknown>
  404006:       ff ff  <unknown>
.ctors:
  404008:       40 15 40 00 00 00       adcl    $64, %eax
  40400e:       00 00   addb    %al, (%rax)
__CTOR_END__:
  404010:       00 00   addb    %al, (%rax)
  404012:       00 00   addb    %al, (%rax)
  404014:       00 00   addb    %al, (%rax)
  404016:       00 00   addb    %al, (%rax)
Disassembly of section .dtors:
__DTOR_LIST__:
  405000:       ff ff  <unknown>
  405002:       ff ff  <unknown>
  405004:       ff ff  <unknown>
  405006:       ff ff  <unknown>
.dtors:
  405008:       5b      popq    %rbx
  405009:       15 40 00 00 00  adcl    $64, %eax
  40500e:       00 00   addb    %al, (%rax)
__DTOR_END__:
  405010:       00 00   addb    %al, (%rax)
  405012:       00 00   addb    %al, (%rax)

Which is correct.



>Assignment should work.  Something like this:
>  __image_base__ = __ImageBase;

>Alternatively you could just use the --image-base=<value> linker command
>line option.  This takes care of setting both __image_base__ and __ImageBase
>to <value>.
To have better backwards compatibility for older mingw-w64 builds
assignment would probably be best here.
Would you be able to land this in tree with __ImageBase and
___ImageBase (note extra underscore)
I believe we do both because of an old backward compatibility with
older gcc/mingw toolchains where the wrapping was wrong for having on
x64 but not on x86.
Much like how we do
    PROVIDE(___DTOR_LIST__ = .);
    PROVIDE(__DTOR_LIST__ = .);
even though we have pe and pep in separate files.

Kind Regards
Martell

On Thu, Aug 10, 2017 at 1:24 PM, Nick Clifton <nickc@redhat.com> wrote:
> Hi Martell,
>
>>> In fact to be clearer - and more consistent - I would suggest changing
>>> the names of the start and end constructor sections to be something like:
>>> .ctors$start and .ctors$end.
>>
>> LLD and LINK.exe do not have any notion of a linker script and will
>> organise the section in the following order.
>> .ctors
>> .ctors.AAAAA
>> .ctors.zzz
>
> Ah - OK - in which case the original section names are OK.  But it would be
> nice if you included a comment in the crtexe.c source file explaining why
> those particular names are chosen.
>
>> Technically someone could add extra z's which would then never get
>> called so I am relying on people not doing something so ridiculous.
>
> Never trust users :-)
>
> But I agree - it is particularly unlikely to happen.
>
>>   With that in mind would the following be acceptable to you.
>>
>> ${CONSTRUCTING+
>>      PROVIDE(__CTOR_LIST__ = .);
>>      PROVIDE(___CTOR_LIST__ = .);
>>      LONG(-1);
>>      KEEP (*(.ctors));
>>      KEEP (*(.ctor));
>>      KEEP (*(SORT_BY_NAME(.ctors.*)));
>>      LONG(0);
>>    }
>
> Yup - that works for me.  In fact I have now checked this change into the
> mainline development sources.
>
>> For my question I also wanted to ask while I we are in discussion on
>> the linker scripts how would I make an alias name to __image_base__ to
>> __ImageBase?
>
> Assignment should work.  Something like this:
>
>   __image_base__ = __ImageBase;
>
> Alternatively you could just use the --image-base=<value> linker command
> line option.  This takes care of setting both __image_base__ and __ImageBase
> to <value>.
>
> Cheers
>   Nick
>
>
>
>