Bug 42757 - lto1 does not emit common symbols with -fuse-linker-plugin
Summary: lto1 does not emit common symbols with -fuse-linker-plugin
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: lto (show other bugs)
Version: 4.5.0
: P3 normal
Target Milestone: ---
Assignee: Cary Coutant
URL:
Keywords: lto
: 43355 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-01-15 13:32 UTC by Dmitry Gorbachev
Modified: 2010-04-06 22:00 UTC (History)
7 users (show)

See Also:
Host: i686-pc-linux-gnu
Target: i686-pc-linux-gnu
Build: i686-pc-linux-gnu
Known to work:
Known to fail:
Last reconfirmed: 2010-03-31 23:30:31


Attachments
Testcase (215 bytes, text/plain)
2010-01-15 13:34 UTC, Dmitry Gorbachev
Details
Gold patch to fix common symbol handling (1.65 KB, patch)
2010-03-31 23:40 UTC, Cary Coutant
Details | Diff
Updated gold patch to fix common symbol handling (3.12 KB, patch)
2010-04-01 06:32 UTC, Cary Coutant
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Gorbachev 2010-01-15 13:32:15 UTC
GCC 4.5.0 20100114.
Comment 1 Dmitry Gorbachev 2010-01-15 13:34:06 UTC
Created attachment 19614 [details]
Testcase

It produces error: undefined reference to 'i'
Comment 2 Richard Biener 2010-01-15 13:37:53 UTC
Works when linking in comm.o main.o order.  I guess the linker
resolution handling is bogus.
Comment 3 Richard Biener 2010-01-15 14:34:50 UTC
I see

2
main.o 2
78 PREVAILING_DEF main
81 RESOLVED_IR i
comm.o 1
78 RESOLVED_IR i

which is obviously bogus for comm.o.
Comment 4 Richard Biener 2010-03-13 13:00:31 UTC
*** Bug 43355 has been marked as a duplicate of this bug. ***
Comment 5 Richard Biener 2010-03-13 13:01:48 UTC
Ian?  What's gold up to here?
Comment 6 espindola 2010-03-13 18:02:13 UTC
It works with the llvm plugin for gold. I also find the gold output funny. What the llvm plugin does is

 if (I->syms[i].resolution == LDPR_PREVAILING_DEF ||
            (I->syms[i].def == LDPK_COMMON &&
             I->syms[i].resolution == LDPR_RESOLVED_IR)) {

....

So it has a special case for common + resolved_ir.  Let me try to find when this was added.
Comment 7 espindola 2010-03-13 18:10:09 UTC
The llvm change is

http://llvm.org/viewvc/llvm-project?view=rev&revision=64616

so I still don't know why gold is not returning LDPR_PREVAILING_DEF for the common symbol definition.
Comment 8 Richard Biener 2010-03-26 14:40:55 UTC
Cary, can you give advice and/or fix gold and/or the linker plugin?
Comment 9 Ralf Wildenhues 2010-03-30 16:39:55 UTC
(In reply to comment #2)
> Works when linking in comm.o main.o order.

FWIW one can construct test cases where neither order works, e.g.,

cat >main.c <<\EOF
extern int i;
int j;

int main(void)
{
    i = 0;
    return i;
}
EOF

cat >comm.c <<\EOF
extern int j;
int i;
int f() { return j; }
EOF

(libtool likes to produce such code with preload symfiles)
Comment 10 Cary Coutant 2010-03-31 23:40:28 UTC
Created attachment 20267 [details]
Gold patch to fix common symbol handling

This gold patch ought to fix the problem. Please verify and I'll send it upstream to binutils.

        * plugin.cc (Pluginobj::get_symbol_resolution_info): Check for
        prevailing definitions of common symbols.
        * testsuite/plugin_test_6.sh: New testcase.
        * testsuite/plugin_common_test_1.c: New testcase.
        * testsuite/plugin_common_test_2.c: New testcase.
Comment 11 Cary Coutant 2010-04-01 06:32:21 UTC
Created attachment 20269 [details]
Updated gold patch to fix common symbol handling

Sorry, I forgot to include the updated testsuite/Makefile.* in the patch...

        * plugin.cc (Pluginobj::get_symbol_resolution_info): Check for
        prevailing definitions of common symbols.
        * testsuite/plugin_test_6.sh: New test case.
        * testsuite/plugin_common_test_1.c: New test case.
        * testsuite/plugin_common_test_2.c: New test case.
        * testsuite/Makefile.am (plugin_test_6): New test case.
        * testsuite/Makefile.in: Regenerate.
Comment 12 Ralf Wildenhues 2010-04-01 18:21:34 UTC
(In reply to comment #11)

This patch works for me.  Thanks for looking into this!
Comment 13 Cary Coutant 2010-04-06 22:00:46 UTC
gold patch committed