remove stray .end directives

Submitted by Fangrui Song on July 2, 2019, 1:09 p.m.

Details

Message ID MWHPR22MB1679F4463361F6C16998B682CBF80@MWHPR22MB1679.namprd22.prod.outlook.com
State New
Series "remove stray .end directives"
Headers show

Commit Message

Fangrui Song July 2, 2019, 1:09 p.m.
This fixes an error when building musl powerpc{32,64} with clang
(it doesn't know GNU as ignores .end arguments):

../src/ldso/powerpc/dlsym.s:8:10: error: unexpected token in '.end' directive
          .end    dlsym

I'm not sure if  .size   dlsym, .-dlsym should also be deleted to be
consistent with other archs.

Patch hide | download patch | download mbox

From 1e8f793a4e191dd86c55f47a8e4ef8d76d773c1a Mon Sep 17 00:00:00 2001
From: Fangrui Song <i@maskray.me>
Date: Mon, 1 Jul 2019 09:42:49 +0000
Subject: [PATCH] remove stray .end directives

---
 src/ldso/powerpc/dlsym.s   | 1 -
 src/ldso/powerpc64/dlsym.s | 1 -
 2 files changed, 2 deletions(-)

diff --git a/src/ldso/powerpc/dlsym.s b/src/ldso/powerpc/dlsym.s
index 357d5771..cfe308ef 100644
--- a/src/ldso/powerpc/dlsym.s
+++ b/src/ldso/powerpc/dlsym.s
@@ -5,5 +5,4 @@ 
 dlsym:
 	mflr    5                      # The return address is arg3.
 	b       __dlsym
-	.end    dlsym
 	.size   dlsym, .-dlsym
diff --git a/src/ldso/powerpc64/dlsym.s b/src/ldso/powerpc64/dlsym.s
index 7eb691d9..a14715fd 100644
--- a/src/ldso/powerpc64/dlsym.s
+++ b/src/ldso/powerpc64/dlsym.s
@@ -8,5 +8,4 @@  dlsym:
 	.localentry dlsym,.-dlsym
 	mflr    5                      # The return address is arg3.
 	b       __dlsym
-	.end    dlsym
 	.size   dlsym, .-dlsym
-- 
2.22.0


Comments

Rich Felker July 2, 2019, 4:58 p.m.
On Tue, Jul 02, 2019 at 01:09:46PM +0000, Song Fangrui wrote:
> This fixes an error when building musl powerpc{32,64} with clang
> (it doesn't know GNU as ignores .end arguments):
> 
> ../src/ldso/powerpc/dlsym.s:8:10: error: unexpected token in '.end' directive
>           .end    dlsym
> 
> I'm not sure if  .size   dlsym, .-dlsym should also be deleted to be
> consistent with other archs.


> From 1e8f793a4e191dd86c55f47a8e4ef8d76d773c1a Mon Sep 17 00:00:00 2001
> From: Fangrui Song <i@maskray.me>
> Date: Mon, 1 Jul 2019 09:42:49 +0000
> Subject: [PATCH] remove stray .end directives
> 
> ---
>  src/ldso/powerpc/dlsym.s   | 1 -
>  src/ldso/powerpc64/dlsym.s | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/src/ldso/powerpc/dlsym.s b/src/ldso/powerpc/dlsym.s
> index 357d5771..cfe308ef 100644
> --- a/src/ldso/powerpc/dlsym.s
> +++ b/src/ldso/powerpc/dlsym.s
> @@ -5,5 +5,4 @@
>  dlsym:
>  	mflr    5                      # The return address is arg3.
>  	b       __dlsym
> -	.end    dlsym
>  	.size   dlsym, .-dlsym
> diff --git a/src/ldso/powerpc64/dlsym.s b/src/ldso/powerpc64/dlsym.s
> index 7eb691d9..a14715fd 100644
> --- a/src/ldso/powerpc64/dlsym.s
> +++ b/src/ldso/powerpc64/dlsym.s
> @@ -8,5 +8,4 @@ dlsym:
>  	.localentry dlsym,.-dlsym
>  	mflr    5                      # The return address is arg3.
>  	b       __dlsym
> -	.end    dlsym
>  	.size   dlsym, .-dlsym

This sounds right. Before I remove this, anyone have any idea what the
purpose of these was to begin with?

Rich
Markus Wichmann July 2, 2019, 5:53 p.m.
On Tue, Jul 02, 2019 at 12:58:41PM -0400, Rich Felker wrote:
> On Tue, Jul 02, 2019 at 01:09:46PM +0000, Song Fangrui wrote:
> > diff --git a/src/ldso/powerpc64/dlsym.s b/src/ldso/powerpc64/dlsym.s
> > index 7eb691d9..a14715fd 100644
> > --- a/src/ldso/powerpc64/dlsym.s
> > +++ b/src/ldso/powerpc64/dlsym.s
> > @@ -8,5 +8,4 @@ dlsym:
> >  	.localentry dlsym,.-dlsym
> >  	mflr    5                      # The return address is arg3.
> >  	b       __dlsym
> > -	.end    dlsym
> >  	.size   dlsym, .-dlsym
>
> This sounds right. Before I remove this, anyone have any idea what the
> purpose of these was to begin with?
>
> Rich

I can't tell you the purpose, but I can tell you the effect: This
directive ends assembly parsing. Therefore the .size directive below
them was never in effect before (if the documentation is to be trusted).
Dunno if that has any apparent effect though (besides the output of
objdump).

The PPC64 file was added in one go by Bobby Bingham in 2016. I guess the
line there is for consistency with PPC32. The PPC32 file was added in
2012 by rofl0r, though the blame also shows one Richard Pennington (not
the log, though). Weird. Anyway, the log message is also not very
enlightening. I can only assume the line was added in error. That, or
the .size directive. Together they make no sense.

Ciao,
Markus
Rich Felker July 2, 2019, 9:03 p.m.
On Tue, Jul 02, 2019 at 07:53:53PM +0200, Markus Wichmann wrote:
> On Tue, Jul 02, 2019 at 12:58:41PM -0400, Rich Felker wrote:
> > On Tue, Jul 02, 2019 at 01:09:46PM +0000, Song Fangrui wrote:
> > > diff --git a/src/ldso/powerpc64/dlsym.s b/src/ldso/powerpc64/dlsym.s
> > > index 7eb691d9..a14715fd 100644
> > > --- a/src/ldso/powerpc64/dlsym.s
> > > +++ b/src/ldso/powerpc64/dlsym.s
> > > @@ -8,5 +8,4 @@ dlsym:
> > >  	.localentry dlsym,.-dlsym
> > >  	mflr    5                      # The return address is arg3.
> > >  	b       __dlsym
> > > -	.end    dlsym
> > >  	.size   dlsym, .-dlsym
> >
> > This sounds right. Before I remove this, anyone have any idea what the
> > purpose of these was to begin with?
> >
> > Rich
> 
> I can't tell you the purpose, but I can tell you the effect: This
> directive ends assembly parsing. Therefore the .size directive below
> them was never in effect before (if the documentation is to be trusted).
> Dunno if that has any apparent effect though (besides the output of
> objdump).
> 
> The PPC64 file was added in one go by Bobby Bingham in 2016. I guess the
> line there is for consistency with PPC32. The PPC32 file was added in
> 2012 by rofl0r, though the blame also shows one Richard Pennington (not
> the log, though). Weird. Anyway, the log message is also not very
> enlightening. I can only assume the line was added in error. That, or
> the .size directive. Together they make no sense.

Thanks. Applying.

Rich
Fangrui Song July 3, 2019, 2:21 a.m.
On 2019-07-02, Rich Felker wrote:
>On Tue, Jul 02, 2019 at 07:53:53PM +0200, Markus Wichmann wrote:
>> On Tue, Jul 02, 2019 at 12:58:41PM -0400, Rich Felker wrote:
>> > On Tue, Jul 02, 2019 at 01:09:46PM +0000, Song Fangrui wrote:
>> > > diff --git a/src/ldso/powerpc64/dlsym.s b/src/ldso/powerpc64/dlsym.s
>> > > index 7eb691d9..a14715fd 100644
>> > > --- a/src/ldso/powerpc64/dlsym.s
>> > > +++ b/src/ldso/powerpc64/dlsym.s
>> > > @@ -8,5 +8,4 @@ dlsym:
>> > >  	.localentry dlsym,.-dlsym
>> > >  	mflr    5                      # The return address is arg3.
>> > >  	b       __dlsym
>> > > -	.end    dlsym
>> > >  	.size   dlsym, .-dlsym
>> >
>> > This sounds right. Before I remove this, anyone have any idea what the
>> > purpose of these was to begin with?
>> >
>> > Rich
>>
>> I can't tell you the purpose, but I can tell you the effect: This
>> directive ends assembly parsing. Therefore the .size directive below
>> them was never in effect before (if the documentation is to be trusted).
>> Dunno if that has any apparent effect though (besides the output of
>> objdump).
>>
>> The PPC64 file was added in one go by Bobby Bingham in 2016. I guess the
>> line there is for consistency with PPC32. The PPC32 file was added in
>> 2012 by rofl0r, though the blame also shows one Richard Pennington (not
>> the log, though). Weird. Anyway, the log message is also not very
>> enlightening. I can only assume the line was added in error. That, or
>> the .size directive. Together they make no sense.
>
>Thanks. Applying.

In linkers, I think st_size is only useful for STT_OBJECT (copy relocation).
It is not really necessary for functions.

There is some small value to have non-zero st_size for STT_FUNC though:
symbolizers and some other binary inspection tools may leverage this field.
Since most assembly files in musl do have .size, I think it makes sense
to omit it for consistency.