[PATCH] PR ld/16746: Don't issue a warning for reference in LTO IR

[PATCH] PR ld/16746: Don't issue a warning for reference in LTO IR

[H.J. Lu]

References in LTO IR may be removed by LTO later.  Instead, make a
warning symbol if needed.  Warnings will be handled after LTO resolves
all references.  OK to install?

Thanks.


H.J.
---
bfd/

	PR ld/16746
	* linker.c (_bfd_generic_link_add_one_symbol): Don't issue a
	warning for reference in LTO IR which may be removed by LTO later.

ld/testsuite/

	PR ld/16746
	* ld-plugin/lto.exp: Add 4 tests for PR ld/16746.
	* ld-plugin/pr16746a.c: New file.
	* ld-plugin/pr16746b.c: Likewise.
	* ld-plugin/pr16746c.c: Likewise.
	* ld-plugin/pr16746d.c: Likewise.
---
 bfd/ChangeLog                     |  6 ++++++
 bfd/linker.c                      | 12 ++++++++++--
 ld/testsuite/ChangeLog            |  9 +++++++++
 ld/testsuite/ld-plugin/lto.exp    | 26 ++++++++++++++++++++++++++
 ld/testsuite/ld-plugin/pr16746a.c |  3 +++
 ld/testsuite/ld-plugin/pr16746b.c |  5 +++++
 ld/testsuite/ld-plugin/pr16746c.c |  8 ++++++++
 ld/testsuite/ld-plugin/pr16746d.c |  8 ++++++++
 8 files changed, 75 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-plugin/pr16746a.c
 create mode 100644 ld/testsuite/ld-plugin/pr16746b.c
 create mode 100644 ld/testsuite/ld-plugin/pr16746c.c
 create mode 100644 ld/testsuite/ld-plugin/pr16746d.c

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index c719d24..630c5a8 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,9 @@
+2014-03-25  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR ld/16746
+	* linker.c (_bfd_generic_link_add_one_symbol): Don't issue a
+	warning for reference in LTO IR which may be removed by LTO later.
+
 2014-03-25  Will Newton  <will.newton@linaro.org>
 
 	 * elfnn-aarch64.c (elfNN_aarch64_finish_dynamic_sections):
diff --git a/bfd/linker.c b/bfd/linker.c
index a20a276..4eeb363 100644
--- a/bfd/linker.c
+++ b/bfd/linker.c
@@ -1902,8 +1902,10 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info,
 	  break;
 
 	case WARNC:
