* [PATCH] Avoid padding in _init and _fini. [BZ #31042]
@ 2023-11-21 12:33 Stefan Liebler
2023-11-21 13:10 ` Adhemerval Zanella Netto
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Liebler @ 2023-11-21 12:33 UTC (permalink / raw)
To: libc-alpha; +Cc: rui314, schwab, Stefan Liebler
The linker just concatenates the .init and .fini sections which
results in the complete _init and _fini functions. If needed the
linker adds padding bytes due to an alignment. GNU ld is adding
NOPs, which is fine. But e.g. mold is adding traps which results
in broken _init and _fini functions.
Thus this patch removes the alignment in .init and .fini sections
in crtn.S files.
We keep the 4 byte function alignment in crti.S files. As the
assembler now also outputs the start of _init and _fini functions
as multiples of 4 byte, it perhaps has to fill it. Although GNU as
is using NOPs here, to be sure, we just keep the alignment with
0x07 (=NOPs) at the end of crti.S.
In order to avoid an obvious NOP slide in _fini, this patch also
uses an lg instead of lgr instruction. Then the emitted instructions
needs a multiple of 4 bytes.
---
sysdeps/s390/s390-64/crti.S | 2 +-
sysdeps/s390/s390-64/crtn.S | 2 --
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/sysdeps/s390/s390-64/crti.S b/sysdeps/s390/s390-64/crti.S
index 11ab75e8d9..4c8246da26 100644
--- a/sysdeps/s390/s390-64/crti.S
+++ b/sysdeps/s390/s390-64/crti.S
@@ -85,7 +85,7 @@ _init:
.type _fini,@function
_fini:
stmg %r6,%r15,48(%r15)
- lgr %r1,%r15
+ lg %r1,120(%r15)
aghi %r15,-160
stg %r1,0(%r15)
larl %r12,_GLOBAL_OFFSET_TABLE_
diff --git a/sysdeps/s390/s390-64/crtn.S b/sysdeps/s390/s390-64/crtn.S
index 0eabcb346c..6bb1bc9dcf 100644
--- a/sysdeps/s390/s390-64/crtn.S
+++ b/sysdeps/s390/s390-64/crtn.S
@@ -37,13 +37,11 @@
corresponding to the prologues in crti.S. */
.section .init
- .align 4
lg %r4,272(%r15)
lmg %r6,%r15,208(%r15)
br %r4
.section .fini
- .align 4
lg %r4,272(%r15)
lmg %r6,%r15,208(%r15)
br %r4
--
2.41.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Avoid padding in _init and _fini. [BZ #31042]
2023-11-21 12:33 [PATCH] Avoid padding in _init and _fini. [BZ #31042] Stefan Liebler
@ 2023-11-21 13:10 ` Adhemerval Zanella Netto
2023-11-21 13:20 ` Rui Ueyama
0 siblings, 1 reply; 6+ messages in thread
From: Adhemerval Zanella Netto @ 2023-11-21 13:10 UTC (permalink / raw)
To: Stefan Liebler, libc-alpha; +Cc: rui314, schwab
On 21/11/23 09:33, Stefan Liebler wrote:
> The linker just concatenates the .init and .fini sections which
> results in the complete _init and _fini functions. If needed the
> linker adds padding bytes due to an alignment. GNU ld is adding
> NOPs, which is fine. But e.g. mold is adding traps which results
> in broken _init and _fini functions.
Shouldn't mold follow GNU ld and fill with NOPs? Otherwise, mold will
only be able to link glibc installations with this fix.
>
> Thus this patch removes the alignment in .init and .fini sections
> in crtn.S files.
>
> We keep the 4 byte function alignment in crti.S files. As the
> assembler now also outputs the start of _init and _fini functions
> as multiples of 4 byte, it perhaps has to fill it. Although GNU as
> is using NOPs here, to be sure, we just keep the alignment with
> 0x07 (=NOPs) at the end of crti.S.
>
> In order to avoid an obvious NOP slide in _fini, this patch also
> uses an lg instead of lgr instruction. Then the emitted instructions
> needs a multiple of 4 bytes.
> ---
> sysdeps/s390/s390-64/crti.S | 2 +-
> sysdeps/s390/s390-64/crtn.S | 2 --
> 2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/sysdeps/s390/s390-64/crti.S b/sysdeps/s390/s390-64/crti.S
> index 11ab75e8d9..4c8246da26 100644
> --- a/sysdeps/s390/s390-64/crti.S
> +++ b/sysdeps/s390/s390-64/crti.S
> @@ -85,7 +85,7 @@ _init:
> .type _fini,@function
> _fini:
> stmg %r6,%r15,48(%r15)
> - lgr %r1,%r15
> + lg %r1,120(%r15)
> aghi %r15,-160
> stg %r1,0(%r15)
> larl %r12,_GLOBAL_OFFSET_TABLE_
> diff --git a/sysdeps/s390/s390-64/crtn.S b/sysdeps/s390/s390-64/crtn.S
> index 0eabcb346c..6bb1bc9dcf 100644
> --- a/sysdeps/s390/s390-64/crtn.S
> +++ b/sysdeps/s390/s390-64/crtn.S
> @@ -37,13 +37,11 @@
> corresponding to the prologues in crti.S. */
>
> .section .init
> - .align 4
> lg %r4,272(%r15)
> lmg %r6,%r15,208(%r15)
> br %r4
>
> .section .fini
> - .align 4
> lg %r4,272(%r15)
> lmg %r6,%r15,208(%r15)
> br %r4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Avoid padding in _init and _fini. [BZ #31042]
2023-11-21 13:10 ` Adhemerval Zanella Netto
@ 2023-11-21 13:20 ` Rui Ueyama
2023-11-21 15:47 ` Stefan Liebler
0 siblings, 1 reply; 6+ messages in thread
From: Rui Ueyama @ 2023-11-21 13:20 UTC (permalink / raw)
To: Adhemerval Zanella Netto; +Cc: Stefan Liebler, libc-alpha, schwab
On Tue, Nov 21, 2023 at 10:10 PM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 21/11/23 09:33, Stefan Liebler wrote:
> > The linker just concatenates the .init and .fini sections which
> > results in the complete _init and _fini functions. If needed the
> > linker adds padding bytes due to an alignment. GNU ld is adding
> > NOPs, which is fine. But e.g. mold is adding traps which results
> > in broken _init and _fini functions.
>
> Shouldn't mold follow GNU ld and fill with NOPs? Otherwise, mold will
> only be able to link glibc installations with this fix.
Well, we do follow GNU ld because otherwise we can't link working
executables. That being said, eliminating the dependency to this
particular GNU ld's behavior is I think generally a good idea,
especially given that only .init and .fini depend on it and it is easy
to eliminate it from them.
> >
> > Thus this patch removes the alignment in .init and .fini sections
> > in crtn.S files.
> >
> > We keep the 4 byte function alignment in crti.S files. As the
> > assembler now also outputs the start of _init and _fini functions
> > as multiples of 4 byte, it perhaps has to fill it. Although GNU as
> > is using NOPs here, to be sure, we just keep the alignment with
> > 0x07 (=NOPs) at the end of crti.S.
> >
> > In order to avoid an obvious NOP slide in _fini, this patch also
> > uses an lg instead of lgr instruction. Then the emitted instructions
> > needs a multiple of 4 bytes.
> > ---
> > sysdeps/s390/s390-64/crti.S | 2 +-
> > sysdeps/s390/s390-64/crtn.S | 2 --
> > 2 files changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/sysdeps/s390/s390-64/crti.S b/sysdeps/s390/s390-64/crti.S
> > index 11ab75e8d9..4c8246da26 100644
> > --- a/sysdeps/s390/s390-64/crti.S
> > +++ b/sysdeps/s390/s390-64/crti.S
> > @@ -85,7 +85,7 @@ _init:
> > .type _fini,@function
> > _fini:
> > stmg %r6,%r15,48(%r15)
> > - lgr %r1,%r15
> > + lg %r1,120(%r15)
> > aghi %r15,-160
> > stg %r1,0(%r15)
> > larl %r12,_GLOBAL_OFFSET_TABLE_
> > diff --git a/sysdeps/s390/s390-64/crtn.S b/sysdeps/s390/s390-64/crtn.S
> > index 0eabcb346c..6bb1bc9dcf 100644
> > --- a/sysdeps/s390/s390-64/crtn.S
> > +++ b/sysdeps/s390/s390-64/crtn.S
> > @@ -37,13 +37,11 @@
> > corresponding to the prologues in crti.S. */
> >
> > .section .init
> > - .align 4
> > lg %r4,272(%r15)
> > lmg %r6,%r15,208(%r15)
> > br %r4
> >
> > .section .fini
> > - .align 4
> > lg %r4,272(%r15)
> > lmg %r6,%r15,208(%r15)
> > br %r4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Avoid padding in _init and _fini. [BZ #31042]
2023-11-21 13:20 ` Rui Ueyama
@ 2023-11-21 15:47 ` Stefan Liebler
2023-11-21 15:57 ` Adhemerval Zanella Netto
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Liebler @ 2023-11-21 15:47 UTC (permalink / raw)
To: Rui Ueyama, Adhemerval Zanella Netto; +Cc: libc-alpha, schwab
On 21.11.23 14:20, Rui Ueyama wrote:
> On Tue, Nov 21, 2023 at 10:10 PM Adhemerval Zanella Netto
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 21/11/23 09:33, Stefan Liebler wrote:
>>> The linker just concatenates the .init and .fini sections which
>>> results in the complete _init and _fini functions. If needed the
>>> linker adds padding bytes due to an alignment. GNU ld is adding
>>> NOPs, which is fine. But e.g. mold is adding traps which results
>>> in broken _init and _fini functions.
>>
>> Shouldn't mold follow GNU ld and fill with NOPs? Otherwise, mold will
>> only be able to link glibc installations with this fix.
>
I also think that mold should follow GNU ld as a user-object files with
a .init/.fini-section might also have an alignment which would result in
a broken _init/_fini function - of course not only on s390x.
> Well, we do follow GNU ld because otherwise we can't link working
> executables. That being said, eliminating the dependency to this
> particular GNU ld's behavior is I think generally a good idea,
> especially given that only .init and .fini depend on it and it is easy
> to eliminate it from them.
>
But we don't need the alignment in crtn.S. s390x (64bit) is the only
crtn.S file with an alignment. As said, it is needed and also not
present in s390 (31bit).
If a linker e.g. wants to add a hardening-flag or switch defaults in
future, s390x will behave the same here as all other architectures.
>>>
>>> Thus this patch removes the alignment in .init and .fini sections
>>> in crtn.S files.
>>>
>>> We keep the 4 byte function alignment in crti.S files. As the
>>> assembler now also outputs the start of _init and _fini functions
>>> as multiples of 4 byte, it perhaps has to fill it. Although GNU as
>>> is using NOPs here, to be sure, we just keep the alignment with
>>> 0x07 (=NOPs) at the end of crti.S.
>>>
>>> In order to avoid an obvious NOP slide in _fini, this patch also
>>> uses an lg instead of lgr instruction. Then the emitted instructions
>>> needs a multiple of 4 bytes.
>>> ---
>>> sysdeps/s390/s390-64/crti.S | 2 +-
>>> sysdeps/s390/s390-64/crtn.S | 2 --
>>> 2 files changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/sysdeps/s390/s390-64/crti.S b/sysdeps/s390/s390-64/crti.S
>>> index 11ab75e8d9..4c8246da26 100644
>>> --- a/sysdeps/s390/s390-64/crti.S
>>> +++ b/sysdeps/s390/s390-64/crti.S
>>> @@ -85,7 +85,7 @@ _init:
>>> .type _fini,@function
>>> _fini:
>>> stmg %r6,%r15,48(%r15)
>>> - lgr %r1,%r15
>>> + lg %r1,120(%r15)
>>> aghi %r15,-160
>>> stg %r1,0(%r15)
>>> larl %r12,_GLOBAL_OFFSET_TABLE_
>>> diff --git a/sysdeps/s390/s390-64/crtn.S b/sysdeps/s390/s390-64/crtn.S
>>> index 0eabcb346c..6bb1bc9dcf 100644
>>> --- a/sysdeps/s390/s390-64/crtn.S
>>> +++ b/sysdeps/s390/s390-64/crtn.S
>>> @@ -37,13 +37,11 @@
>>> corresponding to the prologues in crti.S. */
>>>
>>> .section .init
>>> - .align 4
>>> lg %r4,272(%r15)
>>> lmg %r6,%r15,208(%r15)
>>> br %r4
>>>
>>> .section .fini
>>> - .align 4
>>> lg %r4,272(%r15)
>>> lmg %r6,%r15,208(%r15)
>>> br %r4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Avoid padding in _init and _fini. [BZ #31042]
2023-11-21 15:47 ` Stefan Liebler
@ 2023-11-21 15:57 ` Adhemerval Zanella Netto
2023-11-30 12:36 ` Stefan Liebler
0 siblings, 1 reply; 6+ messages in thread
From: Adhemerval Zanella Netto @ 2023-11-21 15:57 UTC (permalink / raw)
To: Stefan Liebler, Rui Ueyama; +Cc: libc-alpha, schwab
On 21/11/23 12:47, Stefan Liebler wrote:
> On 21.11.23 14:20, Rui Ueyama wrote:
>> On Tue, Nov 21, 2023 at 10:10 PM Adhemerval Zanella Netto
>> <adhemerval.zanella@linaro.org> wrote:
>>>
>>>
>>>
>>> On 21/11/23 09:33, Stefan Liebler wrote:
>>>> The linker just concatenates the .init and .fini sections which
>>>> results in the complete _init and _fini functions. If needed the
>>>> linker adds padding bytes due to an alignment. GNU ld is adding
>>>> NOPs, which is fine. But e.g. mold is adding traps which results
>>>> in broken _init and _fini functions.
>>>
>>> Shouldn't mold follow GNU ld and fill with NOPs? Otherwise, mold will
>>> only be able to link glibc installations with this fix.
>>
> I also think that mold should follow GNU ld as a user-object files with
> a .init/.fini-section might also have an alignment which would result in
> a broken _init/_fini function - of course not only on s390x.
Fair enough, from the commit message I had the impression that this change
was motivated to enable mold support.
>
>> Well, we do follow GNU ld because otherwise we can't link working
>> executables. That being said, eliminating the dependency to this
>> particular GNU ld's behavior is I think generally a good idea,
>> especially given that only .init and .fini depend on it and it is easy
>> to eliminate it from them.
>>
> But we don't need the alignment in crtn.S. s390x (64bit) is the only
> crtn.S file with an alignment. As said, it is needed and also not
> present in s390 (31bit).
>
> If a linker e.g. wants to add a hardening-flag or switch defaults in
> future, s390x will behave the same here as all other architectures.
>
>>>>
>>>> Thus this patch removes the alignment in .init and .fini sections
>>>> in crtn.S files.
>>>>
>>>> We keep the 4 byte function alignment in crti.S files. As the
>>>> assembler now also outputs the start of _init and _fini functions
>>>> as multiples of 4 byte, it perhaps has to fill it. Although GNU as
>>>> is using NOPs here, to be sure, we just keep the alignment with
>>>> 0x07 (=NOPs) at the end of crti.S.
>>>>
>>>> In order to avoid an obvious NOP slide in _fini, this patch also
>>>> uses an lg instead of lgr instruction. Then the emitted instructions
>>>> needs a multiple of 4 bytes.
>>>> ---
>>>> sysdeps/s390/s390-64/crti.S | 2 +-
>>>> sysdeps/s390/s390-64/crtn.S | 2 --
>>>> 2 files changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/sysdeps/s390/s390-64/crti.S b/sysdeps/s390/s390-64/crti.S
>>>> index 11ab75e8d9..4c8246da26 100644
>>>> --- a/sysdeps/s390/s390-64/crti.S
>>>> +++ b/sysdeps/s390/s390-64/crti.S
>>>> @@ -85,7 +85,7 @@ _init:
>>>> .type _fini,@function
>>>> _fini:
>>>> stmg %r6,%r15,48(%r15)
>>>> - lgr %r1,%r15
>>>> + lg %r1,120(%r15)
>>>> aghi %r15,-160
>>>> stg %r1,0(%r15)
>>>> larl %r12,_GLOBAL_OFFSET_TABLE_
>>>> diff --git a/sysdeps/s390/s390-64/crtn.S b/sysdeps/s390/s390-64/crtn.S
>>>> index 0eabcb346c..6bb1bc9dcf 100644
>>>> --- a/sysdeps/s390/s390-64/crtn.S
>>>> +++ b/sysdeps/s390/s390-64/crtn.S
>>>> @@ -37,13 +37,11 @@
>>>> corresponding to the prologues in crti.S. */
>>>>
>>>> .section .init
>>>> - .align 4
>>>> lg %r4,272(%r15)
>>>> lmg %r6,%r15,208(%r15)
>>>> br %r4
>>>>
>>>> .section .fini
>>>> - .align 4
>>>> lg %r4,272(%r15)
>>>> lmg %r6,%r15,208(%r15)
>>>> br %r4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Avoid padding in _init and _fini. [BZ #31042]
2023-11-21 15:57 ` Adhemerval Zanella Netto
@ 2023-11-30 12:36 ` Stefan Liebler
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Liebler @ 2023-11-30 12:36 UTC (permalink / raw)
To: Adhemerval Zanella Netto, Rui Ueyama; +Cc: libc-alpha, schwab
I've just committed the patch and closed the bugzilla
On 21.11.23 16:57, Adhemerval Zanella Netto wrote:
>
>
> On 21/11/23 12:47, Stefan Liebler wrote:
>> On 21.11.23 14:20, Rui Ueyama wrote:
>>> On Tue, Nov 21, 2023 at 10:10 PM Adhemerval Zanella Netto
>>> <adhemerval.zanella@linaro.org> wrote:
>>>>
>>>>
>>>>
>>>> On 21/11/23 09:33, Stefan Liebler wrote:
>>>>> The linker just concatenates the .init and .fini sections which
>>>>> results in the complete _init and _fini functions. If needed the
>>>>> linker adds padding bytes due to an alignment. GNU ld is adding
>>>>> NOPs, which is fine. But e.g. mold is adding traps which results
>>>>> in broken _init and _fini functions.
>>>>
>>>> Shouldn't mold follow GNU ld and fill with NOPs? Otherwise, mold will
>>>> only be able to link glibc installations with this fix.
>>>
>> I also think that mold should follow GNU ld as a user-object files with
>> a .init/.fini-section might also have an alignment which would result in
>> a broken _init/_fini function - of course not only on s390x.
>
> Fair enough, from the commit message I had the impression that this change
> was motivated to enable mold support.
>
>>
>>> Well, we do follow GNU ld because otherwise we can't link working
>>> executables. That being said, eliminating the dependency to this
>>> particular GNU ld's behavior is I think generally a good idea,
>>> especially given that only .init and .fini depend on it and it is easy
>>> to eliminate it from them.
>>>
>> But we don't need the alignment in crtn.S. s390x (64bit) is the only
>> crtn.S file with an alignment. As said, it is needed and also not
>> present in s390 (31bit).
>>
>> If a linker e.g. wants to add a hardening-flag or switch defaults in
>> future, s390x will behave the same here as all other architectures.
>>
>>>>>
>>>>> Thus this patch removes the alignment in .init and .fini sections
>>>>> in crtn.S files.
>>>>>
>>>>> We keep the 4 byte function alignment in crti.S files. As the
>>>>> assembler now also outputs the start of _init and _fini functions
>>>>> as multiples of 4 byte, it perhaps has to fill it. Although GNU as
>>>>> is using NOPs here, to be sure, we just keep the alignment with
>>>>> 0x07 (=NOPs) at the end of crti.S.
>>>>>
>>>>> In order to avoid an obvious NOP slide in _fini, this patch also
>>>>> uses an lg instead of lgr instruction. Then the emitted instructions
>>>>> needs a multiple of 4 bytes.
>>>>> ---
>>>>> sysdeps/s390/s390-64/crti.S | 2 +-
>>>>> sysdeps/s390/s390-64/crtn.S | 2 --
>>>>> 2 files changed, 1 insertion(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/sysdeps/s390/s390-64/crti.S b/sysdeps/s390/s390-64/crti.S
>>>>> index 11ab75e8d9..4c8246da26 100644
>>>>> --- a/sysdeps/s390/s390-64/crti.S
>>>>> +++ b/sysdeps/s390/s390-64/crti.S
>>>>> @@ -85,7 +85,7 @@ _init:
>>>>> .type _fini,@function
>>>>> _fini:
>>>>> stmg %r6,%r15,48(%r15)
>>>>> - lgr %r1,%r15
>>>>> + lg %r1,120(%r15)
>>>>> aghi %r15,-160
>>>>> stg %r1,0(%r15)
>>>>> larl %r12,_GLOBAL_OFFSET_TABLE_
>>>>> diff --git a/sysdeps/s390/s390-64/crtn.S b/sysdeps/s390/s390-64/crtn.S
>>>>> index 0eabcb346c..6bb1bc9dcf 100644
>>>>> --- a/sysdeps/s390/s390-64/crtn.S
>>>>> +++ b/sysdeps/s390/s390-64/crtn.S
>>>>> @@ -37,13 +37,11 @@
>>>>> corresponding to the prologues in crti.S. */
>>>>>
>>>>> .section .init
>>>>> - .align 4
>>>>> lg %r4,272(%r15)
>>>>> lmg %r6,%r15,208(%r15)
>>>>> br %r4
>>>>>
>>>>> .section .fini
>>>>> - .align 4
>>>>> lg %r4,272(%r15)
>>>>> lmg %r6,%r15,208(%r15)
>>>>> br %r4
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-11-30 12:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-21 12:33 [PATCH] Avoid padding in _init and _fini. [BZ #31042] Stefan Liebler
2023-11-21 13:10 ` Adhemerval Zanella Netto
2023-11-21 13:20 ` Rui Ueyama
2023-11-21 15:47 ` Stefan Liebler
2023-11-21 15:57 ` Adhemerval Zanella Netto
2023-11-30 12:36 ` Stefan Liebler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).