[PATCH] PR26983

[Steven Bosscher <stevenb dot gcc at gmail dot com>]

:ADDPATCH middle-end:

This is my proposed fix for PR26983, a P1 regression present in all GCC4
releases and on the trunk.  The test case is quite simple:

-------------------------------------
void* jmpbuf[6];

void foo()
{
    __builtin_setjmp (jmpbuf);
}
-------------------------------------

What happens is this:
1. expand_builtin_setjmp creates a setjmp setup and a setjmp receiver.
   Just before the receiver, it emits a label.  This label has no edges
   coming in to it, because setjmp is not represented in the CFG.
2. In the very first CFG cleanup at the RTL level, cleanup_cfg notices
   that the basic block with said label has no incoming edges, and so
   it decides that the block is not reachable and should be removed.
3. delete_insn queries can_delete_label_p to see if the label can be
   removed.  The label does not have a name (it is not a user label),
   it is not forced, and it is not marked LABEL_PRESERVE_P.  There _is_
   a REG_LABEL note referencing the label, so the LABEL_NUSES of the
   label is greater than 0.  However, can_delete_label_p does not test
   LABEL_NUSES (it can't know whether the refcount is up to date, and
   it almost never is unless a pass has just called rebuild_jump_labels).
   So can_delete_label_p says, "yes, you can delete this label", and
   delete_insn does so.

It turns out that in GCC3, there was another check in can_delete_label_p,
namely a lookup of the label in a list of all labels that are referenced
in non-jump RTL: label_value_list.  However, that list is gone in GCC4,
and even though I can't find the change in the ChangeLogs, it's likely
one of the things that became unnecessary when cfgexpand was merged.  In
any case, in GCC3 can_delete_label_p would find the label in that list
so that delete_insn would turn the label into a NOTE_INSN_DELETED_LABEL.

In GCC4, we don't have the label_value_list, so we believe the label can
be removed even though it is still referenced.

Now there are several ways to fix the bug:
1. Check LABEL_NUSES in can_delete_label_p.  This would be the obvious
   fix, if it weren't so that LABEL_NUSES is highly unreliable.  I have
   fixed at least half a dozen LABEL_NUSES-related bugs in the last two
   years and IMHO the entire idea of refcounting on labels ought to be
   abandoned in favor of something simpler (the LABEL_NUSES stuff looks
   like legacy to me from the days when GCC did not have a CFG...).
   So I decided not to trust LABEL_NUSES.
2. Force the label to be preserved.  This has another advantage over the
   LABEL_NUSES idea, in that it only affects the setjmp expander.  Small
   changes makes me feel more comfortable at this late stage in the 4.2
   release process, and this change is very small.
   But as the great dutch soccer philosopher once said: "Every advantage
   comes with a disadvantage": The patch only addresses the issue for
   setjmp, but there could be other cases where we can end up with an
   unreachable-but-still-referenced label.  I can't think of any, but if
   they do exist, then my patch won't help for those cases.
   It seems to me that this second option is still better because there
   are no bugzilla reports of similar problems for non-setjmp cases.
  

With this patch, can_delete_label_p will see the LABEL_PRESERVE_P flag
on the label before the setjmp receiver.  The result is that delete_insn
will change the label into a NOTE_INSN_DELETED_LABEL, just like in GCC3.

Bootstrapped c,c++,objc,fortran, and regression tested (unix/{,-m32}) on 
x86_64-suse-linux-gnu.
OK for the trunk?

FWIW I am not going to spend effort on backporting the patch if it is
approved for the trunk.

Gr.
Steven


gcc/
	* builtins.c (expand_builtin_setjmp): Force next_lab to be
	preserved.

testsuite/
	* gcc.dg/pr26983.c: New test.


Index: builtins.c
===================================================================
--- builtins.c	(revision 116785)
+++ builtins.c	(working copy)
@@ -760,6 +760,12 @@ expand_builtin_setjmp (tree arglist, rtx
 
   emit_label (next_lab);
 
+  /* Because setjmp and longjmp are not represented in the CFG, a cfgcleanup
+     may find that the basic block starting with NEXT_LAB is unreachable.
+     The whole block, along with NEXT_LAB, would be removed (see PR26983).
+     Make sure that never happens.  */
+  LABEL_PRESERVE_P (next_lab) = 1;
+     
   expand_builtin_setjmp_receiver (next_lab);
 
   /* Set TARGET to one.  */
Index: testsuite/gcc.dg/pr26983.c
===================================================================
--- testsuite/gcc.dg/pr26983.c	(revision 0)
+++ testsuite/gcc.dg/pr26983.c	(revision 0)
@@ -0,0 +1,19 @@
+/* { dg-do link } */
+
+/* This used to cause a linker failure because GCC would output
+   assembler code referencing labels that it had not output.  */
+
+void *jmpbuf[6];
+
+void
+foo (void)
+{
+  __builtin_setjmp (jmpbuf);
+}
+
+int
+main (void)
+{
+  return 0;
+}
+