-	  /* Issue a warning and cycle.  */
-	  if (h->u.i.warning != NULL)
+	  /* Issue a warning and cycle.  Don't issue a warning for
+	     reference in LTO IR which may be removed by LTO later. */
+	  if (h->u.i.warning != NULL
+	      && (abfd->flags & BFD_PLUGIN) == 0)
 	    {
 	      if (! (*info->callbacks->warning) (info, h->u.i.warning,
 						 h->root.string, abfd,
@@ -1928,6 +1930,11 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info,
 	  break;
 
 	case WARN:
+	  /* Don't issue a warning for reference in LTO IR which may be
+	     removed by LTO later.  Make a warning symbol instead.  */
+	  if ((hash_entry_bfd (h)->flags & BFD_PLUGIN) != 0)
+	    goto mwarn;
+
 	  /* Issue a warning.  */
 	  if (! (*info->callbacks->warning) (info, string, h->root.string,
 					     hash_entry_bfd (h), NULL, 0))
@@ -1949,6 +1956,7 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info,
 	    }
 	  /* Fall through.  */
 	case MWARN:
+mwarn:
 	  /* Make a warning symbol.  */
 	  {
 	    struct bfd_link_hash_entry *sub;
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index d200277..21df737 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,3 +1,12 @@
+2014-03-25  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR ld/16746
+	* ld-plugin/lto.exp: Add 4 tests for PR ld/16746.
+	* ld-plugin/pr16746a.c: New file.
+	* ld-plugin/pr16746b.c: Likewise.
+	* ld-plugin/pr16746c.c: Likewise.
+	* ld-plugin/pr16746d.c: Likewise.
+
 2014-03-25  Will Newton  <will.newton@linaro.org>
 
 	 * ld-aarch64/aarch64-elf.exp: Add relasz dump test.
diff --git a/ld/testsuite/ld-plugin/lto.exp b/ld/testsuite/ld-plugin/lto.exp
index af7307f..c37ecf9 100644
--- a/ld/testsuite/ld-plugin/lto.exp
+++ b/ld/testsuite/ld-plugin/lto.exp
@@ -205,6 +205,18 @@ set lto_link_elf_tests {
   {"Build libpr15146d.a"
    "" "-flto -O2"
    {pr15146d.c} {} "lib15146d.a"}
+  {"Build libpr16746a.a"
+   "" ""
+   {pr16746a.c pr16746b.c} {} "lib15146a.a"}
+  {"Build libpr16746b.a"
+   "" "-O2 -flto"
+   {pr16746c.c pr16746d.c} {} "lib15146b.a"}
+  {"PR ld/16746 (1)"
+   "-O2 -flto -fuse-linker-plugin tmpdir/pr16746a.o tmpdir/pr16746c.o" "-O2 -flto"
+   {dummy.c} {} "pr16746a.exe"}
+  {"PR ld/16746 (2)"
+   "-O2 -flto -fuse-linker-plugin tmpdir/pr16746c.o tmpdir/pr16746a.o" "-O2 -flto"
+   {dummy.c} {} "pr16746b.exe"}
 }
 
 # Check final symbols in executables.
@@ -308,6 +320,20 @@ if { [is_elf_format] && [check_lto_shared_available] } {
     } {
 	fail $testname
     }
+    set testname "PR ld/16746 (3)"
+    set exec_output [run_host_cmd "$CC" "$gcc_gas_flag $gcc_ld_flag -O2 -flto -fuse-linker-plugin tmpdir/pr16746b.o tmpdir/pr16746d.o"]
+    if { [ regexp "warning: foobar" $exec_output ] && ![ regexp "symbol from plugin" $exec_output ] } {
+	pass $testname
+    } {
+	fail $testname
+    }
+    set testname "PR ld/16746 (4)"
+    set exec_output [run_host_cmd "$CC" "$gcc_gas_flag $gcc_ld_flag -O2 -flto -fuse-linker-plugin tmpdir/pr16746d.o tmpdir/pr16746b.o"]
+    if { [ regexp "warning: foobar" $exec_output ] && ![ regexp "symbol from plugin" $exec_output ] } {
+	pass $testname
+    } {
+	fail $testname
+    }
 }
 
 set testname "Build liblto-11.a"
diff --git a/ld/testsuite/ld-plugin/pr16746a.c b/ld/testsuite/ld-plugin/pr16746a.c
new file mode 100644
index 0000000..1705ef1
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr16746a.c
@@ -0,0 +1,3 @@
+static const char __evoke_link_warning_foobar[]
+ __attribute__ ((used, section (".gnu.warning.foobar\n\t#")))
+ = "foobar";
diff --git a/ld/testsuite/ld-plugin/pr16746b.c b/ld/testsuite/ld-plugin/pr16746b.c
new file mode 100644
index 0000000..c3b7a78
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr16746b.c
@@ -0,0 +1,5 @@
+static const char __evoke_link_warning_foobar[]
+ __attribute__ ((used, section (".gnu.warning.foobar\n\t#")))
+ = "foobar";
+
+void foobar (void) {}
diff --git a/ld/testsuite/ld-plugin/pr16746c.c b/ld/testsuite/ld-plugin/pr16746c.c
new file mode 100644
index 0000000..fb68ab7
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr16746c.c
@@ -0,0 +1,8 @@
+extern void foobar (void);
+int
+main (int argc, char **argv)
+{
+  if (__builtin_constant_p (argc))
+    foobar ();
+  return 0;
+}
diff --git a/ld/testsuite/ld-plugin/pr16746d.c b/ld/testsuite/ld-plugin/pr16746d.c
new file mode 100644
index 0000000..1fee4dd
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr16746d.c
@@ -0,0 +1,8 @@
+extern void foobar (void);
+int
+main (int argc, char **argv)
+{
+  if (!__builtin_constant_p (argc))
+    foobar ();
+  return 0;
+}
-- 
1.8.5.3

Re: [PATCH] PR ld/16746: Don't issue a warning for reference in LTO IR

[Alan Modra]

On Tue, Mar 25, 2014 at 01:39:52PM -0700, H.J. Lu wrote:
>  	case WARNC:
> -	  /* Issue a warning and cycle.  */
> -	  if (h->u.i.warning != NULL)
> +	  /* Issue a warning and cycle.  Don't issue a warning for
> +	     reference in LTO IR which may be removed by LTO later. */
> +	  if (h->u.i.warning != NULL
> +	      && (abfd->flags & BFD_PLUGIN) == 0)

So here you're handling references in LTO IR for warning symbols
defined in real object files or other LTO IR files..

>  	case WARN:
> +	  /* Don't issue a warning for reference in LTO IR which may be
> +	     removed by LTO later.  Make a warning symbol instead.  */
> +	  if ((hash_entry_bfd (h)->flags & BFD_PLUGIN) != 0)
> +	    goto mwarn;
> +

And here you have the case where the new symbol is a warning symbol,
in either LTO IR or real object files, but the reference was in an LTO
IR file.

What about the CWARN case, where you already have a definition that
may have been referenced, and the new symbol is a warning symbol?
You'll need to test h->non_ir_ref I think.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] PR ld/16746: Don't issue a warning for reference in LTO IR

[H.J. Lu]

On Tue, Mar 25, 2014 at 3:39 PM, Alan Modra <amodra@gmail.com> wrote:
> On Tue, Mar 25, 2014 at 01:39:52PM -0700, H.J. Lu wrote:
>>       case WARNC:
>> -       /* Issue a warning and cycle.  */
>> -       if (h->u.i.warning != NULL)
>> +       /* Issue a warning and cycle.  Don't issue a warning for
>> +          reference in LTO IR which may be removed by LTO later. */
>> +       if (h->u.i.warning != NULL
>> +           && (abfd->flags & BFD_PLUGIN) == 0)
>
> So here you're handling references in LTO IR for warning symbols
> defined in real object files or other LTO IR files..

Yes.

>>       case WARN:
>> +       /* Don't issue a warning for reference in LTO IR which may be
>> +          removed by LTO later.  Make a warning symbol instead.  */
>> +       if ((hash_entry_bfd (h)->flags & BFD_PLUGIN) != 0)
>> +         goto mwarn;
>> +
>
> And here you have the case where the new symbol is a warning symbol,
> in either LTO IR or real object files, but the reference was in an LTO
> IR file.

Yes.

> What about the CWARN case, where you already have a definition that
> may have been referenced, and the new symbol is a warning symbol?
> You'll need to test h->non_ir_ref I think.

It is:

https://sourceware.org/bugzilla/show_bug.cgi?id=12760

We are getting:

[hjl@gnu-6 pr12760]$ cat pr12760a.c
extern void bar ();

void
foo ()
{
  bar ();
}
[hjl@gnu-6 pr12760]$ cat pr12760b.c
#define linker_warning(x, msg) \
static const char __warn_##x[] \
__attribute__((used, section(".gnu.warning." #x))) \
= msg

void bar (void) {}
linker_warning(bar, "Bad bar");
[hjl@gnu-6 pr12760]$ make
gcc -B./    -c -o pr12760a.o pr12760a.c
gcc -B./ -flto -O2 -ffat-lto-objects   -c -o pr12760b.o pr12760b.c
ar rcs libpr12760.a pr12760b.o
gcc -B./ -O2 -Wl,-e,foo -nostdlib -flto -fuse-linker-plugin -o foo
pr12760a.o -Wl,--start-group libpr12760.a -Wl,--end-group
pr12760b.o (symbol from plugin): warning: Bad bar
[hjl@gnu-6 pr12760]$

It is similar to:

https://sourceware.org/bugzilla/show_bug.cgi?id=16756

Since we don't keep the undefined reference when seeing
a definition, we lose the reference info.  We can't get the
correct warning message.

-- 
H.J.

Re: [PATCH] PR ld/16746: Don't issue a warning for reference in LTO IR

[Alan Modra]

On Tue, Mar 25, 2014 at 03:50:05PM -0700, H.J. Lu wrote:
> On Tue, Mar 25, 2014 at 3:39 PM, Alan Modra <amodra@gmail.com> wrote:
> > On Tue, Mar 25, 2014 at 01:39:52PM -0700, H.J. Lu wrote:
> >>       case WARNC:
> >> -       /* Issue a warning and cycle.  */
> >> -       if (h->u.i.warning != NULL)
> >> +       /* Issue a warning and cycle.  Don't issue a warning for
> >> +          reference in LTO IR which may be removed by LTO later. */
> >> +       if (h->u.i.warning != NULL
> >> +           && (abfd->flags & BFD_PLUGIN) == 0)
> >
> > So here you're handling references in LTO IR for warning symbols
> > defined in real object files or other LTO IR files..
> 
> Yes.
> 
> >>       case WARN:
> >> +       /* Don't issue a warning for reference in LTO IR which may be
> >> +          removed by LTO later.  Make a warning symbol instead.  */
> >> +       if ((hash_entry_bfd (h)->flags & BFD_PLUGIN) != 0)
> >> +         goto mwarn;
> >> +
> >
> > And here you have the case where the new symbol is a warning symbol,
> > in either LTO IR or real object files, but the reference was in an LTO
> > IR file.
> 
> Yes.
> 
> > What about the CWARN case, where you already have a definition that
> > may have been referenced, and the new symbol is a warning symbol?
> > You'll need to test h->non_ir_ref I think.
> 
> It is:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=12760

That's a different issue, isn't it?  I'm talking about the sequence
1a) symbol referenced,
2a) symbol defined,
3a) warning symbol defined.
or
1b) symbol defined,
2b) symbol referenced,
3b) warning symbol defined.

