| Message ID | 874kqlosdd.fsf@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
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."
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;
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
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."
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.
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.
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
> > 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
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
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 "
> 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
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 "