public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] fix PR51910, take 2
@ 2012-02-03  4:13 Sandra Loosemore
  2012-02-03  9:27 ` Jakub Jelinek
  0 siblings, 1 reply; 6+ messages in thread
From: Sandra Loosemore @ 2012-02-03  4:13 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jason Merrill, Jakub Jelinek

[-- Attachment #1: Type: text/plain, Size: 2104 bytes --]

Here is another attempt to fix the bad interaction between 
--with-demangler-in-ld and -frepo processing.

This version of the patch always disables demangling in ld when 
repository files are present for the link.  If demangling is requested 
(either by an explicit -Wl,--demangle option, by using 
COLLECT_NO_DEMANGLE, or just doing nothing and getting the default), 
it'll be performed by having collect2 filter the output streams no 
matter how you configured --with-demangler-in-ld.  The exception is if 
-Wl,-Map is also provided, in which case it falls back on re-running the 
final link with the correct options to get a demangled map file after it 
is done with the repo file processing.

I could have implemented that last bit by having collect2 intercept the 
map file option and filter the file to demangle it, and make that the 
default behavior when collect2 is otherwise doing demangling itself. 
Given the previous argument that users can't possibly have any reason to 
want a demangled link map, I thought there might be objection to 
polluting collect2 by hacking it up to do just that.  I mean, it's 
horrible enough that ld produces a demangled map file when you request 
demangled output....  :-P

Anyway, thoughts on this patch?  It bootstrapped and regression tested 
OK on i686-linux, and I hand-tested it on every permutation of 
mangling/repository/map options I could think of.

-Sandra


2012-02-02  Sandra Loosemore <sandra@codesourcery.com>
	    Jason Merrill <jason@redhat.com>
	    Jakub Jelinek <jakub@redhat.com>

	PR c++/51910
	gcc/
	* collect2.c (dump_file): Add demangle_in_ld parameter so this
	can be tested dynamically instead only at compile-time.  Update
	callers.
	(main): Initialize no_demangle even when HAVE_LD_DEMANGLE.
	* collect2.h (dump_file): Adjust declaration.
	(no_demangle): Delare as extern.
	(DEMANGLE_IN_LD): Define.
	* tlink.c (do_tlink): Explicitly pass --no-demangle to linker
	for repo processing when HAVE_LD_DEMANGLE is defined, and demangle
	in collect2 instead of the linker.

	gcc/testsuite/
	* g++.dg/torture/pr51910.C: New testcase.



[-- Attachment #2: pr51910-2.patch --]
[-- Type: text/x-patch, Size: 6458 bytes --]

Index: gcc/collect2.c
===================================================================
--- gcc/collect2.c	(revision 183674)
+++ gcc/collect2.c	(working copy)
@@ -403,13 +403,13 @@ collect_exit (int status)
 
   if (ldout != 0 && ldout[0])
     {
-      dump_file (ldout, stdout);
+      dump_file (ldout, stdout, DEMANGLE_IN_LD);
       maybe_unlink (ldout);
     }
 
   if (lderrout != 0 && lderrout[0])
     {
-      dump_file (lderrout, stderr);
+      dump_file (lderrout, stderr, DEMANGLE_IN_LD);
       maybe_unlink (lderrout);
     }
 
@@ -515,7 +515,7 @@ extract_string (const char **pp)
 }
 \f
 void
-dump_file (const char *name, FILE *to)
+dump_file (const char *name, FILE *to, int demangle_in_ld)
 {
   FILE *stream = fopen (name, "r");
 
@@ -540,14 +540,10 @@ dump_file (const char *name, FILE *to)
 	  if (!strncmp (p, USER_LABEL_PREFIX, strlen (USER_LABEL_PREFIX)))
 	    p += strlen (USER_LABEL_PREFIX);
 
-#ifdef HAVE_LD_DEMANGLE
-	  result = 0;
-#else
-	  if (no_demangle)
+	  if (no_demangle || demangle_in_ld)
 	    result = 0;
 	  else
 	    result = cplus_demangle (p, DMGL_PARAMS | DMGL_ANSI | DMGL_VERBOSE);
-#endif
 
 	  if (result)
 	    {
@@ -1114,9 +1110,8 @@ main (int argc, char **argv)
 
   num_c_args = argc + 9;
 
-#ifndef HAVE_LD_DEMANGLE
   no_demangle = !! getenv ("COLLECT_NO_DEMANGLE");
-
+#ifndef HAVE_LD_DEMANGLE
   /* Suppress demangling by the real linker, which may be broken.  */
   putenv (xstrdup ("COLLECT_NO_DEMANGLE=1"));
 #endif
@@ -1566,15 +1561,14 @@ main (int argc, char **argv)
 	    case '-':
 	      if (strcmp (arg, "--no-demangle") == 0)
 		{
-#ifndef HAVE_LD_DEMANGLE
 		  no_demangle = 1;
+#ifndef HAVE_LD_DEMANGLE
 		  ld1--;
 		  ld2--;
 #endif
 		}
 	      else if (strncmp (arg, "--demangle", 10) == 0)
 		{
-#ifndef HAVE_LD_DEMANGLE
 		  no_demangle = 0;
 		  if (arg[10] == '=')
 		    {
@@ -1585,6 +1579,7 @@ main (int argc, char **argv)
 		      else
 			current_demangling_style = style;
 		    }
+#ifndef HAVE_LD_DEMANGLE
 		  ld1--;
 		  ld2--;
 #endif
Index: gcc/collect2.h
===================================================================
--- gcc/collect2.h	(revision 183674)
+++ gcc/collect2.h	(working copy)
@@ -30,7 +30,7 @@ extern void collect_exit (int) ATTRIBUTE
 
 extern int collect_wait (const char *, struct pex_obj *);
 
-extern void dump_file (const char *, FILE *);
+extern void dump_file (const char *, FILE *, int);
 
 extern int file_exists (const char *);
 
@@ -40,6 +40,13 @@ extern const char *c_file_name;
 extern struct obstack temporary_obstack;
 extern char *temporary_firstobj;
 extern bool vflag, debug;
+extern int no_demangle;
+
+#ifdef HAVE_LD_DEMANGLE
+#define DEMANGLE_IN_LD 1
+#else
+#define DEMANGLE_IN_LD 0
+#endif
 
 extern void notice_translated (const char *, ...) ATTRIBUTE_PRINTF_1;
 extern void notice (const char *, ...) ATTRIBUTE_PRINTF_1;
Index: gcc/tlink.c
===================================================================
--- gcc/tlink.c	(revision 183674)
+++ gcc/tlink.c	(working copy)
@@ -766,8 +766,49 @@ scan_linker_output (const char *fname)
 void
 do_tlink (char **ld_argv, char **object_lst ATTRIBUTE_UNUSED)
 {
-  int exit = tlink_execute ("ld", ld_argv, ldout, lderrout);
+  int demangle_in_ld = DEMANGLE_IN_LD;
+  int map_file = FALSE;
+  int exit;
+  char **ld1_argv = ld_argv;
 
+#ifdef HAVE_LD_DEMANGLE
+  /* If there are repositories, proper processing of them depends on
+     the linker passing back mangled symbols.  So, explicitly tell the
+     linker not to demangle.  Also note whether the user requested a map
+     file, which requires special postprocessing.
+     We can skip this whole business if the user already asked
+     for non-demangled output.  */
+  if (!no_demangle)
+    {
+      int argc = 0;
+      int repo_files_exist = FALSE;
+      while (ld_argv[argc])
+	{
+	  if (*ld_argv[argc] == '-')
+	    {
+	      if (strncmp (ld_argv[argc], "-Map", 4) == 0)
+		map_file = TRUE;
+	    }
+	  else if (!repo_files_exist)
+	    {
+	      const char *p = frob_extension (ld_argv[argc], ".rpo");
+	      if (file_exists (p))
+		repo_files_exist = TRUE;
+	    }
+	  ++argc;
+	}
+      if (repo_files_exist)
+	{
+	  ld1_argv = XNEWVEC (char *, argc + 2);
+	  memcpy (ld1_argv, ld_argv, sizeof (ld_argv[0]) * argc);
+	  ld1_argv[argc] = CONST_CAST (char *, "--no-demangle");
+	  ld1_argv[argc + 1] = NULL;
+	  demangle_in_ld = FALSE;
+	}
+    }
+#endif
+
+  exit = tlink_execute ("ld", ld1_argv, ldout, lderrout);
   tlink_init ();
 
   if (exit)
@@ -781,8 +822,8 @@ do_tlink (char **ld_argv, char **object_
 	  {
 	    if (tlink_verbose >= 3)
 	      {
-		dump_file (ldout, stdout);
-		dump_file (lderrout, stderr);
+		dump_file (ldout, stdout, demangle_in_ld);
+		dump_file (lderrout, stderr, demangle_in_ld);
 	      }
 	    demangle_new_symbols ();
 	    if (! scan_linker_output (ldout)
@@ -792,13 +833,30 @@ do_tlink (char **ld_argv, char **object_
 	      break;
 	    if (tlink_verbose)
 	      fprintf (stderr, _("collect: relinking\n"));
-	    exit = tlink_execute ("ld", ld_argv, ldout, lderrout);
+	    exit = tlink_execute ("ld", ld1_argv, ldout, lderrout);
 	  }
     }
 
-  dump_file (ldout, stdout);
+#ifdef HAVE_LD_DEMANGLE
+  if (!demangle_in_ld)
+    {
+      XDELETEVEC (ld1_argv);
+      /* If the user specified all of demangling, a map file, and repositories,
+	 we must re-link once more with the original options to get the map
+	 file demangled. */
+      if (map_file)
+	{
+	  demangle_in_ld = TRUE;
+	  if (tlink_verbose)
+	    fprintf (stderr, _("collect: relinking\n"));
+	  exit = tlink_execute ("ld", ld_argv, ldout, lderrout);
+	}
+    }
+#endif
+
+  dump_file (ldout, stdout, demangle_in_ld);
   unlink (ldout);
-  dump_file (lderrout, stderr);
+  dump_file (lderrout, stderr, demangle_in_ld);
   unlink (lderrout);
   if (exit)
     {
Index: gcc/testsuite/g++.dg/torture/pr51910.C
===================================================================
--- gcc/testsuite/g++.dg/torture/pr51910.C	(revision 0)
+++ gcc/testsuite/g++.dg/torture/pr51910.C	(revision 0)
@@ -0,0 +1,19 @@
+// PR c++/51910 
+// Check that -frepo works in the presence of linker symbol demangling.
+//
+// { dg-options "-frepo -Wl,--demangle" }
+// { dg-require-host-local "" }
+// { dg-skip-if "dkms are not final links" { vxworks_kernel } }
+
+template<typename T>
+struct Foo
+{
+  virtual ~Foo() { }
+};
+
+int main( int, char*[] )
+{
+  Foo<int> test;
+}
+
+// { dg-final { cleanup-repo-files } }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fix PR51910, take 2
  2012-02-03  4:13 [PATCH] fix PR51910, take 2 Sandra Loosemore
@ 2012-02-03  9:27 ` Jakub Jelinek
  2012-02-03 12:03   ` Jakub Jelinek
  2012-02-08  9:44   ` Jason Merrill
  0 siblings, 2 replies; 6+ messages in thread
