Re: [PATCH] PR32219, weak hidden reference segfault [PING^2]

[Chung-Lin Tang]

On 13/7/15 1:43 AM, Diego Novillo wrote:
> Could you please repost the patch with its description?  This thread
> is sufficiently old and noisy that I'm not even sure what the patch
> does nor why.

Taking the same example in my first post:

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 ...


Under -fPIC, the code in rtlanal.c:nonzero_address_p() does not properly
recognize the "PIC-reg + <constant>" form of load as a weak symbol; it
returns 'true' immediately after seeing the pic-reg indexing, and does
not test the wrapped symbol for DECL_WEAK.

My patch slightly modifies the test to look into the wrapped symbol when
seeing a "PIC-reg + <unspec-constant>" form, which I believe has become
the idiomatic form in GCC for such PIC addresses. Richard Sandiford's
concerns were that, UNSPEC really is unspecified, and this might be
overassuming its semantics, plus some targets may not be really
following the idiomatic use.

My final take at the time was, for targets that do follow the common
PIC-reg+const-unspec form, this patch solves the problem, while for
other targets, nothing is changed.

I have re-tested the patch on i686-linux, with clean results. Please see
if this patch can be accepted into trunk (patch attached again for
convenience).

Thanks,
Chung-Lin

2013-08-04  Chung-Lin Tang  <clt...@codesourcery.com>

        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 201473)
+++ rtlanal.c   (working copy)
@@ -393,7 +393,15 @@ nonzero_address_p (const_rtx x)
       /* Handle PIC references.  */
       if (XEXP (x, 0) == pic_offset_table_rtx
               && CONSTANT_P (XEXP (x, 1)))
-       return true;
+       {
+         rtx offset = XEXP (x, 1);
+         if (GET_CODE (offset) == CONST
+             && GET_CODE (XEXP (offset, 0)) == UNSPEC
+             && GET_CODE (XVECEXP (XEXP (offset, 0), 0, 0)) == SYMBOL_REF)
+           return nonzero_address_p (XVECEXP (XEXP (offset, 0), 0, 0));
+
+         return true;
+       }
       return false;
 
     case PRE_MODIFY:
Index: testsuite/gcc.dg/torture/pr32219.c
===================================================================
--- testsuite/gcc.dg/torture/pr32219.c  (revision 0)
+++ testsuite/gcc.dg/torture/pr32219.c  (revision 0)
@@ -0,0 +1,12 @@
+/* PR target/32219 */
+/* { dg-do run } */
+/* { dg-require-visibility "" } */
+/* { dg-options "-fPIC" { target fpic } } */
+
+extern void f() __attribute__((weak,visibility("hidden")));
+int main()
+{
+  if (f)
+    f();
+  return 0;
+}