Bug 31537 - duplicate weakref emitted with IMA
Summary: duplicate weakref emitted with IMA
Status: RESOLVED WONTFIX
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.3.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: assemble-failure, build, patch
Depends on:
Blocks: 31546 4.4pending 37515
  Show dependency treegraph
 
Reported: 2007-04-11 15:50 UTC by Bernhard Reutner-Fischer
Modified: 2009-06-04 13:41 UTC (History)
4 users (show)

See Also:
Host: i686-linux-gnu
Target: i686-linux-gnu
Build: i686-linux-gnu
Known to work:
Known to fail: 4.2.0 4.3.0 4.4.0
Last reconfirmed: 2008-01-25 18:02:47


Attachments
inaccurate bypass (667 bytes, patch)
2007-04-13 14:32 UTC, Bernhard Reutner-Fischer
Details | Diff
patch in testing (510 bytes, patch)
2008-01-29 21:07 UTC, Bernhard Reutner-Fischer
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bernhard Reutner-Fischer 2007-04-11 15:50:00 UTC
$ /scratch/obj.i686/gcc-4.3/./gcc/xgcc -B/scratch/obj.i686/gcc-4.3/./gcc/ -B/opt/i686/gcc-4.3//i686-linux-gnu/bin/ -B/opt/i686/gcc-4.3//i686-linux-gnu/lib/ -c f?.i -combine -o /dev/null 
/tmp/ccqmLD9J.s: Assembler messages:
/tmp/ccqmLD9J.s:3: Error: symbol `__gthrw_pthread_once' is already defined


$ cat f1.i
static __gthrw_pthread_once __attribute__ ((__weakref__ ("pthread_once")));
$ cat f2.i
static __gthrw_pthread_once __attribute__ ((__weakref__ ("pthread_once")));


It fails to drop duplicate weakrefs:
$ /scratch/obj.i686/gcc-4.3/./gcc/xgcc -B/scratch/obj.i686/gcc-4.3/./gcc/ -B/opt/i686/gcc-4.3//i686-linux-gnu/bin/ -B/opt/i686/gcc-4.3//i686-linux-gnu/lib/ -S f?.i -combine -o -
        .file   "f1.i"
        .weakref        __gthrw_pthread_once,pthread_once
        .weakref        __gthrw_pthread_once,pthread_once
        .ident  "GCC: (GNU) 4.3.0 20070410 (experimental)"
        .section        .note.GNU-stack,"",@progbits
Comment 1 Bernhard Reutner-Fischer 2007-04-13 14:32:16 UTC
Created attachment 13363 [details]
inaccurate bypass

Not a patch.

By marking ultimate target's asm_written_flag and bailing if it was already set, one could bypass this, perhaps..
Comment 2 Geoff Keating 2007-05-02 21:23:18 UTC
Since these are both 'static', shouldn't they be named things like __gthrw_pthread_once.247 in the assembler?
Comment 3 Bernhard Reutner-Fischer 2007-05-02 21:43:07 UTC
Richi suggested it was a FE bug.
Comment 4 Geoff Keating 2007-05-03 20:14:34 UTC
The following testcase should be equivalent to the original one but not involve IMA:

void f1()
{ 
  static __gthrw_pthread_once __attribute__ ((__weakref__ ("pthread_once")));
}
void f2()
{ 
  static __gthrw_pthread_once __attribute__ ((__weakref__ ("pthread_once")));
}
Comment 5 Bernhard Reutner-Fischer 2007-06-21 16:44:34 UTC
Without combine, the attribute is ignored:

$ gcc-4.3.orig-HEAD  -c pr.c  -o /dev/null
pr.c: In function 'f1':
pr.c:3: warning: '__weakref__' attribute ignored
pr.c: In function 'f2':
pr.c:7: warning: '__weakref__' attribute ignored


while with combine and the original testcase, current trunk still gives:
$ gcc-4.3.orig-HEAD  -c f1.i f2.i -combine -o /dev/null
/tmp/ccmg6Twk.s: Assembler messages:
/tmp/ccmg6Twk.s:3: Error: symbol `__gthrw_pthread_once' is already defined
Comment 6 Bernhard Reutner-Fischer 2008-01-02 19:06:25 UTC
Any update?
Current trunk still produces wrong code for weakrefs..
Comment 7 Bernhard Reutner-Fischer 2008-01-25 08:28:05 UTC
Testing a patch.
Comment 8 Bernhard Reutner-Fischer 2008-01-25 18:02:47 UTC
Mine.

$ cat pr31537.i
static int __gthrw_pthread_once __attribute__ ((__weakref__ ("pthread_once")));
$ gcc-4.3-HEAD -S -o - -Os pr31537.i pr31537.i pr31537.i \
pr31537.i pr31537.i pr31537.i -fno-tree-vnhoist -combine
        .file   "pr31537.i"
        .weakref        __gthrw_pthread_once,pthread_once
        .ident  "GCC: (GNU) 4.3.0 20080125 (experimental)"
        .section        .note.GNU-stack,"",@progbits

Regression-testing still in progress..
Comment 9 Bernhard Reutner-Fischer 2008-01-29 21:07:47 UTC
Created attachment 15053 [details]
patch in testing

This is a simple fix to adjust the respective vector (that get's filled/finalized far too early, AFAICT; furthermore it's never pruned and if a new target is added  the alias is never rejected).

This patch warns -- conditionally, should most likely be a warning(0.. -- if the target changes and adjust the vector accordingly.

Should a subsequent weakref to a new target invalidate the weak?
Comment 10 Bernhard Reutner-Fischer 2008-09-16 19:17:44 UTC
unassigning. The BE workaround bypasses it for me, no time to look further.
Comment 11 Steven Bosscher 2009-06-04 12:55:07 UTC
Oh, the temptation to close this as WONTFIX....  Objections?
Comment 12 Bernhard Reutner-Fischer 2009-06-04 13:24:43 UTC
Well, without it fixed it's impossible to build libgfortran (and other apps) with combine, which would be a very nice thing to have.
The sample patch above exposed no regressions fwiw.
Comment 13 Richard Biener 2009-06-04 13:41:55 UTC
We are getting LTO (and maybe LIPO), no need for -combine being fixed.