public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Alan Modra <amodra@gmail.com>
To: Dave Korn <dave.korn.cygwin@gmail.com>
Cc: binutils@sourceware.org
Subject: Re: [PATCH,take 2,trunk+2.21.1] Re: Fix link order problem with LD plugin API.
Date: Sat, 05 Feb 2011 01:56:00 -0000	[thread overview]
Message-ID: <20110205015629.GC9489@bubble.grove.modra.org> (raw)
In-Reply-To: <20110203132835.GY9489@bubble.grove.modra.org>

On Thu, Feb 03, 2011 at 11:58:35PM +1030, Alan Modra wrote:
> Good.  That can hide, at least until runtime, lto bugs.  I've been
> poking a little at one today where a function was mentioned in the IR,
> called in the final object, but not emitted.  So ld quite happily sent
> the call to an IR symbol.
> 
> I have this in my tree to expose that particular problem, but haven't
> tested it properly yet.  Results in
> 
> ../../gcc-curr/gcc/xgcc -B../../gcc-curr/gcc/ -w -O2 -m32 -flto abs-2.i abs-2-lib.i main.i -lm
> `main_test' referenced in section `.text.startup' of /tmp/ccBT9axe.ltrans0.ltrans.o: defined in discarded section `.text' of /tmp/ccgfeeJG.o.ironly\x04
> collect2: ld returned 1 exit status

With this patch I get these failures

FAIL: plugin claimfile replace symbol
FAIL: plugin claimfile resolve symbol
FAIL: plugin set symbol visibility

The problem is that these tests result in "func" having no real
definition, just an IR one.  Also, in the last test none of the IR
symbols will be emitted in the final object, which makes it a little
dificult to check their visibilities.

However, unless I'm missing something, we really do want that
a) a reloc against an IR only sym should give an error, and
b) IR only syms should not be emitted in the final object.

So, what should I do here?

	* plugin.c (plugin_get_ir_dummy_bfd): Set SEC_EXCLUDE on dummy
	.text section, use newer bfd_make_section variant.  Error handling.
	Correct setting of gp size.
	(asymbol_from_plugin_symbol): Properly cast last arg of concat.
	(message): Likewise for ACONCAT.
	(get_symbols): Formatting.

