[RFA:] Fix PR37170: weak-1.c regression, second edition
Hans-Peter Nilsson
hans-peter.nilsson@axis.com
Thu Aug 28 16:50:00 GMT 2008
Main points:
- #ifndef ASM_OUTPUT_EXTERNAL was around the weak-handling too.
- Inlined functions and emitted variables can be marked as weak;
there must be no weak-*reference* annotation for them.
- output_operand doesn't catch all from code referenced
symbol_ref's.
The need for the special Darwin hack was revealed by one of the
test-cases, I forgot which one, perhaps weak-1.c. I hope all
this is temporarily, and that eventually the language front-ends
will stop calling assemble_external and that the expand pass
will handle all weak and external et al attributes.
The g++.dg/ext/inline1.C shows a case where an inlined function
is spuriously marked as weak (which with an earlier version of
this aptch broke on Darwin, whose assembler doesn't take kindly
to inconsistent weak annotations). The testcase fails for
e.g. x86_64-unknown-linux-gnu. The TREE_STATIC check fixes
this. The gcc.dg/weak/weak-15 testcase doesn't really reveal
any flaws, it just covers a few holes in the weak-testing: I
missed tests that referenced a weak symbol, but offset, and the
address of a weak variable, but from data, not code. Also in
there is kind-of an attempt to verify that the inlined-function-
marked-as-weak situation doesn't happen in C, though the code
calling assemble_external for that case was indeed specific to
C++.
I'd like to thank Dominique d'Humieres, Eric Weddington and
Andreas Tobler for testing various versions of this patch.
This version was bootstrapped and regtested on
x86_64-unknown-linux-gnu, i686-darwin9 and ppc-darwin9 and cross
to cris-elf.
Ok to commit?
gcc/testsuite:
PR middle-end/37170
* gcc.dg/weak/weak-15.c, g++.dg/ext/inline1.C: New tests.
gcc:
PR middle-end/37170
* final.c (mark_symbol_ref_as_used): New helper function.
(output_operand): Instead of just looking inside MEMs for
SYMBOL_REFs, use new helper function and for_each_rtx.
* varasm.c (assemble_external): Move #ifndef ASM_OUTPUT_EXTERNAL
to after weak-handling. Return early also for decls with
TREE_STATIC. Make head comment more general.
* config/darwin.c (machopic_output_indirection): Handle weak
references here, like in assemble_external.
--- /dev/null 2008-02-22 17:23:55.052000250 +0100
+++ g++.dg/ext/inline1.C 2008-08-24 06:21:50.000000000 +0200
@@ -0,0 +1,34 @@
+// { dg-do compile }
+// { dg-options "-O" }
+// Make sure inlined non-outlined functions aren't marked weak.
+// We'd get a ".weak xyzzy" annotation trigged by the second declaration.
+
+// { dg-final { scan-assembler-not "weak\[^ \t\]*\[ \t\]_?xyzzy" } }
+
+// The next check isn't really part of the actual test, just to make
+// sure there's no outline-copy of xyzzy, because if that really
+// happened, it *should* be marked linkonce or perhaps weak.
+// { dg-final { scan-assembler-not "xyzzy" } }
+
+extern int x;
+extern void foo(void);
+extern void bar(void);
+
+extern "C" inline int xyzzy(int a)
+{
+ foo();
+ return a + x;
+}
+
+extern "C" int xyzzy(int);
+
+extern inline int plugh(int c)
+{
+ return xyzzy (c);
+}
+
+int y;
+void doit(int b)
+{
+ y = xyzzy (b) + plugh (b);
+}
--- /dev/null 2008-02-22 17:23:55.052000250 +0100
+++ gcc.dg/weak/weak-15.c 2008-08-25 03:41:31.000000000 +0200
@@ -0,0 +1,39 @@
+/* { dg-do compile } */
+/* { dg-require-weak "" } */
+/* { dg-options "-fno-common" } */
+
+/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]_?a" } } */
+/* { dg-final { scan-assembler-not "weak\[^ \t\]*\[ \t\]_?b" } } */
+/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]_?c" } } */
+/* { dg-final { scan-assembler "weak\[^ \t\]*\[ \t\]_?d" } } */
+
+#pragma weak a
+extern char a[];
+
+char *user_a(void)
+{
+ return a+1;
+}
+
+int x;
+int extern inline b(int y)
+{
+ return x+y;
+}
+
+extern int b(int y);
+
+int user_b(int z)
+{
+ return b(z);
+}
+
+#pragma weak c
+extern int c;
+
+int *user_c = &c;
+
+#pragma weak d
+extern char d[];
+
+char *user_d = &d[1];
Index: final.c
===================================================================
--- final.c (revision 139624)
+++ final.c (working copy)
@@ -3346,6 +3346,31 @@ output_asm_label (rtx x)
assemble_name (asm_out_file, buf);
}
+/* Helper rtx-iteration-function for output_operand. Marks
+ SYMBOL_REFs as referenced through use of assemble_external. */
+
+static int
+mark_symbol_ref_as_used (rtx *xp, void *dummy ATTRIBUTE_UNUSED)
+{
+ rtx x = *xp;
+
+ /* If we have a used symbol, we may have to emit assembly
+ annotations corresponding to whether the symbol is external, weak
+ or has non-default visibility. */
+ if (GET_CODE (x) == SYMBOL_REF)
+ {
+ tree t;
+
+ t = SYMBOL_REF_DECL (x);
+ if (t)
+ assemble_external (t);
+
+ return -1;
+ }
+
+ return 0;
+}
+
/* Print operand X using machine-dependent assembler syntax.
The macro PRINT_OPERAND is defined just to control this function.
CODE is a non-digit that preceded the operand-number in the % spec,
@@ -3366,14 +3391,11 @@ output_operand (rtx x, int code ATTRIBUT
gcc_assert (!x || !REG_P (x) || REGNO (x) < FIRST_PSEUDO_REGISTER);
PRINT_OPERAND (asm_out_file, x, code);
- if (x && MEM_P (x) && GET_CODE (XEXP (x, 0)) == SYMBOL_REF)
- {
- tree t;
- x = XEXP (x, 0);
- t = SYMBOL_REF_DECL (x);
- if (t)
- assemble_external (t);
- }
+
+ if (x == NULL_RTX)
+ return;
+
+ for_each_rtx (&x, mark_symbol_ref_as_used, NULL);
}
/* Print a memory reference operand for address X
Index: varasm.c
===================================================================
--- varasm.c (revision 139624)
+++ varasm.c (working copy)
@@ -2290,9 +2290,10 @@ process_pending_assemble_externals (void
to be emitted. */
static GTY(()) tree weak_decls;
-/* Output something to declare an external symbol to the assembler.
- (Most assemblers don't need this, so we normally output nothing.)
- Do nothing if DECL is not external. */
+/* Output something to declare an external symbol to the assembler,
+ and qualifiers such as weakness. (Most assemblers don't need
+ extern declaration, so we normally output nothing.) Do nothing if
+ DECL is not external. */
void
assemble_external (tree decl ATTRIBUTE_UNUSED)
@@ -2303,15 +2304,20 @@ assemble_external (tree decl ATTRIBUTE_U
open. If it's not, we should not be calling this function. */
gcc_assert (asm_out_file);
-#ifdef ASM_OUTPUT_EXTERNAL
- if (!DECL_P (decl) || !DECL_EXTERNAL (decl) || !TREE_PUBLIC (decl))
+ if (!DECL_P (decl) || !DECL_EXTERNAL (decl) || !TREE_PUBLIC (decl)
+ /* Handle only actual external-only definitions, not e.g. extern
+ inline code or variables for which storage has been
+ allocated. */
+ || TREE_STATIC (decl))
return;
+ /* We want to output annotation for weak and external symbols at
+ very last to check if they are references or not. */
+
if (SUPPORTS_WEAK && DECL_WEAK (decl))
weak_decls = tree_cons (NULL, decl, weak_decls);
- /* We want to output external symbols at very last to check if they
- are references or not. */
+#ifdef ASM_OUTPUT_EXTERNAL
pending_assemble_externals = tree_cons (0, decl,
pending_assemble_externals);
#endif
Index: config/darwin.c
===================================================================
--- config/darwin.c (revision 139624)
+++ config/darwin.c (working copy)
@@ -1002,6 +1002,30 @@ machopic_output_indirection (void **slot
rtx init = const0_rtx;
switch_to_section (darwin_sections[machopic_nl_symbol_ptr_section]);
+
+ /* Mach-O symbols are passed around in code through indirect
+ references and the original symbol_ref hasn't passed through
+ the generic handling and reference-catching in
+ output_operand, so we need to manually mark weak references
+ as such. */
+ if (SYMBOL_REF_WEAK (symbol))
+ {
+ tree decl = SYMBOL_REF_DECL (symbol);
+ gcc_assert (DECL_P (decl));
+
+ if (decl != NULL_TREE
+ && DECL_EXTERNAL (decl) && TREE_PUBLIC (decl)
+ /* Handle only actual external-only definitions, not
+ e.g. extern inline code or variables for which
+ storage has been allocated. */
+ && !TREE_STATIC (decl))
+ {
+ fputs ("\t.weak_reference ", asm_out_file);
+ assemble_name (asm_out_file, sym_name);
+ fputc ('\n', asm_out_file);
+ }
+ }
+
assemble_name (asm_out_file, ptr_name);
fprintf (asm_out_file, ":\n");
brgds, H-P
More information about the Gcc-patches
mailing list