[PATCH] [BFD] Fix override of COMMON symbols for a.out

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] [BFD] Fix override of COMMON symbols for a.out

Gunther Nikl-3
Hello,

The included patch is an attempt to fix a by now ancient bug in the
a.out
backend.

BFD initially merged a COMMON symbol with another symbol found in a
linker archive. Later a modification was suggested to make it possible
for a linker emulation to alter that behaviour. The patch for this
change was posted here:

  https://sourceware.org/ml/binutils/2002-07/msg00717.html

However, the code was modified before the commit:

  https://sourceware.org/git/?p=binutils.git;a=commit;h=d8f30db44739af73d229f540fd9030603350bc35

A default case was added, which completely changed the behaviour. The
problem is, that the default value of link_info.common_skip_ar_symbols
(skip_none) is *not* part of the switch in aout_link_check_ar_symbols.
Without the default case no switch case would trigger and the existing
behaviour would have been kept. Now with the default case attached to
"bfd_link_common_skip_all" a library symbol will never override a
COMMON symbol... I cannot say whether the historical behaviour should
be reestablished as done in the suggested patch. If another change in
behaviour is not desired than the posted patch should be committed with
an addition: change the global default in ld/ldmain.c to "skip_all".
This would keep the current behaviour while allowing the historical
behaviour.


Regards
Gunther Nikl

---
2018-01-12  Gunther Nikl  <[hidden email]>

        * bfd/aoutx.h (aout_link_check_ar_symbols): Add
          bfd_link_common_skip_none and make it the switch default.

--- a/bfd/aoutx.h
+++ b/bfd/aoutx.h
@@ -3366,13 +3366,15 @@ aout_link_check_ar_symbols (bfd *abfd,
 
       switch (info->common_skip_ar_symbols)
  {
+ default:
+ case bfd_link_common_skip_none:
+  break;
  case bfd_link_common_skip_text:
   skip = (type == (N_TEXT | N_EXT));
   break;
  case bfd_link_common_skip_data:
   skip = (type == (N_DATA | N_EXT));
   break;
- default:
  case bfd_link_common_skip_all:
   skip = 1;
   break;
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [BFD] Fix override of COMMON symbols for a.out

Alan Modra-3
On Thu, Jan 11, 2018 at 07:54:54PM +0100, [hidden email] wrote:
> 2018-01-12  Gunther Nikl  <[hidden email]>
>
> * bfd/aoutx.h (aout_link_check_ar_symbols): Add
>  bfd_link_common_skip_none and make it the switch default.

Applied with a slight variation.

        * aoutx.h (aout_link_check_ar_symbols): Remove default and handle
        bfd_link_common_skip_none in switch.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 85ea86a..19364c0 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,8 @@
+2018-01-12  Gunther Nikl  <[hidden email]>
+
+ * aoutx.h (aout_link_check_ar_symbols): Remove default and handle
+ bfd_link_common_skip_none in switch.
+
 2018-01-12  Alan Modra  <[hidden email]>
 
  PR ld/22649
diff --git a/bfd/aoutx.h b/bfd/aoutx.h
index 6dc4c68..eec9c4a 100644
--- a/bfd/aoutx.h
+++ b/bfd/aoutx.h
@@ -3366,13 +3366,14 @@ aout_link_check_ar_symbols (bfd *abfd,
 
       switch (info->common_skip_ar_symbols)
  {
+ case bfd_link_common_skip_none:
+  break;
  case bfd_link_common_skip_text:
   skip = (type == (N_TEXT | N_EXT));
   break;
  case bfd_link_common_skip_data:
   skip = (type == (N_DATA | N_EXT));
   break;
- default:
  case bfd_link_common_skip_all:
   skip = 1;
   break;

--
Alan Modra
Australia Development Lab, IBM
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [BFD] Fix override of COMMON symbols for a.out

Gunther Nikl-3
Hello,

Am Fri, 12 Jan 2018 21:20:43 +1030 schrieb Alan Modra
<[hidden email]>:

> On Thu, Jan 11, 2018 at 07:54:54PM +0100, [hidden email]
> wrote:
> > 2018-01-12  Gunther Nikl  <[hidden email]>
> >
> > * bfd/aoutx.h (aout_link_check_ar_symbols): Add
> >  bfd_link_common_skip_none and make it the switch default.
>
> Applied with a slight variation.
>
> * aoutx.h (aout_link_check_ar_symbols): Remove default and
> handle bfd_link_common_skip_none in switch.

Thank you for the quick response.

I looked at the code since I need similar code in the generic linker.
Does the code below for bfd/linker.c/generic_link_check_archive_element
look ok? It has one additional case for absolute symbols.

Regards,
Gunther Nikl

+  if (h->type == bfd_link_hash_common)
+    {
+      int skip = 0;
+
+      switch (info->common_skip_ar_symbols)
+ {
+ case bfd_link_common_skip_none:
+  break;
+ case bfd_link_common_skip_text:
+  skip = p->section->flags & SEC_CODE ? 1 : 0;
+  break;
+ case bfd_link_common_skip_data:
+  skip = p->section->flags & SEC_DATA ? 1 : 0;
+  break;
+ case bfd_link_common_skip_abs:
+  skip = bfd_is_abs_section (p->section) ? 1 : 0;
+  break;
+ case bfd_link_common_skip_all:
+  skip = 1;
+  break;
+ }
+
+      if (skip)
+ continue;
+    }