If (1a) or (2b) is an IR reference then we don't want to warn.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] PR ld/16746: Don't issue a warning for reference in LTO IR

[H.J. Lu]

On Tue, Mar 25, 2014 at 4:52 PM, Alan Modra <amodra@gmail.com> wrote:
> On Tue, Mar 25, 2014 at 03:50:05PM -0700, H.J. Lu wrote:
>> On Tue, Mar 25, 2014 at 3:39 PM, Alan Modra <amodra@gmail.com> wrote:
>> > On Tue, Mar 25, 2014 at 01:39:52PM -0700, H.J. Lu wrote:
>> >>       case WARNC:
>> >> -       /* Issue a warning and cycle.  */
>> >> -       if (h->u.i.warning != NULL)
>> >> +       /* Issue a warning and cycle.  Don't issue a warning for
>> >> +          reference in LTO IR which may be removed by LTO later. */
>> >> +       if (h->u.i.warning != NULL
>> >> +           && (abfd->flags & BFD_PLUGIN) == 0)
>> >
>> > So here you're handling references in LTO IR for warning symbols
>> > defined in real object files or other LTO IR files..
>>
>> Yes.
>>
>> >>       case WARN:
>> >> +       /* Don't issue a warning for reference in LTO IR which may be
>> >> +          removed by LTO later.  Make a warning symbol instead.  */
>> >> +       if ((hash_entry_bfd (h)->flags & BFD_PLUGIN) != 0)
>> >> +         goto mwarn;
>> >> +
>> >
>> > And here you have the case where the new symbol is a warning symbol,
>> > in either LTO IR or real object files, but the reference was in an LTO
>> > IR file.
>>
>> Yes.
>>
>> > What about the CWARN case, where you already have a definition that
>> > may have been referenced, and the new symbol is a warning symbol?
>> > You'll need to test h->non_ir_ref I think.
>>
>> It is:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=12760
>
> That's a different issue, isn't it?  I'm talking about the sequence
> 1a) symbol referenced,
> 2a) symbol defined,
> 3a) warning symbol defined.
> or
> 1b) symbol defined,
> 2b) symbol referenced,
> 3b) warning symbol defined.
>
> If (1a) or (2b) is an IR reference then we don't want to warn.

