Commit: Fix GOLD testsuite failures for 2.35 branch

Message ID 874kqlosdd.fsf@redhat.com
State New
Headers show
Series
  • Commit: Fix GOLD testsuite failures for 2.35 branch
Related show

Commit Message

Alan Modra via Binutils July 6, 2020, 10:22 a.m.
Hi Guys,

  I am applying the patch below to fix some testsuite failures for the
  gold linker.  I am only applying them on the 2.35 branch as I think
  that there are outstanding RFAs for them to be applied to the
  mainline.

Cheers
  Nick

gold/ChangeLog
2020-07-06  Nick Clifton  <nickc@redhat.com>

	* target-reloc.h (Default_comdat_behaviour:get): Ignore discarded
	relocs that refer to the .gnu.build.attributes section.
	* testsuite/script_test_7.sh: Adjust expected address of the .bss
	section.
	* testsuite/script_test_9.sh: Do not expect the .init section to
	immediately follow the .text section in the mapping of sections to
	segments.

Comments

Andreas Schwab July 6, 2020, 10:50 a.m. | #1
On Jul 06 2020, Nick Clifton via Binutils wrote:

> diff --git a/gold/target-reloc.h b/gold/target-reloc.h

> index e9e3e5b002..d1d0e13589 100644

> --- a/gold/target-reloc.h

> +++ b/gold/target-reloc.h

> @@ -136,6 +136,7 @@ class Default_comdat_behavior

>      if (Layout::is_debug_info_section(name))

>        return CB_PRETEND;

