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

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

[gnikl]

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  <gnikl@users.sourceforge.net>

	* 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;

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

[Alan Modra]

On Thu, Jan 11, 2018 at 07:54:54PM +0100, gnikl@users.sourceforge.net wrote:
> 2018-01-12  Gunther Nikl  <gnikl@users.sourceforge.net>
> 
> 	* 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  <gnikl@users.sourceforge.net>
+
+	* aoutx.h (aout_link_check_ar_symbols): Remove default and handle
+	bfd_link_common_skip_none in switch.
+
 2018-01-12  Alan Modra  <amodra@gmail.com>
 
 	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

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

[Gunther Nikl]

Hello,

Am Fri, 12 Jan 2018 21:20:43 +1030 schrieb Alan Modra
<amodra@gmail.com>:
> On Thu, Jan 11, 2018 at 07:54:54PM +0100, gnikl@users.sourceforge.net
> wrote:
> > 2018-01-12  Gunther Nikl  <gnikl@users.sourceforge.net>
> > 
> > 	* 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;
+	    }