The symbol is referenced and used even if it has been inlined.
Testing h->non_ir_ref isn't sufficient.  We also want to know
if it is used in LTO.

-- 
H.J.

Re: [PATCH] PR ld/16746: Don't issue a warning for reference in LTO IR

[H.J. Lu]

On Tue, Mar 25, 2014 at 5:41 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Mar 25, 2014 at 4:52 PM, Alan Modra <amodra@gmail.com> wrote:
>> On Tue, Mar 25, 2014 at 03:50:05PM -0700, H.J. Lu wrote:
>>> On Tue, Mar 25, 2014 at 3:39 PM, Alan Modra <amodra@gmail.com> wrote:
>>> > On Tue, Mar 25, 2014 at 01:39:52PM -0700, H.J. Lu wrote:
>>> >>       case WARNC:
>>> >> -       /* Issue a warning and cycle.  */
>>> >> -       if (h->u.i.warning != NULL)
>>> >> +       /* Issue a warning and cycle.  Don't issue a warning for
>>> >> +          reference in LTO IR which may be removed by LTO later. */
>>> >> +       if (h->u.i.warning != NULL
>>> >> +           && (abfd->flags & BFD_PLUGIN) == 0)
>>> >
>>> > So here you're handling references in LTO IR for warning symbols
>>> > defined in real object files or other LTO IR files..
>>>
>>> Yes.
>>>
>>> >>       case WARN:
>>> >> +       /* Don't issue a warning for reference in LTO IR which may be
>>> >> +          removed by LTO later.  Make a warning symbol instead.  */
>>> >> +       if ((hash_entry_bfd (h)->flags & BFD_PLUGIN) != 0)
>>> >> +         goto mwarn;
>>> >> +
>>> >
>>> > And here you have the case where the new symbol is a warning symbol,
>>> > in either LTO IR or real object files, but the reference was in an LTO
>>> > IR file.
>>>
>>> Yes.
>>>
>>> > What about the CWARN case, where you already have a definition that
>>> > may have been referenced, and the new symbol is a warning symbol?
>>> > You'll need to test h->non_ir_ref I think.
>>>
>>> It is:
>>>
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=12760
>>
>> That's a different issue, isn't it?  I'm talking about the sequence
>> 1a) symbol referenced,
>> 2a) symbol defined,
>> 3a) warning symbol defined.
>> or
>> 1b) symbol defined,
>> 2b) symbol referenced,
>> 3b) warning symbol defined.
>>
>> If (1a) or (2b) is an IR reference then we don't want to warn.
>
> The symbol is referenced and used even if it has been inlined.
> Testing h->non_ir_ref isn't sufficient.  We also want to know
> if it is used in LTO.
>