Index: ld/plugin.c
===================================================================
RCS file: /cvs/src/src/ld/plugin.c,v
retrieving revision 1.22
diff -u -p -r1.22 plugin.c
--- ld/plugin.c	22 Jan 2011 10:16:29 -0000	1.22
+++ ld/plugin.c	4 Feb 2011 01:41:40 -0000
@@ -229,24 +229,29 @@ plugin_opt_plugin_arg (const char *arg)
 bfd *
 plugin_get_ir_dummy_bfd (const char *name, bfd *srctemplate)
 {
-  asection *sec;
   bfd *abfd;
 
   bfd_use_reserved_id = 1;
-  abfd = bfd_create (concat (name, IRONLY_SUFFIX, (const char *)NULL),
+  abfd = bfd_create (concat (name, IRONLY_SUFFIX, (const char *) NULL),
 		     srctemplate);
-  bfd_set_arch_info (abfd, bfd_get_arch_info (srctemplate));
-  bfd_make_writable (abfd);
-  bfd_copy_private_bfd_data (srctemplate, abfd);
-  bfd_set_gp_size (abfd, bfd_get_gp_size (abfd));
-  /* Create a minimal set of sections to own the symbols.  */
-  sec = bfd_make_section_old_way (abfd, ".text");
-  bfd_set_section_flags (abfd, sec,
-			 (SEC_CODE | SEC_HAS_CONTENTS | SEC_READONLY
-			  | SEC_ALLOC | SEC_LOAD | SEC_KEEP));
-  sec->output_section = sec;
-  sec->output_offset = 0;
-  return abfd;
+  if (abfd != NULL)
+    {
+      bfd_set_arch_info (abfd, bfd_get_arch_info (srctemplate));
+      bfd_set_gp_size (abfd, bfd_get_gp_size (srctemplate));
+      if (bfd_make_writable (abfd)
+	  && bfd_copy_private_bfd_data (srctemplate, abfd))
+	{
+	  flagword flags;
+
+	  /* Create a section to own the symbols.  */
+	  flags = (SEC_CODE | SEC_HAS_CONTENTS | SEC_READONLY
+		   | SEC_ALLOC | SEC_LOAD | SEC_KEEP | SEC_EXCLUDE);
+	  if (bfd_make_section_anyway_with_flags (abfd, ".text", flags) != NULL)
+	    return abfd;
+	}
+    }
+  einfo (_("could not create dummy IR bfd: %F%E\n"));
+  return NULL;
 }
 
 /* Check if the BFD passed in is an IR dummy object file.  */
@@ -273,7 +278,7 @@ asymbol_from_plugin_symbol (bfd *abfd, a
 
   asym->the_bfd = abfd;
   asym->name = (ldsym->version
-		? concat (ldsym->name, "@", ldsym->version, NULL)
+		? concat (ldsym->name, "@", ldsym->version, (const char *) NULL)
 		: ldsym->name);
   asym->value = 0;
   switch (ldsym->def)
@@ -488,16 +493,17 @@ get_symbols (const void *handle, int nsy
       /* Find out which section owns the symbol.  Since it's not undef,
 	 it must have an owner; if it's not a common symbol, both defs
 	 and weakdefs keep it in the same place. */
-      owner_sec = (blhe->type == bfd_link_hash_common)
-	? blhe->u.c.p->section
-	: blhe->u.def.section;
+      owner_sec = (blhe->type == bfd_link_hash_common
+		   ? blhe->u.c.p->section
+		   : blhe->u.def.section);
 
       /* We need to know if the sym is referenced from non-IR files.  Or
 	 even potentially-referenced, perhaps in a future final link if
 	 this is a partial one, perhaps dynamically at load-time if the
 	 symbol is externally visible.  */
-      ironly = !is_visible_from_outside (&syms[n], owner_sec, blhe)
-	&& !bfd_hash_lookup (non_ironly_hash, syms[n].name, FALSE, FALSE);
+      ironly = (!is_visible_from_outside (&syms[n], owner_sec, blhe)
+		&& bfd_hash_lookup (non_ironly_hash, syms[n].name,
+				    FALSE, FALSE) == NULL);
 
       /* If it was originally undefined or common, then it has been
 	 resolved; determine how.  */
@@ -535,9 +541,9 @@ get_symbols (const void *handle, int nsy
 	}
 
       /* Was originally def, weakdef, or common, but has been pre-empted.  */
-      syms[n].resolution = is_ir_dummy_bfd (owner_sec->owner)
-	? LDPR_PREEMPTED_IR
-	: LDPR_PREEMPTED_REG;
+      syms[n].resolution = (is_ir_dummy_bfd (owner_sec->owner)
+			    ? LDPR_PREEMPTED_IR
+			    : LDPR_PREEMPTED_REG);
     }
   return LDPS_OK;
 }
@@ -595,9 +601,8 @@ message (int level, const char *format, 
     case LDPL_ERROR:
     default:
 	{
-	  char *newfmt = ACONCAT ((level == LDPL_FATAL
-				   ? "%P%F: " : "%P%X: ",
-				   format, "\n", NULL));
+	  char *newfmt = ACONCAT ((level == LDPL_FATAL ? "%P%F: " : "%P%X: ",
+				   format, "\n", (const char *) NULL));
 	  fflush (stdout);
 	  vfinfo (stderr, newfmt, args, TRUE);
 	  fflush (stderr);

-- 
Alan Modra
Australia Development Lab, IBM

      reply	other threads:[~2011-02-05  1:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-29  1:23 [PATCH,trunk+2.21.1] " Dave Korn
2011-01-29  1:45 ` H.J. Lu
2011-01-29  2:01   ` H.J. Lu
2011-01-29  2:05     ` H.J. Lu
2011-01-29  2:11       ` Dave Korn
2011-01-29  2:45         ` H.J. Lu
2011-01-29  2:59           ` Dave Korn
2011-01-29  4:11       ` Dave Korn
2011-01-29  4:41         ` H.J. Lu
2011-01-31  1:57           ` [PATCH,take 2,trunk+2.21.1] " Dave Korn
2011-02-03  5:12             ` Alan Modra
2011-02-03  5:44               ` Dave Korn
2011-02-03 13:28                 ` Alan Modra
2011-02-05  1:56                   ` Alan Modra [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110205015629.GC9489@bubble.grove.modra.org \
    --to=amodra@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=dave.korn.cygwin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).