Re: [PATCH] Prevent frame-related insn from occurring in delay slots of insns that throw

2012-01-29 Thread Richard Henderson 
On 01/29/2012 07:57 AM, Tom de Vries wrote:
 Richard,
 
 [now cc-ing gcc-patches]
 
 this patch fixes PR50283 in a target-independent way.
 
 it asserts on frame-related insns in the delay slot of insns that can throw,
 and prevents the assert by testing for the same condition in
 eligible_for_{delay,annul_true,annul_false}.
 
 build and reg-tested on mips64el.
 
 I don't know of any tests currently failing on this, so I think it's not
 appropriate for stage4. OK for stage1 ?
 

If we can't tell one unwinder (eh) what to do with this, how do we
expect to be able to tell another unwinder (gdb)?  Thus I'm not sure
how throwing has anything to do with it.

I certainly agree that we shouldn't do anything during stage4 unless
we can come up with a wrong-code test case.


r~

Re: [PATCH] Prevent frame-related insn from occurring in delay slots of insns that throw

2012-01-29 Thread Paul Brook 
 On 01/29/2012 07:57 AM, Tom de Vries wrote:
  Richard,
  
  [now cc-ing gcc-patches]
  
  this patch fixes PR50283 in a target-independent way.
  
  it asserts on frame-related insns in the delay slot of insns that can
  throw, and prevents the assert by testing for the same condition in
  eligible_for_{delay,annul_true,annul_false}.
  
  build and reg-tested on mips64el.
  
  I don't know of any tests currently failing on this, so I think it's not
  appropriate for stage4. OK for stage1 ?
 
 If we can't tell one unwinder (eh) what to do with this, how do we
 expect to be able to tell another unwinder (gdb)?  Thus I'm not sure
 how throwing has anything to do with it.

If you need a reliable backtrace from any point then I guess you shouldn't 
allow frame related insns in delay slots at all.

However this is a issue of debug experiance rather than code correctness.  My 
guess is most architectures don't allow you to singlestep into delay slots, so 
any use of delay slots may cause lossage[1], not just frame related ones.  
Debugging optimized code is always a compromise.

It's worth noting that -fasync-unwind-tables does not guarantee 
backtrace/unwind from arbitrary points.  Only from those instructions that may 
result in a synchronous trap (which matches the semantics of this patch).

I'd guess that putting frame related insns in delay slots of libcalls is 
probably a worthwhile optimization.

Paul

[1] If the insns come from different source lines.

Re: [PATCH] Prevent frame-related insn from occurring in delay slots of insns that throw

2012-01-29 Thread Richard Henderson 
On 01/30/2012 11:07 AM, Paul Brook wrote:
 However this is a issue of debug experiance rather than code correctness.  My 
 guess is most architectures don't allow you to singlestep into delay slots, 
 so 
 any use of delay slots may cause lossage[1], not just frame related ones.  

Certainly Sparc can single-step delay slots.  I assumed others do as well.


r~

Re: [PATCH] Prevent frame-related insn from occurring in delay slots of insns that throw

2012-01-29 Thread Richard Henderson 
On 01/30/2012 11:07 AM, Paul Brook wrote:
 It's worth noting that -fasync-unwind-tables does not guarantee 
 backtrace/unwind from arbitrary points.  Only from those instructions that 
 may 
 result in a synchronous trap (which matches the semantics of this patch).

... and we're not talking about arbitrary points.  We're talking about call 
sites.
Whether or not the call itself is marked nothrow.


r~

Re: [PATCH] Prevent frame-related insn from occurring in delay slots of insns that throw

2012-01-29 Thread Paul Brook 
 On 01/30/2012 11:07 AM, Paul Brook wrote:
  It's worth noting that -fasync-unwind-tables does not guarantee
  backtrace/unwind from arbitrary points.  Only from those instructions
  that may result in a synchronous trap (which matches the semantics of
  this patch).
 
 ... and we're not talking about arbitrary points.  We're talking about call
 sites. Whether or not the call itself is marked nothrow.

What about backtracing from a signal handler?

Paul

[PATCH] Prevent frame-related insn from occurring in delay slots of insns that throw

2012-01-28 Thread Tom de Vries 
Richard,

[now cc-ing gcc-patches]

this patch fixes PR50283 in a target-independent way.

it asserts on frame-related insns in the delay slot of insns that can throw,
and prevents the assert by testing for the same condition in
eligible_for_{delay,annul_true,annul_false}.

build and reg-tested on mips64el.

I don't know of any tests currently failing on this, so I think it's not
appropriate for stage4. OK for stage1 ?

Thanks,
- Tom

2012-01-27  Andrew Pinski  apin...@cavium.com
Tom de Vries  t...@codesourcery.com

* dwarf2cfi.c (scan_trace): Add assert that frame-related insns should
not occur in delay-slots of insns that can throw.
* genattrtab.c (write_eligible_delay): Prevent frame-related insns from
occurring in delay-slots of insns that can throw.

Index: gcc/genattrtab.c
===
--- gcc/genattrtab.c (revision 183557)
+++ gcc/genattrtab.c (working copy)
@@ -4280,6 +4280,12 @@ write_eligible_delay (const char *kind)
   printf (  if (!INSN_P (candidate_insn))\n);
   printf (return 0;\n);
   printf (\n);
+  /* Frame-related instructions should not be put into delay slots of
+ instructions that can throw.  */
+  printf (  if (insn_could_throw_p (delay_insn)\n);
+  printf (   RTX_FRAME_RELATED_P (candidate_insn))\n);
+  printf (return 0;\n);
+  printf (\n);
 
   /* If more than one delay type, find out which type the delay insn is.  */
 
Index: gcc/dwarf2cfi.c
===
--- gcc/dwarf2cfi.c (revision 183557)
+++ gcc/dwarf2cfi.c (working copy)
@@ -2474,6 +2474,8 @@ scan_trace (dw_trace_info *trace)
 	  for (i = 1; i  n; ++i)
 	{
 	  elt = XVECEXP (pat, 0, i);
+	  gcc_assert (!(insn_could_throw_p (control)
+			 RTX_FRAME_RELATED_P (elt)));
 	  scan_insn_after (elt);
 	}