* PATCH: PR ld/12507: Can't build a program with -flto -nostdlib
@ 2011-02-24 22:59 H.J. Lu
2011-02-25 22:45 ` Dave Korn
2011-02-25 23:30 ` Alan Modra
0 siblings, 2 replies; 13+ messages in thread
From: H.J. Lu @ 2011-02-24 22:59 UTC (permalink / raw)
To: binutils
We should never mark entry symbol IR only. I checked in this patch as
an obvious fix.
H.J.
----
diff --git a/ld/ChangeLog b/ld/ChangeLog
index 7b3d9bd..c672209 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,8 @@
+2011-02-24 H.J. Lu <hongjiu.lu@intel.com>
+
+ PR ld/12507
+ * plugin.c (get_symbols): Don't mark entry symbol IR only.
+
2011-02-18 John David Anglin <dave.anglin@nrc-cnnrc.gc.ca>
PR ld/12376
diff --git a/ld/plugin.c b/ld/plugin.c
index 628db41..7892e36 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -490,8 +490,10 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
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)
+ && strcmp (syms[n].name, entry_symbol.name) != 0);
/* If it was originally undefined or common, then it has been
resolved; determine how. */
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: PATCH: PR ld/12507: Can't build a program with -flto -nostdlib
2011-02-24 22:59 PATCH: PR ld/12507: Can't build a program with -flto -nostdlib H.J. Lu
@ 2011-02-25 22:45 ` Dave Korn
2011-02-25 23:08 ` H.J. Lu
2011-02-25 23:30 ` Alan Modra
1 sibling, 1 reply; 13+ messages in thread
From: Dave Korn @ 2011-02-25 22:45 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
[-- Attachment #1: Type: text/plain, Size: 1316 bytes --]
On 24/02/2011 22:59, H.J. Lu wrote:
> We should never mark entry symbol IR only. I checked in this patch as
> an obvious fix.
> --- a/ld/plugin.c
> +++ b/ld/plugin.c
> @@ -490,8 +490,10 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
> 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)
> + && strcmp (syms[n].name, entry_symbol.name) != 0);
This caused a bunch of regressions for me:
> +FAIL: plugin claimfile resolve symbol
> +FAIL: plugin claimfile replace file
> +FAIL: plugin ignore lib
> +FAIL: plugin claimfile replace lib
It turns out that entry_symbol.name can be NULL, and strcmp doesn't have to
handle null pointers gracefully; it segfaulted on cygwin. We need to guard
the test, like the attached.
ld/ChangeLog:
2011-02-25 Dave Korn <....
* plugin.c (get_symbols): Guard against NULL name of entry_symbol.
OK?
cheers,
DaveK
[-- Attachment #2: pr12507-part2.diff --]
[-- Type: text/x-c, Size: 726 bytes --]
Index: ld/plugin.c
===================================================================
RCS file: /cvs/src/src/ld/plugin.c,v
retrieving revision 1.24
diff -p -u -r1.24 plugin.c
--- ld/plugin.c 24 Feb 2011 22:58:05 -0000 1.24
+++ ld/plugin.c 25 Feb 2011 22:41:09 -0000
@@ -496,7 +496,8 @@ get_symbols (const void *handle, int nsy
ironly = (!is_visible_from_outside (&syms[n], owner_sec, blhe)
&& !bfd_hash_lookup (non_ironly_hash, syms[n].name,
FALSE, FALSE)
- && strcmp (syms[n].name, entry_symbol.name) != 0);
+ && (entry_symbol.name == NULL
+ || strcmp (syms[n].name, entry_symbol.name) != 0));
/* If it was originally undefined or common, then it has been
resolved; determine how. */
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: PATCH: PR ld/12507: Can't build a program with -flto -nostdlib
2011-02-25 22:45 ` Dave Korn
@ 2011-02-25 23:08 ` H.J. Lu
0 siblings, 0 replies; 13+ messages in thread
From: H.J. Lu @ 2011-02-25 23:08 UTC (permalink / raw)
To: Dave Korn; +Cc: binutils
On Fri, Feb 25, 2011 at 2:45 PM, Dave Korn <dave.korn.cygwin@gmail.com> wrote:
> On 24/02/2011 22:59, H.J. Lu wrote:
>> We should never mark entry symbol IR only. I checked in this patch as
>> an obvious fix.
>
>> --- a/ld/plugin.c
>> +++ b/ld/plugin.c
>> @@ -490,8 +490,10 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
>> 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)
>> + && strcmp (syms[n].name, entry_symbol.name) != 0);
>
> This caused a bunch of regressions for me:
>
>> +FAIL: plugin claimfile resolve symbol
>> +FAIL: plugin claimfile replace file
>> +FAIL: plugin ignore lib
>> +FAIL: plugin claimfile replace lib
>
> It turns out that entry_symbol.name can be NULL, and strcmp doesn't have to
> handle null pointers gracefully; it segfaulted on cygwin. We need to guard
> the test, like the attached.
>
> ld/ChangeLog:
>
> 2011-02-25 Dave Korn <....
>
> * plugin.c (get_symbols): Guard against NULL name of entry_symbol.
>
> OK?
Sorry for that. I consider it obvious.
--
H.J.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: PATCH: PR ld/12507: Can't build a program with -flto -nostdlib
2011-02-24 22:59 PATCH: PR ld/12507: Can't build a program with -flto -nostdlib H.J. Lu
2011-02-25 22:45 ` Dave Korn
@ 2011-02-25 23:30 ` Alan Modra
2011-02-25 23:45 ` H.J. Lu
1 sibling, 1 reply; 13+ messages in thread
From: Alan Modra @ 2011-02-25 23:30 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
On Thu, Feb 24, 2011 at 02:59:13PM -0800, H.J. Lu wrote:
> @@ -490,8 +490,10 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
> 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)
> + && strcmp (syms[n].name, entry_symbol.name) != 0);
>
> /* If it was originally undefined or common, then it has been
> resolved; determine how. */
It would be better to put the entry symbol, and -u syms on the
entry_symbol chain, into non_ironly_hash.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: PATCH: PR ld/12507: Can't build a program with -flto -nostdlib
2011-02-25 23:30 ` Alan Modra
@ 2011-02-25 23:45 ` H.J. Lu
2011-02-25 23:59 ` Alan Modra
0 siblings, 1 reply; 13+ messages in thread
From: H.J. Lu @ 2011-02-25 23:45 UTC (permalink / raw)
To: H.J. Lu, binutils; +Cc: Alan Modra
On Fri, Feb 25, 2011 at 3:29 PM, Alan Modra <amodra@gmail.com> wrote:
> On Thu, Feb 24, 2011 at 02:59:13PM -0800, H.J. Lu wrote:
>> @@ -490,8 +490,10 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
>> 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)
>> + && strcmp (syms[n].name, entry_symbol.name) != 0);
>>
>> /* If it was originally undefined or common, then it has been
>> resolved; determine how. */
>
> It would be better to put the entry symbol, and -u syms on the
> entry_symbol chain, into non_ironly_hash.
>
I tried it and it doesn't work.
Also, we only care about the entry symbol for LTO.
--
H.J.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: PATCH: PR ld/12507: Can't build a program with -flto -nostdlib
2011-02-25 23:45 ` H.J. Lu
@ 2011-02-25 23:59 ` Alan Modra
2011-02-26 0:09 ` Rafael Ávila de Espíndola
2011-02-26 4:33 ` H.J. Lu
0 siblings, 2 replies; 13+ messages in thread
From: Alan Modra @ 2011-02-25 23:59 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
On Fri, Feb 25, 2011 at 03:45:14PM -0800, H.J. Lu wrote:
> On Fri, Feb 25, 2011 at 3:29 PM, Alan Modra <amodra@gmail.com> wrote:
> > It would be better to put the entry symbol, and -u syms on the
> > entry_symbol chain, into non_ironly_hash.
>
> I tried it and it doesn't work.
How so?
> Also, we only care about the entry symbol for LTO.
I'd be a little surprised if there is a good reason to treat -u syms
any differently from references in non-ir object files.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: PATCH: PR ld/12507: Can't build a program with -flto -nostdlib
2011-02-25 23:59 ` Alan Modra
@ 2011-02-26 0:09 ` Rafael Ávila de Espíndola
2011-02-26 4:34 ` H.J. Lu
2011-02-26 4:33 ` H.J. Lu
1 sibling, 1 reply; 13+ messages in thread
From: Rafael Ávila de Espíndola @ 2011-02-26 0:09 UTC (permalink / raw)
To: binutils
> I'd be a little surprised if there is a good reason to treat -u syms
> any differently from references in non-ir object files.
That is what I would expect, in fact, I added that to gold because I was
surprised that symbols passed to -u were marked as ir only.
Cheers,
Rafael
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: PATCH: PR ld/12507: Can't build a program with -flto -nostdlib
2011-02-25 23:59 ` Alan Modra
2011-02-26 0:09 ` Rafael Ávila de Espíndola
@ 2011-02-26 4:33 ` H.J. Lu
2011-02-26 4:36 ` H.J. Lu
2011-02-26 9:04 ` Alan Modra
1 sibling, 2 replies; 13+ messages in thread
From: H.J. Lu @ 2011-02-26 4:33 UTC (permalink / raw)
To: Binutils; +Cc: Alan Modra
On Fri, Feb 25, 2011 at 3:58 PM, Alan Modra <amodra@gmail.com> wrote:
> On Fri, Feb 25, 2011 at 03:45:14PM -0800, H.J. Lu wrote:
>> On Fri, Feb 25, 2011 at 3:29 PM, Alan Modra <amodra@gmail.com> wrote:
>> > It would be better to put the entry symbol, and -u syms on the
>> > entry_symbol chain, into non_ironly_hash.
>>
>> I tried it and it doesn't work.
>
> How so?
>
>> Also, we only care about the entry symbol for LTO.
>
> I'd be a little surprised if there is a good reason to treat -u syms
> any differently from references in non-ir object files.
>
This patch works.
--
H.J.
---
diff --git a/ld/plugin.c b/ld/plugin.c
index 7892e36..40acbdb 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -492,8 +492,7 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin
_symbol *syms)
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)
- && strcmp (syms[n].name, entry_symbol.name) != 0);
+ FALSE, FALSE));
/* If it was originally undefined or common, then it has been
resolved; determine how. */
@@ -838,6 +837,8 @@ plugin_call_cleanup (void)
static void
init_non_ironly_hash (void)
{
+ struct bfd_sym_chain *sym;
+
if (non_ironly_hash == NULL)
{
non_ironly_hash =
@@ -847,6 +848,12 @@ init_non_ironly_hash (void)
sizeof (struct bfd_hash_entry),
61))
einfo (_("%P%F: bfd_hash_table_init failed: %E\n"));
+
+ for (sym = &entry_symbol; sym != NULL; sym = sym->next)
+ if (sym->name
+ && !bfd_hash_lookup (non_ironly_hash, sym->name, TRUE, TRUE))
+ einfo (_("%P%X: hash table failure adding symbol %s\n"),
+ sym->name);
}
}
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: PATCH: PR ld/12507: Can't build a program with -flto -nostdlib
2011-02-26 0:09 ` Rafael Ávila de Espíndola
@ 2011-02-26 4:34 ` H.J. Lu
2011-02-26 5:16 ` Rafael Ávila de Espíndola
0 siblings, 1 reply; 13+ messages in thread
From: H.J. Lu @ 2011-02-26 4:34 UTC (permalink / raw)
To: Rafael Ávila de Espíndola; +Cc: binutils
2011/2/25 Rafael Ávila de Espíndola <respindola@mozilla.com>:
>> I'd be a little surprised if there is a good reason to treat -u syms
>> any differently from references in non-ir object files.
>
> That is what I would expect, in fact, I added that to gold because I was
> surprised that symbols passed to -u were marked as ir only.
>
Gold failed -u foo.
--
H.J.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: PATCH: PR ld/12507: Can't build a program with -flto -nostdlib
2011-02-26 4:33 ` H.J. Lu
@ 2011-02-26 4:36 ` H.J. Lu
2011-02-26 9:04 ` Alan Modra
1 sibling, 0 replies; 13+ messages in thread
From: H.J. Lu @ 2011-02-26 4:36 UTC (permalink / raw)
To: Binutils; +Cc: Alan Modra
On Fri, Feb 25, 2011 at 8:33 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Feb 25, 2011 at 3:58 PM, Alan Modra <amodra@gmail.com> wrote:
>> On Fri, Feb 25, 2011 at 03:45:14PM -0800, H.J. Lu wrote:
>>> On Fri, Feb 25, 2011 at 3:29 PM, Alan Modra <amodra@gmail.com> wrote:
>>> > It would be better to put the entry symbol, and -u syms on the
>>> > entry_symbol chain, into non_ironly_hash.
>>>
>>> I tried it and it doesn't work.
>>
>> How so?
>>
>>> Also, we only care about the entry symbol for LTO.
>>
>> I'd be a little surprised if there is a good reason to treat -u syms
>> any differently from references in non-ir object files.
>>
>
> This patch works.
>
> --
> H.J.
> ---
> diff --git a/ld/plugin.c b/ld/plugin.c
> index 7892e36..40acbdb 100644
> --- a/ld/plugin.c
> +++ b/ld/plugin.c
> @@ -492,8 +492,7 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin
> _symbol *syms)
> 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)
> - && strcmp (syms[n].name, entry_symbol.name) != 0);
> + FALSE, FALSE));
>
> /* If it was originally undefined or common, then it has been
> resolved; determine how. */
> @@ -838,6 +837,8 @@ plugin_call_cleanup (void)
> static void
> init_non_ironly_hash (void)
> {
> + struct bfd_sym_chain *sym;
> +
> if (non_ironly_hash == NULL)
> {
> non_ironly_hash =
> @@ -847,6 +848,12 @@ init_non_ironly_hash (void)
> sizeof (struct bfd_hash_entry),
> 61))
> einfo (_("%P%F: bfd_hash_table_init failed: %E\n"));
> +
> + for (sym = &entry_symbol; sym != NULL; sym = sym->next)
> + if (sym->name
> + && !bfd_hash_lookup (non_ironly_hash, sym->name, TRUE, TRUE))
> + einfo (_("%P%X: hash table failure adding symbol %s\n"),
> + sym->name);
> }
> }
>
I am checking it in.
--
H.J.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: PATCH: PR ld/12507: Can't build a program with -flto -nostdlib
2011-02-26 4:34 ` H.J. Lu
@ 2011-02-26 5:16 ` Rafael Ávila de Espíndola
2011-02-26 5:32 ` H.J. Lu
0 siblings, 1 reply; 13+ messages in thread
From: Rafael Ávila de Espíndola @ 2011-02-26 5:16 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
>
> Gold failed -u foo.
>
>
I should have been fixed by
2010-02-08 Rafael Ãvila de EspÃndola <respindola@mozilla.com>
* plugin.cc (is_visible_from_outside): Return true for symbols
in the -u option.
In what case is it failing for you?
Cheers,
Rafael
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: PATCH: PR ld/12507: Can't build a program with -flto -nostdlib
2011-02-26 5:16 ` Rafael Ávila de Espíndola
@ 2011-02-26 5:32 ` H.J. Lu
0 siblings, 0 replies; 13+ messages in thread
From: H.J. Lu @ 2011-02-26 5:32 UTC (permalink / raw)
To: Rafael Ávila de Espíndola; +Cc: binutils
2011/2/25 Rafael Ávila de Espíndola <respindola@mozilla.com>:
>>
>> Gold failed -u foo.
>>
>>
>
> I should have been fixed by
>
> 2010-02-08 Rafael Ávila de Espíndola <respindola@mozilla.com>
>
> * plugin.cc (is_visible_from_outside): Return true for symbols
> in the -u option.
>
> In what case is it failing for you?
Sorry. Gold is OK.
--
H.J.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: PATCH: PR ld/12507: Can't build a program with -flto -nostdlib
2011-02-26 4:33 ` H.J. Lu
2011-02-26 4:36 ` H.J. Lu
@ 2011-02-26 9:04 ` Alan Modra
1 sibling, 0 replies; 13+ messages in thread
From: Alan Modra @ 2011-02-26 9:04 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
On Fri, Feb 25, 2011 at 08:33:09PM -0800, H.J. Lu wrote:
> This patch works.
Looks good to me.
> diff --git a/ld/plugin.c b/ld/plugin.c
> index 7892e36..40acbdb 100644
> --- a/ld/plugin.c
> +++ b/ld/plugin.c
> @@ -492,8 +492,7 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin
> _symbol *syms)
> 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)
> - && strcmp (syms[n].name, entry_symbol.name) != 0);
> + FALSE, FALSE));
>
> /* If it was originally undefined or common, then it has been
> resolved; determine how. */
> @@ -838,6 +837,8 @@ plugin_call_cleanup (void)
> static void
> init_non_ironly_hash (void)
> {
> + struct bfd_sym_chain *sym;
> +
> if (non_ironly_hash == NULL)
> {
> non_ironly_hash =
> @@ -847,6 +848,12 @@ init_non_ironly_hash (void)
> sizeof (struct bfd_hash_entry),
> 61))
> einfo (_("%P%F: bfd_hash_table_init failed: %E\n"));
> +
> + for (sym = &entry_symbol; sym != NULL; sym = sym->next)
> + if (sym->name
> + && !bfd_hash_lookup (non_ironly_hash, sym->name, TRUE, TRUE))
> + einfo (_("%P%X: hash table failure adding symbol %s\n"),
> + sym->name);
> }
> }
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-02-26 9:04 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-24 22:59 PATCH: PR ld/12507: Can't build a program with -flto -nostdlib H.J. Lu
2011-02-25 22:45 ` Dave Korn
2011-02-25 23:08 ` H.J. Lu
2011-02-25 23:30 ` Alan Modra
2011-02-25 23:45 ` H.J. Lu
2011-02-25 23:59 ` Alan Modra
2011-02-26 0:09 ` Rafael Ávila de Espíndola
2011-02-26 4:34 ` H.J. Lu
2011-02-26 5:16 ` Rafael Ávila de Espíndola
2011-02-26 5:32 ` H.J. Lu
2011-02-26 4:33 ` H.J. Lu
2011-02-26 4:36 ` H.J. Lu
2011-02-26 9:04 ` Alan Modra
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).