This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Don't use .gnu.linkonce with COMDAT


Mark Mitchell wrote:
Daniel Jacobowitz wrote:

How does this patch interact with non-GNU linkers?  This is one of the
problems with version-number based checks...


All the ld_vers* stuff is conditionalized on GNU ld, so I think this patch would disable COMDAT for non-GNU linkers. Julian, I suggest you change it so that if $ld_vers is empty, you just set HAVE_COMDAT_GROUP based on the the assembler, as is done at present. That's no more or less broken than the current state.

I think this is done anyway with the patch as-is:


... && test x"$ld_vers" != x; then ...

This bit:

+       memcpy (rname, ".rodata", 7);
+       memcpy (rname + 7, name + 5, len - 5);

We shouldn't right "7" and such here; use strlen (".rodata"), instead. GCC knows to collapse that at compile time. And, for that matter, just use strcpy; GCC knows to turn that into more efficient operations if the string is known.

With those changes, I think the patch is OK. But, you'll need to be alert; this stuff seems very fragile, and I don't know if there are other things like the .rodata problem that will pop up.

I've rewritten this bit to use strcpy/strcat, and it is much clearer now. I'll wait a day or so before applying this though, I think, in case anyone notices or predicts any further problems. I agree about the fragility of this stuff.


Cheers,

Julian
Index: gcc/configure.ac
===================================================================
RCS file: /home/gcc/repos/gcc/gcc/gcc/configure.ac,v
retrieving revision 2.106
diff -c -p -r2.106 configure.ac
*** gcc/configure.ac	27 Apr 2005 20:35:38 -0000	2.106
--- gcc/configure.ac	6 May 2005 15:03:48 -0000
*************** changequote(,)dnl
*** 2103,2110 ****
  	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)$,\1,p' \
  	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\)[	 ].*$,\1,p' \
  	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)[	 ].*$,\1,p' \
! 	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)[	 ].*$,\1,p'`
      ld_date=`echo $ld_ver | sed -n 's,^.*\([2-9][0-9][0-9][0-9]\)[-]*\([01][0-9]\)[-]*\([0-3][0-9]\).*$,\1\2\3,p'`
      if test 0"$ld_date" -lt 20020404; then
        if test -n "$ld_date"; then
  	# If there was date string, but was earlier than 2002-04-04, fail
--- 2103,2114 ----
  	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)$,\1,p' \
  	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\)[	 ].*$,\1,p' \
  	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)[	 ].*$,\1,p' \
! 	-e 's,^.*[ 	 ]\([0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)[ 	].*$,\1,p' \
! 	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)[	 ].*$,\1,p'`
      ld_date=`echo $ld_ver | sed -n 's,^.*\([2-9][0-9][0-9][0-9]\)[-]*\([01][0-9]\)[-]*\([0-3][0-9]\).*$,\1\2\3,p'`
+     ld_vers_major=`expr "$ld_vers" : '\([0-9]*\)'`
+     ld_vers_minor=`expr "$ld_vers" : '[0-9]*\.\([0-9]*\)'`
+     ld_vers_patch=`expr "$ld_vers" : '[0-9]*\.[0-9]*\.\([0-9]*\)'`
      if test 0"$ld_date" -lt 20020404; then
        if test -n "$ld_date"; then
  	# If there was date string, but was earlier than 2002-04-04, fail
*************** changequote(,)dnl
*** 2113,2121 ****
  	# If there was no date string nor ld version number, something is wrong
  	gcc_cv_ld_hidden=no
        else
- 	ld_vers_major=`expr "$ld_vers" : '\([0-9]*\)'`
- 	ld_vers_minor=`expr "$ld_vers" : '[0-9]*\.\([0-9]*\)'`
- 	ld_vers_patch=`expr "$ld_vers" : '[0-9]*\.[0-9]*\.\([0-9]*\)'`
  	test -z "$ld_vers_patch" && ld_vers_patch=0
  	if test "$ld_vers_major" -lt 2; then
  	  gcc_cv_ld_hidden=no
--- 2117,2122 ----
*************** AC_DEFINE_UNQUOTED(HAVE_GAS_SHF_MERGE,
*** 2256,2273 ****
  [Define 0/1 if your assembler supports marking sections with SHF_MERGE flag.])
  
  gcc_GAS_CHECK_FEATURE(COMDAT group support, gcc_cv_as_comdat_group,
!  [elf,2,15,91], [--fatal-warnings],
   [.section .text,"axG",@progbits,.foo,comdat])
  if test $gcc_cv_as_comdat_group = yes; then
    gcc_cv_as_comdat_group_percent=no
  else
   gcc_GAS_CHECK_FEATURE(COMDAT group support, gcc_cv_as_comdat_group_percent,
!    [elf,2,15,91], [--fatal-warnings],
     [.section .text,"axG",%progbits,.foo,comdat])
  fi
! AC_DEFINE_UNQUOTED(HAVE_GAS_COMDAT_GROUP,
    [`if test $gcc_cv_as_comdat_group = yes || test $gcc_cv_as_comdat_group_percent = yes; then echo 1; else echo 0; fi`],
! [Define 0/1 if your assembler supports COMDAT group.])
  
  # Thread-local storage - the check is heavily parametrized.
  conftest_s=
--- 2257,2291 ----
  [Define 0/1 if your assembler supports marking sections with SHF_MERGE flag.])
  
  gcc_GAS_CHECK_FEATURE(COMDAT group support, gcc_cv_as_comdat_group,
!  [elf,2,16,0], [--fatal-warnings],
   [.section .text,"axG",@progbits,.foo,comdat])
  if test $gcc_cv_as_comdat_group = yes; then
    gcc_cv_as_comdat_group_percent=no
  else
   gcc_GAS_CHECK_FEATURE(COMDAT group support, gcc_cv_as_comdat_group_percent,
!    [elf,2,16,0], [--fatal-warnings],
     [.section .text,"axG",%progbits,.foo,comdat])
  fi