>      if (strcmp(name, ".eh_frame") == 0

> +	|| strncmp(name, ".gnu.build.attributes", 21) == 0


Can you avoid the magic number?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Alan Modra via Binutils July 6, 2020, 11:22 a.m. | #2
Hi Andreas,

>> +	|| strncmp(name, ".gnu.build.attributes", 21) == 0

> 

> Can you avoid the magic number?


Is this the sort of thing that you had in mind ?

Cheers
  Nick

diff --git a/gold/target-reloc.h b/gold/target-reloc.h
index d1d0e13589..376bbd97c1 100644
--- a/gold/target-reloc.h
+++ b/gold/target-reloc.h
@@ -136,7 +136,8 @@ class Default_comdat_behavior
     if (Layout::is_debug_info_section(name))
       return CB_PRETEND;
     if (strcmp(name, ".eh_frame") == 0
-	|| strncmp(name, ".gnu.build.attributes", 21) == 0
+#define ATTR_SECTION_PREFIX ".gnu.build.attributes"
+	|| strncmp(name, ATTR_SECTION_PREFIX, sizeof ATTR_SECTION_PREFIX) == 0
 	|| strcmp(name, ".gcc_except_table") == 0)
       return CB_IGNORE;
     return CB_ERROR;
Hans-Peter Nilsson July 6, 2020, 11:51 a.m. | #3
On Mon, 6 Jul 2020, Nick Clifton via Binutils wrote:

> -	|| strncmp(name, ".gnu.build.attributes", 21) == 0

> +#define ATTR_SECTION_PREFIX ".gnu.build.attributes"

> +	|| strncmp(name, ATTR_SECTION_PREFIX, sizeof ATTR_SECTION_PREFIX) == 0


Beware: not the same thing.  Use strlen (which likely is
constant-folded to 21), not sizeof (which counts the nil; 22).

brgds, H-P
Andreas Schwab July 6, 2020, 11:52 a.m. | #4
On Jul 06 2020, Nick Clifton wrote:

> Hi Andreas,

>

>>> +	|| strncmp(name, ".gnu.build.attributes", 21) == 0

>> 

>> Can you avoid the magic number?

>

> Is this the sort of thing that you had in mind ?

>

> Cheers

>   Nick

>

> diff --git a/gold/target-reloc.h b/gold/target-reloc.h

> index d1d0e13589..376bbd97c1 100644

> --- a/gold/target-reloc.h

> +++ b/gold/target-reloc.h

> @@ -136,7 +136,8 @@ class Default_comdat_behavior

>      if (Layout::is_debug_info_section(name))

>        return CB_PRETEND;

>      if (strcmp(name, ".eh_frame") == 0

> -	|| strncmp(name, ".gnu.build.attributes", 21) == 0

> +#define ATTR_SECTION_PREFIX ".gnu.build.attributes"

> +	|| strncmp(name, ATTR_SECTION_PREFIX, sizeof ATTR_SECTION_PREFIX) == 0


You want strlen or sizeof - 1.  CONST_STRNEQ would be even better, if
gold wasn't such an alien part of binutils.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Alan Modra via Binutils July 6, 2020, 11:56 a.m. | #5
On Mon, Jul 6, 2020 at 4:24 AM Nick Clifton via Binutils
<binutils@sourceware.org> wrote:
>

> Hi Andreas,

>

> >> +    || strncmp(name, ".gnu.build.attributes", 21) == 0

> >

> > Can you avoid the magic number?

>

> Is this the sort of thing that you had in mind ?

>

> Cheers

>   Nick

>

> diff --git a/gold/target-reloc.h b/gold/target-reloc.h

> index d1d0e13589..376bbd97c1 100644

> --- a/gold/target-reloc.h

> +++ b/gold/target-reloc.h

> @@ -136,7 +136,8 @@ class Default_comdat_behavior

>      if (Layout::is_debug_info_section(name))

>        return CB_PRETEND;

>      if (strcmp(name, ".eh_frame") == 0

> -       || strncmp(name, ".gnu.build.attributes", 21) == 0

> +#define ATTR_SECTION_PREFIX ".gnu.build.attributes"

> +       || strncmp(name, ATTR_SECTION_PREFIX, sizeof ATTR_SECTION_PREFIX) == 0


Shouldn't it be sizeof ATTR_SECTION_PREFIX - 1?

>         || strcmp(name, ".gcc_except_table") == 0)

>        return CB_IGNORE;

>      return CB_ERROR;

>



-- 
H.J.
Alan Modra via Binutils July 6, 2020, 12:07 p.m. | #6
On Mon, Jul 6, 2020 at 3:23 AM Nick Clifton via Binutils
<binutils@sourceware.org> wrote:
>

> Hi Guys,

>

>   I am applying the patch below to fix some testsuite failures for the

>   gold linker.  I am only applying them on the 2.35 branch as I think

>   that there are outstanding RFAs for them to be applied to the

>   mainline.

>


FWIW, my gold patches are at

https://gitlab.com/x86-binutils/binutils-gdb/-/commits/users/hjl/gold/x86

which includes CET patches I submitted almost 2 years ago.

-- 
H.J.
Alan Modra via Binutils July 6, 2020, 1 p.m. | #7
Hi Guys,

  Yes  - you were all right - I was off by one.  *sigh*
  Still at least I had not applied the patch.  I have now
  done so however, with the correction for the sizeof snafu.

Cheers
  Nick
Alan Modra via Binutils July 6, 2020, 5 p.m. | #8
> >      if (strcmp(name, ".eh_frame") == 0

> > -     || strncmp(name, ".gnu.build.attributes", 21) == 0

> > +#define ATTR_SECTION_PREFIX ".gnu.build.attributes"

> > +     || strncmp(name, ATTR_SECTION_PREFIX, sizeof ATTR_SECTION_PREFIX) == 0

>

> You want strlen or sizeof - 1.  CONST_STRNEQ would be even better, if

> gold wasn't such an alien part of binutils.


Actually, you want is_prefix_of, defined in gold.h.

With that, it's OK for trunk.

-cary
Alan Modra via Binutils July 7, 2020, 8:55 a.m. | #9
Hi Cary,

> Actually, you want is_prefix_of, defined in gold.h.

> With that, it's OK for trunk.


Fantastic.  Thanks very much Cary.  Patch applied.

Cheers
  Nick
Alan Modra via Binutils July 7, 2020, 9:02 a.m. | #10
Hi Cary,

> Actually, you want is_prefix_of, defined in gold.h.

> With that, it's OK for trunk.


On this topic, would it be OK to apply the patch below as well ?
It fixes the two remaining gold testsuite failures for the x86_64
architecture.  (I have not checked other architectures).

Cheers
  Nick

gold/ChangeLog
2020-07-07  Nick Clifton  <nickc@redhat.com>

	* testsuite/script_test_7.sh: Adjust expected address of the .bss
	section.
	* testsuite/script_test_9.sh: Do not expect the .init section to
	immediately follow the .text section in the mapping of sections to
	segments.

diff --git a/gold/testsuite/script_test_7.sh b/gold/testsuite/script_test_7.sh
index adddf6c34b..091875e384 100755
--- a/gold/testsuite/script_test_7.sh
+++ b/gold/testsuite/script_test_7.sh
@@ -40,4 +40,4 @@ check()
 
 check script_test_7.stdout "\\.interp[ 	]*PROGBITS[ 	]*0*10000100"
 check script_test_7.stdout "\\.data[ 	]*PROGBITS[ 	]*0*10200000"
-check script_test_7.stdout "\\.bss[ 	]*NOBITS[ 	]*0*10400..."
+check script_test_7.stdout "\\.bss[ 	]*NOBITS[ 	]*0*1040...."
diff --git a/gold/testsuite/script_test_9.sh b/gold/testsuite/script_test_9.sh
index 3dcd3c49f3..da75b65c93 100755
--- a/gold/testsuite/script_test_9.sh
+++ b/gold/testsuite/script_test_9.sh
@@ -38,5 +38,6 @@ check()
 
 check script_test_9.stdout "LOAD .*R E "
 check script_test_9.stdout "LOAD .*RW "
-check script_test_9.stdout "00 .*\.text .init"
+check script_test_9.stdout "00 .*\.text"
+check script_test_9.stdout "00 .*\.init"
 check script_test_9.stdout "01 .*\.data "
Alan Modra via Binutils July 7, 2020, 11:35 p.m. | #11
> gold/ChangeLog

> 2020-07-07  Nick Clifton  <nickc@redhat.com>

>

>         * testsuite/script_test_7.sh: Adjust expected address of the .bss

>         section.

>         * testsuite/script_test_9.sh: Do not expect the .init section to

>         immediately follow the .text section in the mapping of sections to

>         segments.


This is OK. Thanks!

-cary

Patch

diff --git a/gold/target-reloc.h b/gold/target-reloc.h
index e9e3e5b002..d1d0e13589 100644
--- a/gold/target-reloc.h
+++ b/gold/target-reloc.h
@@ -136,6 +136,7 @@  class Default_comdat_behavior
     if (Layout::is_debug_info_section(name))
       return CB_PRETEND;
     if (strcmp(name, ".eh_frame") == 0
+	|| strncmp(name, ".gnu.build.attributes", 21) == 0
 	|| strcmp(name, ".gcc_except_table") == 0)
       return CB_IGNORE;
     return CB_ERROR;
diff --git a/gold/testsuite/script_test_7.sh b/gold/testsuite/script_test_7.sh
index adddf6c34b..091875e384 100755
--- a/gold/testsuite/script_test_7.sh
+++ b/gold/testsuite/script_test_7.sh
@@ -40,4 +40,4 @@  check()
 
 check script_test_7.stdout "\\.interp[ 	]*PROGBITS[ 	]*0*10000100"
 check script_test_7.stdout "\\.data[ 	]*PROGBITS[ 	]*0*10200000"
-check script_test_7.stdout "\\.bss[ 	]*NOBITS[ 	]*0*10400..."
+check script_test_7.stdout "\\.bss[ 	]*NOBITS[ 	]*0*1040...."
diff --git a/gold/testsuite/script_test_9.sh b/gold/testsuite/script_test_9.sh
index 3dcd3c49f3..da75b65c93 100755
--- a/gold/testsuite/script_test_9.sh
+++ b/gold/testsuite/script_test_9.sh
@@ -38,5 +38,6 @@  check()
 
 check script_test_9.stdout "LOAD .*R E "
 check script_test_9.stdout "LOAD .*RW "
-check script_test_9.stdout "00 .*\.text .init"
+check script_test_9.stdout "00 .*\.text"
+check script_test_9.stdout "00 .*\.init"
 check script_test_9.stdout "01 .*\.data "