Hi, with reference to the old dicussion on PR 32219:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32219
It seems that a patch was submitted to put the DECL_WEAK check before
the visibility check, but that patch was never approved or applied, due
to concerns in the wording of surrounding comments:
http://gcc.gnu.org/ml/gcc-patches/2010-03/msg00666.html
That patch does solve the segfault in the PR, and happens to also solve
the other weak-hidden + section-anchor issue Nathan's other patch solves
(simply because it works the same way by rejecting DECL_WEAK inside
binds_local_p).
However, my own testing of the PR patch on recent trunk indicates a
regression: it filters out the TLS wrapper functions for C++11
thread_local variables, causing them to be called by @PLT, and failing a
few tests that check for this.
Looking into the generated code for:
extern void weakfun() __attribute__((weak,visibility("hidden")));
void foo ()
{
if (weakfun) weakfun();
}
Under -O1 -m32 -fPIC, the address load and test will look like:
(insn 5 2 6 2 (set (reg/f:SI 60)
(plus:SI (reg:SI 3 bx)
(const:SI (unspec:SI [
(symbol_ref/i:SI ("f") [flags 0x43] <function_decl f>)
] UNSPEC_GOTOFF)))) {*leasi}
(expr_list:REG_EQUAL (symbol_ref/i:SI ("f") <function_decl f>)
(nil)))
(insn 6 5 7 2 (set (reg:CCZ 17 flags)
(compare:CCZ (reg/f:SI 60)
(const_int 0 [0]))) p.c:8 3 {*cmpsi_ccno_1}
(nil))
(jump_insn 7 6 8 2 ...
However, the logic currently used in rtlanal.c:nonzero_address_p() only
test for "PIC-reg + <constant_p>", instead of more sophisticated
checking to expose the wrapped weak symbol_ref, thus confusing CSE to
eliminate the needed test.
The DECL_WEAK test upward movement in binds_local_p() works because when
bound non-local, x86 expand turns the address load into (mem (plus
(const (unspec ... UNSPEC_GOT)))) form, with the MEM helping to avoid
the above case.
Attached is a patch to make the nonzero_address_p() work for this case,
bootstrapped and tested on i686-linux without regressions.
I have the impression from the PR32219 discussion, that the solution
should be to make all weak-hidden-undefined symbols as potentially
non-local. However, I don't think that is needed, no? As long as the
linker added zero value is in the same module, weak hidden semantics
should remain just the same...
Thanks,
Chung-Lin
2013-05-09 Chung-Lin Tang <[email protected]>
PR target/32219
* rtlanal.c (nonzero_address_p): Robustify checking by look
recursively into PIC constant offsets and (CONST (UNSPEC ...))
expressions.
Index: rtlanal.c
===================================================================
--- rtlanal.c (revision 198735)
+++ rtlanal.c (working copy)
@@ -387,13 +387,22 @@ nonzero_address_p (const_rtx x)
return false;
case CONST:
- return nonzero_address_p (XEXP (x, 0));
+ {
+ rtx base, offset;
+ /* Peel away any constant offsets from the base symbol. */
+ split_const (CONST_CAST_RTX (x), &base, &offset);
+ return nonzero_address_p (base);
+ }
+ case UNSPEC:
+ /* Reach for a contained symbol. */
+ return nonzero_address_p (XVECEXP (x, 0, 0));
+
case PLUS:
/* Handle PIC references. */
if (XEXP (x, 0) == pic_offset_table_rtx
&& CONSTANT_P (XEXP (x, 1)))
- return true;
+ return nonzero_address_p (XEXP (x, 1));
return false;
case PRE_MODIFY:
Index: testsuite/gcc.dg/visibility-21.c
===================================================================
--- testsuite/gcc.dg/visibility-21.c (revision 0)
+++ testsuite/gcc.dg/visibility-21.c (revision 0)
@@ -0,0 +1,12 @@
+/* PR target/32219 */
+/* { dg-do run } */
+/* { dg-require-visibility "" } */
+/* { dg-options "-O1 -fPIC" { target fpic } } */
+
+extern void f() __attribute__((weak,visibility("hidden")));
+int main()
+{
+ if (f)
+ f();
+ return 0;
+}