With the fix for PR 16756:

https://sourceware.org/ml/binutils/2014-03/msg00275.html

we won't issue warnings for (1a) or (2b) if they are either
inlined or removed.

-- 
H.J.

Merge WARN and CWARN cases in _bfd_generic_link_add_one_symbol

[Alan Modra]

WARN has always been able to be handled by the CWARN case (WARN was
used when the previous symbols was undef, undefweak or common and thus
must be on the undefs list, so the CWARN test passes).  So let's merge
those two cases.

	* linker.c (WARN, CWARN): Collapse these states to WARN.
	(_bfd_generic_link_add_one_symbol): Use old CWARN case for
	new WARN.

diff --git a/bfd/linker.c b/bfd/linker.c
index 1877e67..93c5465 100644
--- a/bfd/linker.c
+++ b/bfd/linker.c
@@ -1342,8 +1342,7 @@ enum link_action
   CIND,		/* Make indirect symbol from existing common symbol.  */
   SET,		/* Add value to set.  */
   MWARN,	/* Make warning symbol.  */
-  WARN,		/* Issue warning.  */
-  CWARN,	/* Warn if referenced, else MWARN.  */
+  WARN,		/* Warn if referenced, else MWARN.  */
   CYCLE,	/* Repeat with symbol pointed to.  */
   REFC,		/* Mark indirect symbol referenced and then CYCLE.  */
   WARNC		/* Issue warning and then CYCLE.  */