From: Jakub Jelinek @ 2012-02-03  9:27 UTC (permalink / raw)
  To: Sandra Loosemore; +Cc: gcc-patches, Jason Merrill

On Thu, Feb 02, 2012 at 09:10:50PM -0700, Sandra Loosemore wrote:
> --- gcc/testsuite/g++.dg/torture/pr51910.C	(revision 0)
> +++ gcc/testsuite/g++.dg/torture/pr51910.C	(revision 0)
> @@ -0,0 +1,19 @@
> +// PR c++/51910 
> +// Check that -frepo works in the presence of linker symbol demangling.
> +//
> +// { dg-options "-frepo -Wl,--demangle" }

I don't think you want to use this in the testsuite, that assumes the
linker handles it, which is not the case for non-GNU linkers.

Anyway, here is the tweaking all the symbols that demangle the same
equally patch (untested so far) as an alternative.  On the example it
unfortunately causes also the D2 dtor into the link which wasn't there
otherwise (and with -Wl,--no-demangle).  While D2 dtors are generally small,
perhaps it will sometimes force into the link also ctor or dtor variants
that couldn't be aliased together.

I wonder if it wouldn't be better to just use a linker plugin for -frepo
if the linker supports plugins.

2012-02-03  Jakub Jelinek  <jakub@redhat.com>

	PR c++/51910
	* tlink.c (demangleq_equiv): New typedef.
	(demangled): Add equiv field.
	(demangle_new_symbols): Chain mangled symbols that demangle the
	same into dem->equiv chain.
	(handle_sym): New function.
	(scan_linker_output): Use it.  Call handle_sym on all other symbols
	that demangle the same.