! if test $in_tree_ld != yes && test x"$ld_vers" != x; then
!   comdat_group=yes
!   if test 0"$ld_date" -lt 20050308; then
!     if test -n "$ld_date"; then
!       # If there was date string, but was earlier than 2005-03-08, fail
!       comdat_group=no
!     elif test "$ld_vers_major" -lt 2; then
!       comdat_group=no
!     elif test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -lt 16; then
!       comdat_group=no
!     fi
!   fi
!   if test $comdat_group = no; then
!     gcc_cv_as_comdat_group=no
!     gcc_cv_as_comdat_group_percent=no
!   fi
! fi
! AC_DEFINE_UNQUOTED(HAVE_COMDAT_GROUP,
    [`if test $gcc_cv_as_comdat_group = yes || test $gcc_cv_as_comdat_group_percent = yes; then echo 1; else echo 0; fi`],
! [Define 0/1 if your assembler and linker support COMDAT groups.])
  
  # Thread-local storage - the check is heavily parametrized.
  conftest_s=
Index: gcc/varasm.c
===================================================================
RCS file: /home/gcc/repos/gcc/gcc/gcc/varasm.c,v
retrieving revision 1.507
diff -c -p -r1.507 varasm.c
*** gcc/varasm.c	27 Apr 2005 21:35:20 -0000	1.507
--- gcc/varasm.c	6 May 2005 18:12:44 -0000
*************** default_function_rodata_section (tree de
*** 618,625 ****
      {
        const char *name = TREE_STRING_POINTER (DECL_SECTION_NAME (decl));
  
        /* For .gnu.linkonce.t.foo we want to use .gnu.linkonce.r.foo.  */
!       if (DECL_ONE_ONLY (decl) && strncmp (name, ".gnu.linkonce.t.", 16) == 0)
  	{
  	  size_t len = strlen (name) + 1;
  	  char *rname = alloca (len);
--- 618,636 ----
      {
        const char *name = TREE_STRING_POINTER (DECL_SECTION_NAME (decl));
  
+       if (DECL_ONE_ONLY (decl) && HAVE_COMDAT_GROUP)
+         {
+ 	  size_t len = strlen (name) + 3;
+ 	  char* rname = alloca (len);
+          
+ 	  strcpy (rname, ".rodata");
+ 	  strcat (rname, name + 5); 
+           named_section_real (rname, SECTION_LINKONCE, decl);
+ 	  return;
+ 	}
        /* For .gnu.linkonce.t.foo we want to use .gnu.linkonce.r.foo.  */
!       else if (DECL_ONE_ONLY (decl)
! 	       && strncmp (name, ".gnu.linkonce.t.", 16) == 0)
  	{
  	  size_t len = strlen (name) + 1;
  	  char *rname = alloca (len);
*************** default_elf_asm_named_section (const cha
*** 4866,4872 ****
       abbreviated form to switch back to it -- unless this section is
       part of a COMDAT groups, in which case GAS requires the full
       declaration every time.  */
!   if (!(HAVE_GAS_COMDAT_GROUP && (flags & SECTION_LINKONCE))
        && ! named_section_first_declaration (name))
      {
        fprintf (asm_out_file, "\t.section\t%s\n", name);
--- 4877,4883 ----
       abbreviated form to switch back to it -- unless this section is
       part of a COMDAT groups, in which case GAS requires the full
       declaration every time.  */
!   if (!(HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE))
        && ! named_section_first_declaration (name))
      {
        fprintf (asm_out_file, "\t.section\t%s\n", name);
*************** default_elf_asm_named_section (const cha
*** 4887,4893 ****
      *f++ = 'S';
    if (flags & SECTION_TLS)
      *f++ = 'T';
!   if (HAVE_GAS_COMDAT_GROUP && (flags & SECTION_LINKONCE))
      *f++ = 'G';
    *f = '\0';
  
--- 4898,4904 ----
      *f++ = 'S';
    if (flags & SECTION_TLS)
      *f++ = 'T';
!   if (HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE))
      *f++ = 'G';
    *f = '\0';
  
*************** default_elf_asm_named_section (const cha
*** 4914,4920 ****
  
        if (flags & SECTION_ENTSIZE)
  	fprintf (asm_out_file, ",%d", flags & SECTION_ENTSIZE);
!       if (HAVE_GAS_COMDAT_GROUP && (flags & SECTION_LINKONCE))
  	fprintf (asm_out_file, ",%s,comdat", 
  		 lang_hooks.decls.comdat_group (decl));
      }
--- 4925,4931 ----
  
        if (flags & SECTION_ENTSIZE)
  	fprintf (asm_out_file, ",%d", flags & SECTION_ENTSIZE);
!       if (HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE))
  	fprintf (asm_out_file, ",%s,comdat", 
  		 lang_hooks.decls.comdat_group (decl));
      }
*************** default_unique_section (tree decl, int r
*** 5226,5232 ****
  void
  default_unique_section_1 (tree decl, int reloc, int shlib)
  {
!   bool one_only = DECL_ONE_ONLY (decl);
    const char *prefix, *name;
    size_t nlen, plen;
    char *string;
--- 5237,5244 ----
  void
  default_unique_section_1 (tree decl, int reloc, int shlib)
  {
!   /* We only need to use .gnu.linkonce if we don't have COMDAT groups.  */
!   bool one_only = DECL_ONE_ONLY (decl) && !HAVE_COMDAT_GROUP;
    const char *prefix, *name;
    size_t nlen, plen;
    char *string;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]