@@ -1361,7 +1360,7 @@ static const enum link_action link_action[8][8] =
   /* DEFW_ROW 	*/  {DEFW,  DEFW,  DEFW,  NOACT, NOACT, NOACT, NOACT, CYCLE },
   /* COMMON_ROW	*/  {COM,   COM,   COM,   CREF,  COM,   BIG,   REFC,  WARNC },
   /* INDR_ROW	*/  {IND,   IND,   IND,   MDEF,  IND,   CIND,  MIND,  CYCLE },
-  /* WARN_ROW   */  {MWARN, WARN,  WARN,  CWARN, CWARN, WARN,  CWARN, NOACT },
+  /* WARN_ROW   */  {MWARN, WARN,  WARN,  WARN,  WARN,  WARN,  WARN,  NOACT },
   /* SET_ROW	*/  {SET,   SET,   SET,   SET,   SET,   SET,   CYCLE, CYCLE }
 };
 
@@ -1803,13 +1802,6 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info,
 	  break;
 
 	case WARN:
-	  /* Issue a warning.  */
-	  if (! (*info->callbacks->warning) (info, string, h->root.string,
-					     hash_entry_bfd (h), NULL, 0))
-	    return FALSE;
-	  break;
-
-	case CWARN:
 	  /* Warn if this symbol has been referenced already,
 	     otherwise add a warning.  A symbol has been referenced if
 	     the u.undef.next field is not NULL, or it is the tail of the

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] PR ld/16746: Don't issue a warning for reference in LTO IR

[Alan Modra]

The thread https://sourceware.org/ml/binutils/2014-03/msg00261.html 
died without any resolution, I guess a case of me wanting perfection
and rejecting the good.  So now that I've merged the WARN and CWARN
my concern with HJ's patch is covered.  This is what I'm about to
commit on HJ's behalf.

bfd/
	PR ld/16746
	* linker.c (_bfd_generic_link_add_one_symbol): Don't issue a
	warning for references in LTO IR to warning symbols.

ld/testsuite/
	PR ld/16746
	* ld-plugin/lto.exp: Add 4 tests for PR ld/16746.
	* ld-plugin/pr16746a.c: New file.
	* ld-plugin/pr16746b.c: Likewise.
	* ld-plugin/pr16746c.c: Likewise.
	* ld-plugin/pr16746d.c: Likewise.

diff --git a/bfd/linker.c b/bfd/linker.c
index 93c5465..d4b053c 100644
--- a/bfd/linker.c
+++ b/bfd/linker.c
@@ -1776,8 +1776,10 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info,
 	  break;
 
 	case WARNC:
-	  /* Issue a warning and cycle.  */
-	  if (h->u.i.warning != NULL)
+	  /* Issue a warning and cycle, except when the reference is
+	     in LTO IR.  */
+	  if (h->u.i.warning != NULL
+	      && (abfd->flags & BFD_PLUGIN) == 0)
 	    {
 	      if (! (*info->callbacks->warning) (info, h->u.i.warning,
 						 h->root.string, abfd,
@@ -1802,12 +1804,9 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info,
 	  break;
 
 	case WARN:
-	  /* Warn if this symbol has been referenced already,
-	     otherwise add a warning.  A symbol has been referenced if
-	     the u.undef.next field is not NULL, or it is the tail of the
-	     undefined symbol list.  The REF case above helps to
-	     ensure this.  */
-	  if (h->u.undef.next != NULL || info->hash->undefs_tail == h)
+	  /* Warn if this symbol has been referenced already from non-IR,
+	     otherwise add a warning.  */
+	  if (h->non_ir_ref)
 	    {
 	      if (! (*info->callbacks->warning) (info, string, h->root.string,
 						 hash_entry_bfd (h), NULL, 0))
diff --git a/ld/testsuite/ld-plugin/lto.exp b/ld/testsuite/ld-plugin/lto.exp
index 432c753..7ff5bfb 100644
--- a/ld/testsuite/ld-plugin/lto.exp
+++ b/ld/testsuite/ld-plugin/lto.exp
@@ -232,6 +232,18 @@ set lto_link_elf_tests [list \
   [list "Build libpr15146d.a" \
    "$plug_opt" "-flto -O2" \
    {pr15146d.c} {} "lib15146d.a"] \
+  [list "Build libpr16746a.a" \
+   "" "" \
+   {pr16746a.c pr16746b.c} {} "lib15146a.a"] \
+  [list "Build libpr16746b.a" \
+   "$plug_opt" "-O2 -flto" \
+   {pr16746c.c pr16746d.c} {} "lib15146b.a"] \
+  [list "PR ld/16746 (1)" \
+   "-O2 -flto -fuse-linker-plugin tmpdir/pr16746a.o tmpdir/pr16746c.o" "-O2 -flto" \
+   {dummy.c} {} "pr16746a.exe"] \
+  [list "PR ld/16746 (2)" \
+   "-O2 -flto -fuse-linker-plugin tmpdir/pr16746c.o tmpdir/pr16746a.o" "-O2 -flto" \
+   {dummy.c} {} "pr16746b.exe"] \
 ]
 
 # Check final symbols in executables.
@@ -335,6 +347,20 @@ if { [is_elf_format] && [check_lto_shared_available] } {
     } {
 	fail $testname
     }
+    set testname "PR ld/16746 (3)"
+    set exec_output [run_host_cmd "$CC" "-O2 -flto -fuse-linker-plugin tmpdir/pr16746b.o tmpdir/pr16746d.o"]
+    if { [ regexp "warning: foobar" $exec_output ] && ![ regexp "symbol from plugin" $exec_output ] } {
+	pass $testname
+    } {
+	fail $testname
+    }
+    set testname "PR ld/16746 (4)"
+    set exec_output [run_host_cmd "$CC" "-O2 -flto -fuse-linker-plugin tmpdir/pr16746d.o tmpdir/pr16746b.o"]
+    if { [ regexp "warning: foobar" $exec_output ] && ![ regexp "symbol from plugin" $exec_output ] } {
+	pass $testname
+    } {
+	fail $testname
+    }
 }
 
 set testname "Build liblto-11.a"
diff --git a/ld/testsuite/ld-plugin/pr16746a.c b/ld/testsuite/ld-plugin/pr16746a.c
new file mode 100644
index 0000000..1705ef1
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr16746a.c
@@ -0,0 +1,3 @@
+static const char __evoke_link_warning_foobar[]
+ __attribute__ ((used, section (".gnu.warning.foobar\n\t#")))
+ = "foobar";
diff --git a/ld/testsuite/ld-plugin/pr16746b.c b/ld/testsuite/ld-plugin/pr16746b.c
new file mode 100644
index 0000000..c3b7a78
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr16746b.c
@@ -0,0 +1,5 @@
+static const char __evoke_link_warning_foobar[]
+ __attribute__ ((used, section (".gnu.warning.foobar\n\t#")))
+ = "foobar";
+
+void foobar (void) {}
diff --git a/ld/testsuite/ld-plugin/pr16746c.c b/ld/testsuite/ld-plugin/pr16746c.c
new file mode 100644
index 0000000..fb68ab7
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr16746c.c
@@ -0,0 +1,8 @@
+extern void foobar (void);
+int
+main (int argc, char **argv)
+{
+  if (__builtin_constant_p (argc))
+    foobar ();
+  return 0;
+}
diff --git a/ld/testsuite/ld-plugin/pr16746d.c b/ld/testsuite/ld-plugin/pr16746d.c
new file mode 100644
index 0000000..1fee4dd
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr16746d.c
@@ -0,0 +1,8 @@
+extern void foobar (void);
+int
+main (int argc, char **argv)
+{
+  if (!__builtin_constant_p (argc))
+    foobar ();
+  return 0;
+}


-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] PR ld/16746: Don't issue a warning for reference in LTO IR

[Hans-Peter Nilsson]

> From: Alan Modra <amodra@gmail.com>
> Date: Thu, 7 Aug 2014 02:54:51 +0200

> So now that I've merged the WARN and CWARN
> my concern with HJ's patch is covered.  This is what I'm about to
> commit on HJ's behalf.
> 
> bfd/
> 	PR ld/16746
> 	* linker.c (_bfd_generic_link_add_one_symbol): Don't issue a
> 	warning for references in LTO IR to warning symbols.
> 
> ld/testsuite/
> 	PR ld/16746
> 	* ld-plugin/lto.exp: Add 4 tests for PR ld/16746.
> 	* ld-plugin/pr16746a.c: New file.
> 	* ld-plugin/pr16746b.c: Likewise.
> 	* ld-plugin/pr16746c.c: Likewise.
> 	* ld-plugin/pr16746d.c: Likewise.

This, commit db7129469b, regressed ld-cris/warn3 for cris-elf:
Running /home/hp/binutils/src/ld/testsuite/ld-cris/cris.exp ...
FAIL: ld-cris/warn3

(The test-case checks stabs warning support for cris a.out; the
expected warning is no longer emitted after that commit.  If you
want to remove stabs-aout-warning support altogether I don't
really mind, but I assume you agree that should be an explicit
change only done after verifying that there is no other issue
affecting warnings support.  Stabs warnings for ELF was broken
since long, see ld-cris/warn4.d)

brgds, H-P

Re: [PATCH] PR ld/16746: Don't issue a warning for reference in LTO IR

[Alan Modra]

On Tue, Aug 12, 2014 at 12:21:47AM +0200, Hans-Peter Nilsson wrote:
> This, commit db7129469b, regressed ld-cris/warn3 for cris-elf:
> Running /home/hp/binutils/src/ld/testsuite/ld-cris/cris.exp ...
> FAIL: ld-cris/warn3

That was silly of me.  non_ir_ref is only valid when plugins are
active.

	PR ld/16746
include/
	* bfdlink.h (struct bfd_link_info): Add lto_plugin_active.
bfd/
	* linker.c (_bfd_generic_link_add_one_symbol <WARN>): Handle
	!lto_plugin_active.
ld/
	* plugin.c (plugin_load_plugins): Set link_info.lto_plugin_active.

diff --git a/bfd/linker.c b/bfd/linker.c
index d4b053c..1a5ecef 100644
--- a/bfd/linker.c
+++ b/bfd/linker.c
@@ -1806,7 +1806,9 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info,
 	case WARN:
 	  /* Warn if this symbol has been referenced already from non-IR,
 	     otherwise add a warning.  */
-	  if (h->non_ir_ref)
+	  if ((!info->lto_plugin_active
+	       && (h->u.undef.next != NULL || info->hash->undefs_tail == h))
+	      || h->non_ir_ref)
 	    {
 	      if (! (*info->callbacks->warning) (info, string, h->root.string,
 						 hash_entry_bfd (h), NULL, 0))
diff --git a/include/bfdlink.h b/include/bfdlink.h
index fc14a9f..58dba2a 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -292,6 +292,9 @@ struct bfd_link_info
      callback.  */
   unsigned int notice_all: 1;
 
+  /* TRUE if the LTO plugin is active.  */
+  unsigned int lto_plugin_active: 1;
+
   /* TRUE if we are loading LTO outputs.  */
   unsigned int loading_lto_outputs: 1;
 
diff --git a/ld/plugin.c b/ld/plugin.c
index f10c273..8d6ae05 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -841,6 +841,7 @@ plugin_load_plugins (void)
   plugin_callbacks = *orig_callbacks;
   plugin_callbacks.notice = &plugin_notice;
   link_info.notice_all = TRUE;
+  link_info.lto_plugin_active = TRUE;
   link_info.callbacks = &plugin_callbacks;
 }
 

-- 
Alan Modra
Australia Development Lab, IBM