--- gcc/tlink.c.jj	2012-01-30 00:08:28.000000000 +0100
+++ gcc/tlink.c	2012-02-03 10:13:08.799161429 +0100
@@ -67,10 +67,17 @@ typedef struct file_hash_entry
   int tweaking;
 } file;
 
+typedef struct demangled_equiv_entry
+{
+  const char *mangled;
+  struct demangled_equiv_entry *next;
+} demangled_equiv;
+
 typedef struct demangled_hash_entry
 {
   const char *key;
   const char *mangled;
+  demangled_equiv *equiv;
 } demangled;
 
 /* Hash and comparison functions for these hash tables.  */
@@ -598,10 +605,43 @@ demangle_new_symbols (void)
 	continue;
 
       dem = demangled_hash_lookup (p, true);
-      dem->mangled = sym->key;
+      if (dem->mangled)
+	{
+	  demangled_equiv *equiv = XCNEW (demangled_equiv);
+	  equiv->mangled = sym->key;
+	  equiv->next = dem->equiv;
+	  dem->equiv = equiv;
+	}
+      else
+        dem->mangled = sym->key;
     }
 }
 
+/* Helper function of scan_linker_output.  Handle a single symbol SYM.
+   Returns 1 if an error happened, 0 otherwise.  */
+
+static bool
+handle_sym (FILE *stream, symbol *sym)
+{
+  if (sym && sym->tweaked)
+    {
+      error ("'%s' was assigned to '%s', but was not defined "
+	     "during recompilation, or vice versa",
+	     sym->key, sym->file->key);
+      fclose (stream);
+      return true;
+    }
+  if (sym && !sym->tweaking)
+    {
+      if (tlink_verbose >= 2)
+	fprintf (stderr, _("collect: tweaking %s in %s\n"),
+		 sym->key, sym->file->key);
+      sym->tweaking = 1;
+      file_push (sym->file);
+    }
+  return false;
+}
+
 /* Step through the output of the linker, in the file named FNAME, and
    adjust the settings for each symbol encountered.  */
 
