* [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC
@ 2021-01-13 12:41 H.J. Lu
2021-01-14 6:00 ` Mike Frysinger
0 siblings, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2021-01-13 12:41 UTC (permalink / raw)
To: binutils
Linker should never generate dynamic relocations for relocations in
non-SEC_ALLOC section which has no impact on run-time behavior. Such
relocations should be resolved to 0.
PR ld/26688
* elf32-bfin.c (bfinfdpic_relocate_section): Skip non SEC_ALLOC
section for R_BFIN_FUNCDESC.
---
bfd/elf32-bfin.c | 235 ++++++++++++++++++++++++-----------------------
1 file changed, 118 insertions(+), 117 deletions(-)
diff --git a/bfd/elf32-bfin.c b/bfd/elf32-bfin.c
index 7c926b8a96e..401683c5367 100644
--- a/bfd/elf32-bfin.c
+++ b/bfd/elf32-bfin.c
@@ -2727,129 +2727,130 @@ bfinfdpic_relocate_section (bfd * output_bfd,
break;
case R_BFIN_FUNCDESC:
- {
- int dynindx;
- bfd_vma addend = rel->r_addend;
-
- if (! (h && h->root.type == bfd_link_hash_undefweak
- && BFINFDPIC_SYM_LOCAL (info, h)))
- {
- /* If the symbol is dynamic and there may be dynamic
- symbol resolution because we are or are linked with a
- shared library, emit a FUNCDESC relocation such that
- the dynamic linker will allocate the function
- descriptor. If the symbol needs a non-local function
- descriptor but binds locally (e.g., its visibility is
- protected, emit a dynamic relocation decayed to
- section+offset. */
- if (h && ! BFINFDPIC_FUNCDESC_LOCAL (info, h)
- && BFINFDPIC_SYM_LOCAL (info, h)
- && !bfd_link_pde (info))
- {
- dynindx = elf_section_data (h->root.u.def.section
- ->output_section)->dynindx;
- addend += h->root.u.def.section->output_offset
- + h->root.u.def.value;
- }
- else if (h && ! BFINFDPIC_FUNCDESC_LOCAL (info, h))
- {
- if (addend)
- {
- info->callbacks->warning
- (info, _("R_BFIN_FUNCDESC references dynamic symbol with nonzero addend"),
- name, input_bfd, input_section, rel->r_offset);
- return FALSE;
- }
- dynindx = h->dynindx;
- }
- else
- {
- /* Otherwise, we know we have a private function
- descriptor, so reference it directly. */
- BFD_ASSERT (picrel->privfd);
- r_type = R_BFIN_BYTE4_DATA;
- dynindx = elf_section_data (bfinfdpic_got_section (info)
- ->output_section)->dynindx;
- addend = bfinfdpic_got_section (info)->output_offset
- + bfinfdpic_got_initial_offset (info)
- + picrel->fd_entry;
- }
-
- /* If there is room for dynamic symbol resolution, emit
- the dynamic relocation. However, if we're linking an
- executable at a fixed location, we won't have emitted a
- dynamic symbol entry for the got section, so idx will
- be zero, which means we can and should compute the
- address of the private descriptor ourselves. */
- if (bfd_link_pde (info)
- && (!h || BFINFDPIC_FUNCDESC_LOCAL (info, h)))
- {
- bfd_vma offset;
-
- addend += bfinfdpic_got_section (info)->output_section->vma;
- if ((bfd_section_flags (input_section->output_section)
- & (SEC_ALLOC | SEC_LOAD)) == (SEC_ALLOC | SEC_LOAD))
- {
- if (_bfinfdpic_osec_readonly_p (output_bfd,
- input_section
- ->output_section))
- {
- info->callbacks->warning
- (info,
- _("cannot emit fixups in read-only section"),
- name, input_bfd, input_section, rel->r_offset);
- return FALSE;
- }
+ if ((input_section->flags & SEC_ALLOC) != 0)
+ {
+ int dynindx;
+ bfd_vma addend = rel->r_addend;
- offset = _bfd_elf_section_offset
- (output_bfd, info,
- input_section, rel->r_offset);
+ if (! (h && h->root.type == bfd_link_hash_undefweak
+ && BFINFDPIC_SYM_LOCAL (info, h)))
+ {
+ /* If the symbol is dynamic and there may be dynamic
+ symbol resolution because we are or are linked with a
+ shared library, emit a FUNCDESC relocation such that
+ the dynamic linker will allocate the function
+ descriptor. If the symbol needs a non-local function
+ descriptor but binds locally (e.g., its visibility is
+ protected, emit a dynamic relocation decayed to
+ section+offset. */
+ if (h && ! BFINFDPIC_FUNCDESC_LOCAL (info, h)
+ && BFINFDPIC_SYM_LOCAL (info, h)
+ && !bfd_link_pde (info))
+ {
+ dynindx = elf_section_data (h->root.u.def.section
+ ->output_section)->dynindx;
+ addend += h->root.u.def.section->output_offset
+ + h->root.u.def.value;
+ }
+ else if (h && ! BFINFDPIC_FUNCDESC_LOCAL (info, h))
+ {
+ if (addend)
+ {
+ info->callbacks->warning
+ (info, _("R_BFIN_FUNCDESC references dynamic symbol with nonzero addend"),
+ name, input_bfd, input_section, rel->r_offset);
+ return FALSE;
+ }
+ dynindx = h->dynindx;
+ }
+ else
+ {
+ /* Otherwise, we know we have a private function
+ descriptor, so reference it directly. */
+ BFD_ASSERT (picrel->privfd);
+ r_type = R_BFIN_BYTE4_DATA;
+ dynindx = elf_section_data (bfinfdpic_got_section (info)
+ ->output_section)->dynindx;
+ addend = bfinfdpic_got_section (info)->output_offset
+ + bfinfdpic_got_initial_offset (info)
+ + picrel->fd_entry;
+ }
- if (offset != (bfd_vma)-1)
- _bfinfdpic_add_rofixup (output_bfd,
- bfinfdpic_gotfixup_section
- (info),
+ /* If there is room for dynamic symbol resolution, emit
+ the dynamic relocation. However, if we're linking an
+ executable at a fixed location, we won't have emitted a
+ dynamic symbol entry for the got section, so idx will
+ be zero, which means we can and should compute the
+ address of the private descriptor ourselves. */
+ if (bfd_link_pde (info)
+ && (!h || BFINFDPIC_FUNCDESC_LOCAL (info, h)))
+ {
+ bfd_vma offset;
+
+ addend += bfinfdpic_got_section (info)->output_section->vma;
+ if ((bfd_section_flags (input_section->output_section)
+ & (SEC_ALLOC | SEC_LOAD)) == (SEC_ALLOC | SEC_LOAD))
+ {
+ if (_bfinfdpic_osec_readonly_p (output_bfd,
+ input_section
+ ->output_section))
+ {
+ info->callbacks->warning
+ (info,
+ _("cannot emit fixups in read-only section"),
+ name, input_bfd, input_section, rel->r_offset);
+ return FALSE;
+ }
+
+ offset = _bfd_elf_section_offset
+ (output_bfd, info,
+ input_section, rel->r_offset);
+
+ if (offset != (bfd_vma)-1)
+ _bfinfdpic_add_rofixup (output_bfd,
+ bfinfdpic_gotfixup_section
+ (info),
+ offset + input_section
+ ->output_section->vma
+ + input_section->output_offset,
+ picrel);
+ }
+ }
+ else if ((bfd_section_flags (input_section->output_section)
+ & (SEC_ALLOC | SEC_LOAD)) == (SEC_ALLOC | SEC_LOAD))
+ {
+ bfd_vma offset;
+
+ if (_bfinfdpic_osec_readonly_p (output_bfd,
+ input_section
+ ->output_section))
+ {
+ info->callbacks->warning
+ (info,
+ _("cannot emit dynamic relocations in read-only section"),
+ name, input_bfd, input_section, rel->r_offset);
+ return FALSE;
+ }
+ offset = _bfd_elf_section_offset (output_bfd, info,
+ input_section, rel->r_offset);
+
+ if (offset != (bfd_vma)-1)
+ _bfinfdpic_add_dyn_reloc (output_bfd,
+ bfinfdpic_gotrel_section (info),
offset + input_section
->output_section->vma
+ input_section->output_offset,
- picrel);
- }
- }
- else if ((bfd_section_flags (input_section->output_section)
- & (SEC_ALLOC | SEC_LOAD)) == (SEC_ALLOC | SEC_LOAD))
- {
- bfd_vma offset;
-
- if (_bfinfdpic_osec_readonly_p (output_bfd,
- input_section
- ->output_section))
- {
- info->callbacks->warning
- (info,
- _("cannot emit dynamic relocations in read-only section"),
- name, input_bfd, input_section, rel->r_offset);
- return FALSE;
- }
- offset = _bfd_elf_section_offset (output_bfd, info,
- input_section, rel->r_offset);
-
- if (offset != (bfd_vma)-1)
- _bfinfdpic_add_dyn_reloc (output_bfd,
- bfinfdpic_gotrel_section (info),
- offset + input_section
- ->output_section->vma
- + input_section->output_offset,
- r_type,
- dynindx, addend, picrel);
- }
- else
- addend += bfinfdpic_got_section (info)->output_section->vma;
- }
+ r_type,
+ dynindx, addend, picrel);
+ }
+ else
+ addend += bfinfdpic_got_section (info)->output_section->vma;
+ }
- /* We want the addend in-place because dynamic
- relocations are REL. Setting relocation to it should
- arrange for it to be installed. */
- relocation = addend - rel->r_addend;
+ /* We want the addend in-place because dynamic
+ relocations are REL. Setting relocation to it should
+ arrange for it to be installed. */
+ relocation = addend - rel->r_addend;
}
check_segment[0] = check_segment[1] = got_segment;
break;
--
2.29.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC
2021-01-13 12:41 [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC H.J. Lu
@ 2021-01-14 6:00 ` Mike Frysinger
2021-01-14 13:28 ` H.J. Lu
0 siblings, 1 reply; 7+ messages in thread
From: Mike Frysinger @ 2021-01-14 6:00 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
[-- Attachment #1: Type: text/plain, Size: 324 bytes --]
On 13 Jan 2021 04:41, H.J. Lu via Binutils wrote:
> Linker should never generate dynamic relocations for relocations in
> non-SEC_ALLOC section which has no impact on run-time behavior. Such
> relocations should be resolved to 0.
does elf32-frv.c need the same fix ? kind of looks like it.
otherwise, lgtm, thanks
-mike
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC
2021-01-14 6:00 ` Mike Frysinger
@ 2021-01-14 13:28 ` H.J. Lu
2021-01-14 13:31 ` Nick Clifton
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: H.J. Lu @ 2021-01-14 13:28 UTC (permalink / raw)
To: H.J. Lu, Nick Clifton, Alexandre Oliva; +Cc: Binutils
On Wed, Jan 13, 2021 at 10:00 PM Mike Frysinger <vapier@gentoo.org> wrote:
>
> On 13 Jan 2021 04:41, H.J. Lu via Binutils wrote:
> > Linker should never generate dynamic relocations for relocations in
> > non-SEC_ALLOC section which has no impact on run-time behavior. Such
> > relocations should be resolved to 0.
>
> does elf32-frv.c need the same fix ? kind of looks like it.
Yes. FRV maintainers need to take a look. There are many code/bug
duplications in FDPIC support. They should be merged.
> otherwise, lgtm, thanks
> -mike
Fixed on master. Nick, is it OK for 2.36 branch?
--
H.J.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC
2021-01-14 13:28 ` H.J. Lu
@ 2021-01-14 13:31 ` Nick Clifton
2021-01-14 19:40 ` Update my email address (was: Re: [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC) Alexandre Oliva
2021-01-14 20:27 ` [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC Mike Frysinger
2 siblings, 0 replies; 7+ messages in thread
From: Nick Clifton @ 2021-01-14 13:31 UTC (permalink / raw)
To: H.J. Lu, Alexandre Oliva; +Cc: Binutils
Hi H.J.
> Fixed on master. Nick, is it OK for 2.36 branch?
Yes - please apply.
Cheers
Nick
^ permalink raw reply [flat|nested] 7+ messages in thread
* Update my email address (was: Re: [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC)
2021-01-14 13:28 ` H.J. Lu
2021-01-14 13:31 ` Nick Clifton
@ 2021-01-14 19:40 ` Alexandre Oliva
2021-01-14 20:27 ` [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC Mike Frysinger
2 siblings, 0 replies; 7+ messages in thread
From: Alexandre Oliva @ 2021-01-14 19:40 UTC (permalink / raw)
To: H.J. Lu; +Cc: Nick Clifton, Binutils
On Jan 14, 2021, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> On Wed, Jan 13, 2021 at 10:00 PM Mike Frysinger <vapier@gentoo.org> wrote:
>>
>> On 13 Jan 2021 04:41, H.J. Lu via Binutils wrote:
>> > Linker should never generate dynamic relocations for relocations in
>> > non-SEC_ALLOC section which has no impact on run-time behavior. Such
>> > relocations should be resolved to 0.
>>
>> does elf32-frv.c need the same fix ? kind of looks like it.
> Yes. FRV maintainers need to take a look.
I haven't had access to FR-V (or SH, or MN10300) hardware for a while,
but I think I can still take care of this one. Thanks,
This message made me realize I had failed to update my email address in
binutils/MAINTAINERS. I'm checking this in now.
Update my email address (long overdue!)
From: Alexandre Oliva <oliva@gnu.org>
for binutils/ChangeLog
* MAINTAINERS: Update my email address.
---
binutils/MAINTAINERS | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/binutils/MAINTAINERS b/binutils/MAINTAINERS
index af9a08e5720c5..b7a3376b714f4 100644
--- a/binutils/MAINTAINERS
+++ b/binutils/MAINTAINERS
@@ -84,7 +84,7 @@ responsibility among the other maintainers.
EPIPHANY Joern Rennecke <joern.rennecke@embecosm.com>
FR30 Nick Clifton <nickc@redhat.com>
FRV Nick Clifton <nickc@redhat.com>
- FRV Alexandre Oliva <aoliva@redhat.com>
+ FRV Alexandre Oliva <aoliva@sourceware.org>
GOLD Ian Lance Taylor <iant@google.com>
GOLD Cary Coutant <ccoutant@gmail.com>
H8300 Prafulla Thakare <prafulla.thakare@kpitcummins.com>
@@ -109,7 +109,7 @@ responsibility among the other maintainers.
MIPS Chenghua Xu <paul.hua.gm@gmail.com>
MIPS I-IV Maciej W. Rozycki <macro@linux-mips.org>
MMIX Hans-Peter Nilsson <hp@bitrange.com>
- MN10300 Alexandre Oliva <aoliva@redhat.com>
+ MN10300 Alexandre Oliva <aoliva@sourceware.org>
Moxie Anthony Green <green@moxielogic.com>
MSP430 Dmitry Diky <diwil@spec.ru>
NDS32 Kuan-Lin Chen <kuanlinchentw@gmail.com>
@@ -133,7 +133,7 @@ responsibility among the other maintainers.
S12Z John Darrington <john@darrington.wattle.id.au>
s390, s390x Martin Schwidefsky <schwidefsky@de.ibm.com>
s390, s390x Andreas Krebbel <krebbel@linux.vnet.ibm.com>
- SH Alexandre Oliva <aoliva@redhat.com>
+ SH Alexandre Oliva <aoliva@sourceware.org>
SPARC David S. Miller <davem@davemloft.net>
SPARC Jose E. Marchesi <jose.marchesi@oracle.com>
SPU Alan Modra <amodra@gmail.com>
--
Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
Vim, Vi, Voltei pro Emacs -- GNUlius Caesar
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC
2021-01-14 13:28 ` H.J. Lu
2021-01-14 13:31 ` Nick Clifton
2021-01-14 19:40 ` Update my email address (was: Re: [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC) Alexandre Oliva
@ 2021-01-14 20:27 ` Mike Frysinger
2021-01-14 20:36 ` H.J. Lu
2 siblings, 1 reply; 7+ messages in thread
From: Mike Frysinger @ 2021-01-14 20:27 UTC (permalink / raw)
To: H.J. Lu; +Cc: Nick Clifton, Alexandre Oliva, Binutils
[-- Attachment #1: Type: text/plain, Size: 1094 bytes --]
On 14 Jan 2021 05:28, H.J. Lu via Binutils wrote:
> On Wed, Jan 13, 2021 at 10:00 PM Mike Frysinger wrote:
> > On 13 Jan 2021 04:41, H.J. Lu via Binutils wrote:
> > > Linker should never generate dynamic relocations for relocations in
> > > non-SEC_ALLOC section which has no impact on run-time behavior. Such
> > > relocations should be resolved to 0.
> >
> > does elf32-frv.c need the same fix ? kind of looks like it.
>
> Yes. FRV maintainers need to take a look.
well, didn't you merge this ? :)
frv: Don't generate dynamic relocation for non SEC_ALLOC sections
> There are many code/bug duplications in FDPIC support.
> They should be merged.
to be fair, Blackfin copied FRV :). we owe a ton to FRV wrt FDPIC in the
toolchain & kernel.
i took a look a while ago about trying to factor things out at least in
gdb, but it got so invasive, and i didn't have FRV to verify against,
that i ended up shelving it.
i'd feel similarly about such large refactors in bfd. not sure how to
even structure it. would we create files like fdpic.c/fdpic32.c ?
-mike
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC
2021-01-14 20:27 ` [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC Mike Frysinger
@ 2021-01-14 20:36 ` H.J. Lu
0 siblings, 0 replies; 7+ messages in thread
From: H.J. Lu @ 2021-01-14 20:36 UTC (permalink / raw)
To: Nick Clifton, Alexandre Oliva, Binutils
On Thu, Jan 14, 2021 at 12:27 PM Mike Frysinger <vapier@gentoo.org> wrote:
>
> On 14 Jan 2021 05:28, H.J. Lu via Binutils wrote:
> > On Wed, Jan 13, 2021 at 10:00 PM Mike Frysinger wrote:
> > > On 13 Jan 2021 04:41, H.J. Lu via Binutils wrote:
> > > > Linker should never generate dynamic relocations for relocations in
> > > > non-SEC_ALLOC section which has no impact on run-time behavior. Such
> > > > relocations should be resolved to 0.
> > >
> > > does elf32-frv.c need the same fix ? kind of looks like it.
> >
> > Yes. FRV maintainers need to take a look.
>
> well, didn't you merge this ? :)
> frv: Don't generate dynamic relocation for non SEC_ALLOC sections
>
> > There are many code/bug duplications in FDPIC support.
> > They should be merged.
>
> to be fair, Blackfin copied FRV :). we owe a ton to FRV wrt FDPIC in the
> toolchain & kernel.
>
> i took a look a while ago about trying to factor things out at least in
> gdb, but it got so invasive, and i didn't have FRV to verify against,
> that i ended up shelving it.
>
> i'd feel similarly about such large refactors in bfd. not sure how to
> even structure it. would we create files like fdpic.c/fdpic32.c ?
> -mike
Take a look at elfxx-x86.[ch] to see how I merged common parts
of i386 and x86-64.
--
H.J.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-01-14 20:37 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-13 12:41 [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC H.J. Lu
2021-01-14 6:00 ` Mike Frysinger
2021-01-14 13:28 ` H.J. Lu
2021-01-14 13:31 ` Nick Clifton
2021-01-14 19:40 ` Update my email address (was: Re: [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC) Alexandre Oliva
2021-01-14 20:27 ` [PATCH] bfin: Skip non SEC_ALLOC section for R_BFIN_FUNCDESC Mike Frysinger
2021-01-14 20:36 ` H.J. Lu
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).