@@ -719,7 +759,19 @@ scan_linker_output (const char *fname)
 	      *q = 0;
 	      dem = demangled_hash_lookup (p, false);
 	      if (dem)
-		sym = symbol_hash_lookup (dem->mangled, false);
+		{
+		  if (dem->equiv)
+		    {
+		      demangled_equiv *equiv;
+		      for (equiv = dem->equiv; equiv; equiv = equiv->next)
+			{
+			  sym = symbol_hash_lookup (equiv->mangled, false);
+			  if (handle_sym (stream, sym))
+			    return 0;
+			}
+		    }
+		  sym = symbol_hash_lookup (dem->mangled, false);
+		}
 	      else
 		{
 		  if (!strncmp (p, USER_LABEL_PREFIX,
@@ -730,22 +782,8 @@ scan_linker_output (const char *fname)
 	    }
 	}
 
-      if (sym && sym->tweaked)
-	{
-	  error ("'%s' was assigned to '%s', but was not defined "
-		 "during recompilation, or vice versa",
-		 sym->key, sym->file->key);
-	  fclose (stream);
-	  return 0;
-	}
-      if (sym && !sym->tweaking)
-	{
-	  if (tlink_verbose >= 2)
-	    fprintf (stderr, _("collect: tweaking %s in %s\n"),
-		     sym->key, sym->file->key);
-	  sym->tweaking = 1;
-	  file_push (sym->file);
-	}
+      if (handle_sym (stream, sym))
+	return 0;
 
       obstack_free (&temporary_obstack, temporary_firstobj);
     }

	Jakub

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fix PR51910, take 2
  2012-02-03  9:27 ` Jakub Jelinek
@ 2012-02-03 12:03   ` Jakub Jelinek
  2012-02-08  9:44   ` Jason Merrill
  1 sibling, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2012-02-03 12:03 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Sandra Loosemore, gcc-patches

On Fri, Feb 03, 2012 at 10:27:17AM +0100, Jakub Jelinek wrote:
> Anyway, here is the tweaking all the symbols that demangle the same
> equally patch (untested so far) as an alternative.  On the example it

Now bootstrapped/regtested on x86_64-linux and i686-linux.

	Jakub

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fix PR51910, take 2
  2012-02-03  9:27 ` Jakub Jelinek
  2012-02-03 12:03   ` Jakub Jelinek
@ 2012-02-08  9:44   ` Jason Merrill
  2012-02-08 13:11     ` Jakub Jelinek
  1 sibling, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2012-02-08  9:44 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Sandra Loosemore, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

On 02/02/2012 11:27 PM, Jakub Jelinek wrote:
> Anyway, here is the tweaking all the symbols that demangle the same
> equally patch (untested so far) as an alternative.  On the example it
> unfortunately causes also the D2 dtor into the link which wasn't there
> otherwise (and with -Wl,--no-demangle).  While D2 dtors are generally small,
> perhaps it will sometimes force into the link also ctor or dtor variants
> that couldn't be aliased together.

Hmm, I wrote up something quite similar on the plane.  One difference is 
that mine synchronizes .rpo files that start with some variants chosen 
and others not.  Does this make sense to you?

> I wonder if it wouldn't be better to just use a linker plugin for -frepo
> if the linker supports plugins.

Probably.  But I don't feel motivated to write it.

Jason

[-- Attachment #2: 51910.patch --]
[-- Type: text/x-patch, Size: 5843 bytes --]

commit 1f4b0d273cfd81948762d3922cdafac40a06a2cf
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Feb 2 00:07:44 2012 -0500

    	PR c++/51910
    	* tlink.c (demangled_hash_entry): Change mangled to a VEC.
    	(demangle_new_symbols): Fill it.
    	(scan_linker_output): Walk it.
    	(start_tweaking): Split out from scan_linker_output.
    	(maybe_tweak): Update sym->chosen.
    	* Makefile.in (COLLECT2_OBJS): Add vec.o and gcc-none.o

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index c9ecc4b..486538d 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1946,7 +1946,7 @@ gcc-ranlib.c: gcc-ar.c
 gcc-nm.c: gcc-ar.c
 	cp $^ $@
 
-COLLECT2_OBJS = collect2.o collect2-aix.o tlink.o
+COLLECT2_OBJS = collect2.o collect2-aix.o tlink.o vec.o ggc-none.o
 COLLECT2_LIBS = @COLLECT2_LIBS@
 collect2$(exeext): $(COLLECT2_OBJS) $(LIBDEPS)
 # Don't try modifying collect2 (aka ld) in place--it might be linking this.
diff --git a/gcc/testsuite/g++.dg/template/repo10.C b/gcc/testsuite/g++.dg/template/repo10.C
new file mode 100644
index 0000000..c92f7a5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/repo10.C
@@ -0,0 +1,16 @@
+// PR c++/51910
+// { dg-options -frepo }
+// { dg-require-host-local "" }
+// { dg-skip-if "dkms are not final links" { vxworks_kernel } }
+// { dg-final cleanup-repo-files }
+
+template<typename T>
+struct Foo
+{
+  virtual ~Foo() { }
+};
+
+int main( int, char*[] )
+{
+  Foo<int> test;
+}
diff --git a/gcc/tlink.c b/gcc/tlink.c
index f054047..67c7086 100644
--- a/gcc/tlink.c
+++ b/gcc/tlink.c
@@ -32,6 +32,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "collect2.h"
 #include "filenames.h"
 #include "diagnostic-core.h"
+#include "vec.h"
 
 /* TARGET_64BIT may be defined to use driver specific functionality. */
 #undef TARGET_64BIT
@@ -67,10 +68,14 @@ typedef struct file_hash_entry
   int tweaking;
 } file;
 
+typedef const char *str;
+DEF_VEC_P(str);
+DEF_VEC_ALLOC_P(str,heap);
+
 typedef struct demangled_hash_entry
 {
   const char *key;
-  const char *mangled;
+  VEC(str,heap) *mangled;
 } demangled;
 
 /* Hash and comparison functions for these hash tables.  */
@@ -435,9 +440,15 @@ maybe_tweak (char *line, file *f)
       sym->tweaked = 1;
 
       if (line[0] == 'O')
-	line[0] = 'C';
+	{
+	  line[0] = 'C';
+	  sym->chosen = 1;
+	}
       else
-	line[0] = 'O';
+	{
+	  line[0] = 'O';
+	  sym->chosen = 0;
+	}
     }
 }
 
@@ -598,10 +609,34 @@ demangle_new_symbols (void)
 	continue;
 
       dem = demangled_hash_lookup (p, true);
-      dem->mangled = sym->key;
+      VEC_safe_push (str, heap, dem->mangled, sym->key);
     }
 }
 
+/* We want to tweak symbol SYM.  Return true if all is well, false on
+   error.  */
+
+static bool
+start_tweaking (symbol *sym)
+{
+  if (sym && sym->tweaked)
+    {
+      error ("'%s' was assigned to '%s', but was not defined "
+	     "during recompilation, or vice versa",
+	     sym->key, sym->file->key);
+      return 0;
+    }
+  if (sym && !sym->tweaking)
+    {
+      if (tlink_verbose >= 2)
+	fprintf (stderr, _("collect: tweaking %s in %s\n"),
+		 sym->key, sym->file->key);
+      sym->tweaking = 1;
+      file_push (sym->file);
+    }
+  return true;
+}
+
 /* Step through the output of the linker, in the file named FNAME, and
    adjust the settings for each symbol encountered.  */
 
@@ -616,8 +651,11 @@ scan_linker_output (const char *fname)
     {
       char *p = line, *q;
       symbol *sym;
+      demangled *dem = 0;
       int end;
       int ok = 0;
+      unsigned ix;
+      str s;
 
       /* On darwin9, we might have to skip " in " lines as well.  */
       if (skip_next_in_line
@@ -662,7 +700,6 @@ scan_linker_output (const char *fname)
 	/* Try a mangled name in quotes.  */
 	{
 	  char *oldq = q + 1;
-	  demangled *dem = 0;
 	  q = 0;
 
 	  /* On darwin9, we look for "foo" referenced from:\n\(.* in .*\n\)*  */
@@ -718,9 +755,7 @@ scan_linker_output (const char *fname)
 	    {
 	      *q = 0;
 	      dem = demangled_hash_lookup (p, false);
-	      if (dem)
-		sym = symbol_hash_lookup (dem->mangled, false);
-	      else
+	      if (!dem)
 		{
 		  if (!strncmp (p, USER_LABEL_PREFIX,
 				strlen (USER_LABEL_PREFIX)))
@@ -730,24 +765,43 @@ scan_linker_output (const char *fname)
 	    }
 	}
 
-      if (sym && sym->tweaked)
+      if (dem)
+	{
+	  /* We found a demangled name.  If this is the name of a
+	     constructor or destructor, there can be several mangled names
+	     that match it, so choose or unchoose all of them.  If some are
+	     chosen and some not, leave the later ones that don't match
+	     alone for now; either this will cause the link to suceed, or
+	     on the next attempt we will switch all of them the other way
+	     and that will cause it to succeed.  */
+	  int chosen = 0;
+	  int len = VEC_length (str, dem->mangled);
+	  ok = true;
+	  FOR_EACH_VEC_ELT (str, dem->mangled, ix, s)
+	    {
+	      sym = symbol_hash_lookup (s, false);
+	      if (ix == 0)
+		chosen = sym->chosen;
+	      else if (sym->chosen != chosen)
+		/* Mismatch.  */
+		continue;
+	      /* Avoid an error about re-tweaking when we guess wrong in
+		 the case of mismatch.  */
+	      if (len > 1)
+		sym->tweaked = false;
+	      ok = start_tweaking (sym);
+	    }
+	}
+      else
+	ok = start_tweaking (sym);
+
+      obstack_free (&temporary_obstack, temporary_firstobj);
+
+      if (!ok)
 	{
-	  error ("'%s' was assigned to '%s', but was not defined "
-		 "during recompilation, or vice versa",
-		 sym->key, sym->file->key);
 	  fclose (stream);
 	  return 0;
 	}
-      if (sym && !sym->tweaking)
-	{
-	  if (tlink_verbose >= 2)
-	    fprintf (stderr, _("collect: tweaking %s in %s\n"),
-		     sym->key, sym->file->key);
-	  sym->tweaking = 1;
-	  file_push (sym->file);
-	}
-
-      obstack_free (&temporary_obstack, temporary_firstobj);
     }
 
   fclose (stream);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fix PR51910, take 2
  2012-02-08  9:44   ` Jason Merrill
@ 2012-02-08 13:11     ` Jakub Jelinek
  2012-02-11  8:57       ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2012-02-08 13:11 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Sandra Loosemore, gcc-patches

On Tue, Feb 07, 2012 at 11:41:03PM -1000, Jason Merrill wrote:
> Hmm, I wrote up something quite similar on the plane.  One
> difference is that mine synchronizes .rpo files that start with some
> variants chosen and others not.  Does this make sense to you?

You mean the maybe_tweak change?  Not sure I understand it.

Anyway, using VEC is probably cleaner than my patch, on the other
side might be more expensive for the common case where most
symbols don't demangle the same like any other symbol,
because then you allocate a VEC (I think 4 pointers) for each of the
hash table entries, i.e. one extra allocation for each.
> 
> >I wonder if it wouldn't be better to just use a linker plugin for -frepo
> >if the linker supports plugins.
> 
> Probably.  But I don't feel motivated to write it.

Me neither ;).
Anyway, the question is if the increases in object sizes with -frepo
(due to bringing in unused ctor or dtor variants from time to time)
will be acceptable to -frepo users (if there are any).

	Jakub

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] fix PR51910, take 2
  2012-02-08 13:11     ` Jakub Jelinek
@ 2012-02-11  8:57       ` Jason Merrill
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2012-02-11  8:57 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Sandra Loosemore, gcc-patches

On 02/08/2012 03:01 AM, Jakub Jelinek wrote:
> On Tue, Feb 07, 2012 at 11:41:03PM -1000, Jason Merrill wrote:
>> Hmm, I wrote up something quite similar on the plane.  One
>> difference is that mine synchronizes .rpo files that start with some
>> variants chosen and others not.  Does this make sense to you?
>
> You mean the maybe_tweak change?  Not sure I understand it.

That and some of the changes in scan_linker_output as well.

> Anyway, using VEC is probably cleaner than my patch, on the other
> side might be more expensive for the common case where most
> symbols don't demangle the same like any other symbol,
> because then you allocate a VEC (I think 4 pointers) for each of the
> hash table entries, i.e. one extra allocation for each.

It looks like a VEC adds two unsigned ints, whereas your patch adds one 
pointer; not a big difference.

> Anyway, the question is if the increases in object sizes with -frepo
> (due to bringing in unused ctor or dtor variants from time to time)
> will be acceptable to -frepo users (if there are any).

I suppose we would run into this with classes with virtual bases that 
are not themselves derived from; we wouldn't need the base variant in 
that case.  So this changes from a link-failure regression to, I 
suppose, missed-optimization.  I'm OK with that.

Jason

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-02-11  8:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-03  4:13 [PATCH] fix PR51910, take 2 Sandra Loosemore
2012-02-03  9:27 ` Jakub Jelinek
2012-02-03 12:03   ` Jakub Jelinek
2012-02-08  9:44   ` Jason Merrill
2012-02-08 13:11     ` Jakub Jelinek
2012-02-11  8:57       ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).