BUG? -fomit-frame-pointer and exceptions or setjmp/longjmp

BUG? -fomit-frame-pointer and exceptions or setjmp/longjmp

[Denis Zaitsev]

If gcc is built with the -fomit-frame-pointer in the environment's
CXXFLAGS, then we have a dead exception handling mechanics - a c++
program, compiled with such a gcc, can't catch any of an exceptions it
throws.  All the exceptions leak thru the catch (...) chain and
program finishes by kill() itself.  It's because something becomes
wrong with a thrown object's type recognition.  And the reason is the
absence of the ebp register set to a stack frame at the moment when
the exception is thrown.  The ill function here -
libsupc++/eh_throw.cc/__cxa_throw (and, very probably, __cxa_rethrow
as well).  The problem vanishes, if we recompile eh_throw.cc with
-fno-omit-frame-pointer.  But it's not the real cure.  As I
understand, FRAME_POINTER_REQUIRED must give true for such a function
(by the reason of calling _Unwind_RaiseException inside of it?), but
gcc fails to do that.

The other similar problem is with glibc/linuxthreads.  If
libpthread.so is compiled with -fomit-frame-pointer in CFLAGS, then
any threaded app finishes with SIGSEGV just after the start.  It
seems, that the problem here is in an absence of ebp correctly set
around setjmp/longjmp calls.  There are -fno-omit-frame-pointer flags
especially set for some files in the glibc/linuxthreads tree, but this
doesn't help.

I assume gcc 3.0.4, 3.1 and glibc 2.2.5.

Do I understand right, that it's a gcc's care to set up the stack
frame register when it's necessary, regardless of the
-fomit-frame-pointer flag?

Re: BUG? -fomit-frame-pointer and exceptions or setjmp/longjmp

[Richard Henderson]

On Mon, Jun 03, 2002 at 03:29:31AM +0600, Denis Zaitsev wrote:
> If gcc is built with the -fomit-frame-pointer in the environment's
> CXXFLAGS, then we have a dead exception handling mechanics - a c++
> program, compiled with such a gcc, can't catch any of an exceptions it
> throws.

If true, that's a bug.  The frame pointer is NOT necessary to
throw or catch exceptions.



r~

Re: BUG? -fomit-frame-pointer and exceptions or setjmp/longjmp

[Denis Zaitsev]

On Sun, Jun 02, 2002 at 03:11:41PM -0700, Richard Henderson wrote:
> On Mon, Jun 03, 2002 at 03:29:31AM +0600, Denis Zaitsev wrote:
> > If gcc is built with the -fomit-frame-pointer in the environment's
> > CXXFLAGS, then we have a dead exception handling mechanics - a c++
> > program, compiled with such a gcc, can't catch any of an exceptions it
> > throws.
> 
> If true, that's a bug.  The frame pointer is NOT necessary to
> throw or catch exceptions.
> 
But is it necessary for the things like __builtin_(frame|return)_address,
if the level is much deep than 0?

Re: BUG? -fomit-frame-pointer and exceptions or setjmp/longjmp

[Richard Henderson]

On Mon, Jun 03, 2002 at 04:33:41AM +0600, Denis Zaitsev wrote:
> > If true, that's a bug.  The frame pointer is NOT necessary to
> > throw or catch exceptions.
> > 
> But is it necessary for the things like __builtin_(frame|return)_address,
> if the level is much deep than 0?

That's not how exceptions are implemented.  We examine
dwarf2 frame unwind information that describes the layout
of the frame very very well.



r~

Re: BUG? -fomit-frame-pointer and exceptions or setjmp/longjmp

[Nix]

On Sun, 2 Jun 2002, Richard Henderson said:
> On Mon, Jun 03, 2002 at 03:29:31AM +0600, Denis Zaitsev wrote:
>> If gcc is built with the -fomit-frame-pointer in the environment's
>> CXXFLAGS, then we have a dead exception handling mechanics - a c++
>> program, compiled with such a gcc, can't catch any of an exceptions it
>> throws.
> 
> If true, that's a bug.  The frame pointer is NOT necessary to
> throw or catch exceptions.

I see similar things happening inside GiNaC-1.0.8, on an i586, only when
GiNaC is built with -fomit-frame-pointer:

loki 506 /usr/packages/GiNaC/i586-loki/check% ./exams 
GNU gdb 5.2
Copyright 2002 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "i586-pc-linux-gnu"...(no debugging symbols found)...
(gdb) run
Starting program: /usr/packages/GiNaC/i586-loki/check/.libs/lt-exams 
(no debugging symbols found)...
Program received signal SIGSEGV, Segmentation fault.
0x402b1b16 in uw_frame_state_for (context=0xbffff360, fs=0xbffff2a0) at /usr/packages/gcc/3.1/gcc/unwind-dw2.c:910
910           MD_FALLBACK_FRAME_STATE_FOR (context, fs, success);
(gdb) bt
#0  0x402b1b16 in uw_frame_state_for (context=0xbffff360, fs=0xbffff2a0) at /usr/packages/gcc/3.1/gcc/unwind-dw2.c:910
#1  0x402b2108 in _Unwind_RaiseException (exc=0x81952a0) at /usr/packages/gcc/3.1/gcc/unwind.inc:93
#2  0x40289d48 in __cxa_throw (obj=0x81952a0, tinfo=0x402ada20, dest=0x402ada20 <globals_static>)
    at /usr/packages/gcc/3.1/libstdc++-v3/libsupc++/eh_throw.cc:72
#3  0x40116346 in GiNaC::power::eval(int) const () from /usr/packages/GiNaC/i586-loki/ginac/.libs/libginac-1.0.so.0
#4  0x4005fa8c in GiNaC::ex::construct_from_basic(GiNaC::basic const&) () from /usr/packages/GiNaC/i586-loki/ginac/.libs/libginac-1.0.so.0
#5  0x08060788 in std::__simple_alloc<GiNaC::ex, std::__default_alloc_template<true, 0> >::deallocate(GiNaC::ex*, unsigned) ()
#6  0x404a67f9 in __libc_start_main () from /lib/libc.so.6
#7  0xf0e48350 in ?? ()

I've isolated the throw that causes the crash, but haven't finished
trimming the thing down to a reproducible testcase of sane size. (The
throw at fault is on line 345 of power.cpp, the pow(0,0)-undefined
error case.)

As the trace shows, this crash occurs during unwinding, the presence or
absence of a catch is irrelevant. I'm rather wondering why we're hitting
MD_FALLBACK_FRAME_STATE_FOR at all: perhaps there's a problem in FDE
generation or something. (The fallback looks, er, odd to my eyes: diffing
assembler opcodes by hand just looks *ugly*.)

(Note that there are constructors, MI, and inlined functions at play
here.)


Tracking this down some more is on my to-do list if nobody else gets
there first: it might be related to the huge mass of g++ test failures
(in which exception throwing crashes) you get if GCC is built with
-fomit-frame-pointer.

-- 
`What happened?'
                 `Nick shipped buggy code!'
                                             `Oh, no dinner for him...'

Re: BUG? -fomit-frame-pointer and exceptions or setjmp/longjmp

[Denis Zaitsev]

On Mon, Jun 10, 2002 at 09:48:28PM +0100, Nix wrote:
> ...
> As the trace shows, this crash occurs during unwinding, the presence or
> absence of a catch is irrelevant. I'm rather wondering why we're hitting
> MD_FALLBACK_FRAME_STATE_FOR at all: perhaps there's a problem in FDE
> generation or something.

Exact, uw_update_context_1 calculates bad context->cfa, if it works
for a function with the frame pointer omited.  Now I'm looking what
follows...

Re: BUG? -fomit-frame-pointer and exceptions or setjmp/longjmp

[Nix]

On Fri, 14 Jun 2002, Denis Zaitsev said:
> On Mon, Jun 10, 2002 at 09:48:28PM +0100, Nix wrote:
>> ...
>> As the trace shows, this crash occurs during unwinding, the presence or
>> absence of a catch is irrelevant. I'm rather wondering why we're hitting
>> MD_FALLBACK_FRAME_STATE_FOR at all: perhaps there's a problem in FDE
>> generation or something.
> 
> Exact, uw_update_context_1 calculates bad context->cfa, if it works
> for a function with the frame pointer omited.  Now I'm looking what
> follows...

Ah, so it was `working' beforehand only because the fallback function
was recovering from the FDE unwinder's[1] errors?

Why is there a fallback function at all? It doesn't work when fairly
common switches are used, so its usefulness is limited, at best (bit of
a shame the if-of-opcodes hack in the i386 variant is choosing to diff
against the frame pointer, but there's not much else to show where a
function starts, I guess...)

[1] or the FDE frame-location emitter, or something like that

-- 
`What happened?'
                 `Nick shipped buggy code!'
                                             `Oh, no dinner for him...'

Re: BUG? -fomit-frame-pointer and exceptions or setjmp/longjmp

[Andrew Haley]

Nix writes:
 > On Fri, 14 Jun 2002, Denis Zaitsev said:
 > > On Mon, Jun 10, 2002 at 09:48:28PM +0100, Nix wrote:
 > >> ...
 > >> As the trace shows, this crash occurs during unwinding, the presence or
 > >> absence of a catch is irrelevant. I'm rather wondering why we're hitting
 > >> MD_FALLBACK_FRAME_STATE_FOR at all: perhaps there's a problem in FDE
 > >> generation or something.

Probably, yes.

 > > Exact, uw_update_context_1 calculates bad context->cfa, if it works
 > > for a function with the frame pointer omited.  Now I'm looking what
 > > follows...
 > 
 > Ah, so it was `working' beforehand only because the fallback function
 > was recovering from the FDE unwinder's[1] errors?

I don't think it can do that.

 > Why is there a fallback function at all?

In order to unpick signal handler frames.  The idea is that if we fail
to find information for the previous frame, we invoke an operating
system specific fallback handler, which in turn recognizes a signal
handler frame and unwinds through it.

If you've hit MD_FALLBACK_FRAME_STATE_FOR and there has been no signal
it's a bug, as Richard said.

Andrew.

Re: BUG? -fomit-frame-pointer and exceptions or setjmp/longjmp

[Nix]

On Fri, 14 Jun 2002, Andrew Haley stipulated:
> Nix writes:
>  > Why is there a fallback function at all?
> 
> In order to unpick signal handler frames.  The idea is that if we fail
> to find information for the previous frame, we invoke an operating
> system specific fallback handler, which in turn recognizes a signal
> handler frame and unwinds through it.

Aah, hence the comments regarding `handlers' in unwind.inc. I see now.

Wow, ingenious. I'd assumed that throwing through signal handlers was a
no-hope-you're-dead case :) so there's nothing wrong with it and it's
tripping at the wrong moment because of an oversight in FDE generation,
then.

So all I need to do is learn how to read FDEs from the DWARF2 spec. I've
read enough to realise it's not *at all* simple; the sort of thing
that's better done with scripts to make sense of the info, even when
reading them from .S files

> If you've hit MD_FALLBACK_FRAME_STATE_FOR and there has been no signal
> it's a bug, as Richard said.

There's been no signal, and the problem is quite reproducible.
(*Reducing* it to a small testcase, though, is a bit of a swine.)

-- 
`What happened?'
                 `Nick shipped buggy code!'
                                             `Oh, no dinner for him...'

Re: BUG? -fomit-frame-pointer and exceptions or setjmp/longjmp

[Richard Henderson]

On Fri, Jun 14, 2002 at 11:21:09PM +0100, Nix wrote:
> So all I need to do is learn how to read FDEs from the DWARF2 spec. I've
> read enough to realise it's not *at all* simple; the sort of thing
> that's better done with scripts to make sense of the info, even when
> reading them from .S files

readelf -wf on .o files; gcc -dA -S for .s files.


r~

Re: BUG? -fomit-frame-pointer and exceptions or setjmp/longjmp

[Denis Zaitsev]

On Fri, Jun 14, 2002 at 08:00:12PM +0100, Nix wrote:
> On Fri, 14 Jun 2002, Denis Zaitsev said:
> > On Mon, Jun 10, 2002 at 09:48:28PM +0100, Nix wrote:
> >> ...
> >> As the trace shows, this crash occurs during unwinding, the presence or
> >> absence of a catch is irrelevant. I'm rather wondering why we're hitting
> >> MD_FALLBACK_FRAME_STATE_FOR at all: perhaps there's a problem in FDE
> >> generation or something.
> > 
> > Exact, uw_update_context_1 calculates bad context->cfa, if it works
> > for a function with the frame pointer omited.  Now I'm looking what
> > follows...
> 
> Ah, so it was `working' beforehand only because the fallback function
> was recovering from the FDE unwinder's[1] errors?

No, it seems that it's because of ebp set - this saves us from a bad
DWARF info...

readelf shows that DWARF's encoding of the CFA is very broken.  It is
not broken for only a few first pushes a function does.  And if that
function is compiled with the frame pointer not omited, that this
frame pointer will be set thru these first steps.  After that DWARF
info refers for it in the CFA description.  So, we have the CFA ok
despite of the wrong DWARF info about the rest of the function.

But we catch all the bad things, if the function is compiled with the
frame pointer omited.  That way the CFA is described thru the stack
pointer register.  It really drifts thru the function's flow, but the
DWARF info is not describing this drift correctly.  So, this way we
can't avoid the broken DWARF stuff.

BTW, it seems that many GDB problems which are discussed in this list
have the same origin, but I can't affirm...

Re: BUG? -fomit-frame-pointer and exceptions or setjmp/longjmp

[Daniel Berlin]

On Tue, 18 Jun 2002, Denis Zaitsev wrote:

> On Fri, Jun 14, 2002 at 08:00:12PM +0100, Nix wrote:
> > On Fri, 14 Jun 2002, Denis Zaitsev said:
> > > On Mon, Jun 10, 2002 at 09:48:28PM +0100, Nix wrote:
> > >> ...
> > >> As the trace shows, this crash occurs during unwinding, the presence or
> > >> absence of a catch is irrelevant. I'm rather wondering why we're hitting
> > >> MD_FALLBACK_FRAME_STATE_FOR at all: perhaps there's a problem in FDE
> > >> generation or something.
> > > 
> > > Exact, uw_update_context_1 calculates bad context->cfa, if it works
> > > for a function with the frame pointer omited.  Now I'm looking what
> > > follows...
> > 
> > Ah, so it was `working' beforehand only because the fallback function
> > was recovering from the FDE unwinder's[1] errors?
> 
> No, it seems that it's because of ebp set - this saves us from a bad
> DWARF info...
> 
> readelf shows that DWARF's encoding of the CFA is very broken.  It is
> not broken for only a few first pushes a function does.  And if that
> function is compiled with the frame pointer not omited, that this
> frame pointer will be set thru these first steps.  After that DWARF
> info refers for it in the CFA description.  So, we have the CFA ok
> despite of the wrong DWARF info about the rest of the function.
> 
> But we catch all the bad things, if the function is compiled with the
> frame pointer omited.  That way the CFA is described thru the stack
> pointer register.  It really drifts thru the function's flow, but the
> DWARF info is not describing this drift correctly.  So, this way we
> can't avoid the broken DWARF stuff.
> 
> BTW, it seems that many GDB problems which are discussed in this list
> have the same origin, but I can't affirm...
I sincerely doubt it, since only x86-64 reads or uses the frame info.

> 
> 

PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Denis Zaitsev]

The problem (i386): GCC generates bad DWARF call frame info, when it
works with -fomit-frame-pointer, so it becomes impossible to unwind
the call frame correctly.  This makes, for example, any thrown
exception uncatchable.  (This problem has already been discussed here.)

The place, where the call frame info goes astray, is loading of the
PIC register.  It consists of a call for a "just after the call" point
and then pop pc's value there.  The call must be considered just as it
would be push(pc), but instead it asumed to be a usual call, which
leaves the stack pointer the same after return.  And it is an error.

The patch below cures the problem.  Please, apply it.  It is for
3.0.4, but does for 3.1 too.


--- gcc/config/i386/i386.c.orig	Fri Nov 30 02:48:41 2001
+++ gcc/config/i386/i386.c	Tue Jul  2 03:32:29 2002
@@ -1877,7 +1877,7 @@
 void
 load_pic_register ()
 {
-  rtx gotsym, pclab;
+  rtx gotsym, pclab, getpc;
 
   gotsym = gen_rtx_SYMBOL_REF (Pmode, "_GLOBAL_OFFSET_TABLE_");
 
@@ -1892,7 +1892,15 @@
       pclab = gen_rtx_LABEL_REF (VOIDmode, gen_label_rtx ());
     }
 
-  emit_insn (gen_prologue_get_pc (pic_offset_table_rtx, pclab));
+  getpc = emit_insn (gen_prologue_get_pc (pic_offset_table_rtx, pclab));
+  RTX_FRAME_RELATED_P (getpc) = 1;
+  REG_NOTES (getpc) = gen_rtx_EXPR_LIST (REG_FRAME_RELATED_EXPR,
+					 gen_rtx_SET (VOIDmode,
+						      stack_pointer_rtx,
+						      gen_rtx_PLUS (SImode,
+								    stack_pointer_rtx,
+								    GEN_INT (-UNITS_PER_WORD))),
+					 REG_NOTES (getpc));
 
   if (! TARGET_DEEP_BRANCH_PREDICTION)
     emit_insn (gen_popsi1 (pic_offset_table_rtx));

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Paolo Carlini]

Denis Zaitsev wrote:

[snip]

>The patch below cures the problem.  Please, apply it.  It is for
>3.0.4, but does for 3.1 too.
>
I think that, as a rule, you should at least:
1- Describe in detail how did you test the patch (triple, languages).
2- Provide a testcase.
3- Provide a Changelog.
4- Finally, post the bundle to gcc-patches *not* gcc.

Ciao, Paolo.

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Richard Henderson]

On Tue, Jul 02, 2002 at 05:08:49AM +0600, Denis Zaitsev wrote:
> The place, where the call frame info goes astray, is loading of the
> PIC register.  It consists of a call for a "just after the call" point
> and then pop pc's value there.  The call must be considered just as it
> would be push(pc), but instead it asumed to be a usual call, which
> leaves the stack pointer the same after return.  And it is an error.

While this patch may or may not be correct, it is definitely
the only problem.  See PR 6678 for an example that does not
require -fpic at all.

In any case, I won't apply this.  I'd rather see the POP insn
merged into the prologue_get_pc pattern.


r~

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Richard Henderson]

On Tue, Jul 02, 2002 at 04:45:55PM -0700, Richard Henderson wrote:
> While this patch may or may not be correct, it is definitely
> the only problem.  See PR 6678 for an example that does not

     ^ not

Grr.


r~

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Denis Zaitsev]

On Tue, Jul 02, 2002 at 04:45:55PM -0700, Richard Henderson wrote:
> On Tue, Jul 02, 2002 at 05:08:49AM +0600, Denis Zaitsev wrote:
> While this patch may or may not be correct, it is definitely
> the only problem.  See PR 6678 for an example that does not
> require -fpic at all.

The call frame info has other problems too, if you are talking about
that.  But these are the other, not this problem.  BTW, where PR 6678
is?  I haven't found it in the list.

> In any case, I won't apply this.  I'd rather see the POP insn
> merged into the prologue_get_pc pattern.

Will it be merged or won't, how will it influence over the
relationship between the call insn and dwarf2out_frame_debug_expr's
behaviour?  I mean that this is a very special call, and so it should
be followed with the instructions for the call frame info generation,
else dwarf2out_frame_debug_expr does the wrong things.  What do you
think is wrong here?

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Denis Zaitsev]

On Tue, Jul 02, 2002 at 12:21:22PM +0200, Paolo Carlini wrote:
> Denis Zaitsev wrote:
> 
> [snip]
> 
> >The patch below cures the problem.  Please, apply it.  It is for
> >3.0.4, but does for 3.1 too.
> >
> I think that, as a rule, you should at least:
> 1- Describe in detail how did you test the patch (triple, languages).
> 2- Provide a testcase.
> 3- Provide a Changelog.
> 4- Finally, post the bundle to gcc-patches *not* gcc.
> 
> Ciao, Paolo.
> 

Ok, thanks.

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Denis Zaitsev]

On Tue, Jul 02, 2002 at 04:45:55PM -0700, Richard Henderson wrote:
> While this patch may or may not be correct, it is definitely
> the only problem.  See PR 6678 for an example that does not
> require -fpic at all.
> 

Ok, I've found this PR 6678.  It can happy be a complete another
problem.  While my patch cures a concrete bug...  And it is not
eh-related, its subject is bad DWARF call frame info generation.
Exception handling problems are just the consecuencies.  So, I'm
sorry, it seems that I misunderstand something, - what do you mean?

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Richard Henderson]

On Wed, Jul 03, 2002 at 10:55:00PM +0600, Denis Zaitsev wrote:
> While my patch cures a concrete bug...  And it is not
> eh-related, its subject is bad DWARF call frame info generation.

Err..  EH uses call frame info to do it's job.  So both problems are
call frame info related.  So I miss your point.

Anyway, looking again, your patch is buggy in that it breaks
TARGET_DEEP_BRANCH_PREDICTION in exactly the opposite way as the
!TARGET_DEEP_BRANCH_PREDICTION bug you tried to fix.

Mark, I'd like to apply this simpler patch to 3.1.  Summary of the bug
description is that exposing the pop instruction confuses the dwarf2out
routines that scan the insn stream for SP changes, so we mis-calculate
the CFA from that point forward.  The simplest solution is to bury it
inside the pattern that it is always paired with.

This issue has been accidentally fixed on mainline in the process of
cleaning up this section of code.


r~


	* config/i386/i386.md (prologue_get_pc): Issue pop here ...
	* config/i386/i386.c (load_pic_register): ... not here.

Index: i386.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/i386/i386.c,v
retrieving revision 1.368.2.17
diff -c -p -d -r1.368.2.17 i386.c
*** i386.c	16 Jun 2002 22:34:54 -0000	1.368.2.17
--- i386.c	3 Jul 2002 19:58:51 -0000
*************** load_pic_register ()
*** 3918,3926 ****
  
    emit_insn (gen_prologue_get_pc (pic_offset_table_rtx, pclab));
  
-   if (! TARGET_DEEP_BRANCH_PREDICTION)
-     emit_insn (gen_popsi1 (pic_offset_table_rtx));
- 
    emit_insn (gen_prologue_set_got (pic_offset_table_rtx, gotsym, pclab));
  }
  
--- 3918,3923 ----
Index: i386.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/i386/i386.md,v
retrieving revision 1.339.2.11
diff -c -p -d -r1.339.2.11 i386.md
*** i386.md	4 Jun 2002 12:47:51 -0000	1.339.2.11
--- i386.md	3 Jul 2002 19:58:52 -0000
***************
*** 13815,13820 ****
--- 13815,13821 ----
      {
        ASM_OUTPUT_INTERNAL_LABEL (asm_out_file, "L",
  				 CODE_LABEL_NUMBER (operands[1]));
+       return "pop{l}\t%0";
      }
    RET;
  }

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Mark Mitchell]

--On Wednesday, July 03, 2002 01:13:20 PM -0700 Richard Henderson 
<rth@redhat.com> wrote:

> On Wed, Jul 03, 2002 at 10:55:00PM +0600, Denis Zaitsev wrote:
>> While my patch cures a concrete bug...  And it is not
>> eh-related, its subject is bad DWARF call frame info generation.
>
> Err..  EH uses call frame info to do it's job.  So both problems are
> call frame info related.  So I miss your point.
>
> Anyway, looking again, your patch is buggy in that it breaks
> TARGET_DEEP_BRANCH_PREDICTION in exactly the opposite way as the
> !TARGET_DEEP_BRANCH_PREDICTION bug you tried to fix.
>
> Mark, I'd like to apply this simpler patch to 3.1.

OK.  Thanks for the nice simple explanation.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Denis Zaitsev]

On Wed, Jul 03, 2002 at 01:13:20PM -0700, Richard Henderson wrote:
> On Wed, Jul 03, 2002 at 10:55:00PM +0600, Denis Zaitsev wrote:
> > While my patch cures a concrete bug...  And it is not
> > eh-related, its subject is bad DWARF call frame info generation.
> 
> Err..  EH uses call frame info to do it's job.  So both problems are
> call frame info related.  So I miss your point.
> 
> Anyway, looking again, your patch is buggy in that it breaks
> TARGET_DEEP_BRANCH_PREDICTION in exactly the opposite way as the
> !TARGET_DEEP_BRANCH_PREDICTION bug you tried to fix.

Indeed, the fixed version is below...

> Mark, I'd like to apply this simpler patch to 3.1.  Summary of the bug
> description is that exposing the pop instruction confuses the dwarf2out
> routines that scan the insn stream for SP changes, so we mis-calculate
> the CFA from that point forward.  The simplest solution is to bury it
> inside the pattern that it is always paired with.

Yes, your solution is simpler.  But:

a) strictly speaking, the dwarf2out routines aren't confused by the
POP, but by that special CALL;

b) you are just hiding the motion of SP instead of really showing it,
this doesn't harm to EH, but can swindle debugger.


2002-07-04  Denis Zaitsev  <zzz@cd-club.ru>

	* i386.c (load_pic_register): fix call frame info generation.


--- gcc/config/i386/i386.c.orig	Fri Nov 30 02:48:41 2001
+++ gcc/config/i386/i386.c	Thu Jul  4 03:29:10 2002
@@ -1877,7 +1877,7 @@
 void
 load_pic_register ()
 {
-  rtx gotsym, pclab;
+  rtx gotsym, pclab, getpc;
 
   gotsym = gen_rtx_SYMBOL_REF (Pmode, "_GLOBAL_OFFSET_TABLE_");
 
@@ -1892,10 +1892,22 @@
       pclab = gen_rtx_LABEL_REF (VOIDmode, gen_label_rtx ());
     }
 
-  emit_insn (gen_prologue_get_pc (pic_offset_table_rtx, pclab));
+  getpc = emit_insn (gen_prologue_get_pc (pic_offset_table_rtx, pclab));
 
   if (! TARGET_DEEP_BRANCH_PREDICTION)
-    emit_insn (gen_popsi1 (pic_offset_table_rtx));
+    {
+      RTX_FRAME_RELATED_P (getpc) = 1;
+      REG_NOTES (getpc) = gen_rtx_EXPR_LIST (
+	  REG_FRAME_RELATED_EXPR,
+	  gen_rtx_SET (VOIDmode,
+		       stack_pointer_rtx,
+		       gen_rtx_PLUS (SImode,
+				     stack_pointer_rtx,
+				     GEN_INT (-UNITS_PER_WORD))),
+	  REG_NOTES (getpc));
+
+      emit_insn (gen_popsi1 (pic_offset_table_rtx));
+    }
 
   emit_insn (gen_prologue_set_got (pic_offset_table_rtx, gotsym, pclab));
 }

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Richard Henderson]

On Thu, Jul 04, 2002 at 04:15:57AM +0600, Denis Zaitsev wrote:
> a) strictly speaking, the dwarf2out routines aren't confused by the
> POP, but by that special CALL;

A matter of perspective.

> b) you are just hiding the motion of SP instead of really showing it,
> this doesn't harm to EH, but can swindle debugger.

For all of one instruction, iff the user does stepi instead of step, yes.
It wouldn't surprise me if there are other such windows in which the info
is not quite consistent.

Anyway, even with your patch we continue to have the same window in the
TARGET_DEEP_PREDICTION case, which frankly is the one more folk actually
use, so I think we should go with the simpler patch to move the pop insn.


r~

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Denis Zaitsev]

On Wed, Jul 03, 2002 at 03:43:44PM -0700, Richard Henderson wrote:
> It wouldn't surprise me if there are other such windows in which the info
> is not quite consistent.

They really are.

> Anyway, even with your patch we continue to have the same window in the
> TARGET_DEEP_PREDICTION case, which frankly is the one more folk actually
> use, so I think we should go with the simpler patch to move the pop insn.

Do I understand right, that TARGET_DEEP_BRANCH_PREDICTION means the
real call for a helper routine?

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Richard Henderson]

On Thu, Jul 04, 2002 at 05:25:13AM +0600, Denis Zaitsev wrote:
> Do I understand right, that TARGET_DEEP_BRANCH_PREDICTION means the
> real call for a helper routine?

Yes.


r~

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Denis Zaitsev]

On Wed, Jul 03, 2002 at 04:35:36PM -0700, Richard Henderson wrote:
> On Thu, Jul 04, 2002 at 05:25:13AM +0600, Denis Zaitsev wrote:
> > Do I understand right, that TARGET_DEEP_BRANCH_PREDICTION means the
> > real call for a helper routine?
> 
> Yes.

So, how can we have the same window then?  We won't have any such
window at all.  We will have the usual CALL, which is out of the call
frame info scope...

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Richard Henderson]

On Thu, Jul 04, 2002 at 05:44:01AM +0600, Denis Zaitsev wrote:
> So, how can we have the same window then?  We won't have any such
> window at all.  We will have the usual CALL, which is out of the call
> frame info scope...

We have the user doing stepi, and they come to a place at
which they cannot get proper debugging info.


r~

Re: PATCH: -fomit-frame-pointer and exceptions or setjmp/longjmp

[Denis Zaitsev]

On Wed, Jul 03, 2002 at 05:05:53PM -0700, Richard Henderson wrote:
> On Thu, Jul 04, 2002 at 05:44:01AM +0600, Denis Zaitsev wrote:
> > So, how can we have the same window then?  We won't have any such
> > window at all.  We will have the usual CALL, which is out of the call
> > frame info scope...
> 
> We have the user doing stepi, and they come to a place at
> which they cannot get proper debugging info.

No, I mean that:

> Anyway, even with your patch we continue to have the same window in the
> TARGET_DEEP_PREDICTION case, which frankly is the one more folk actually
> use, so I think we should go with the simpler patch to move the pop insn.

I said that under TARGET_DEEP_BRANCH_PREDICTION we won't have the
<call L; L: popl %ebx> window.  So, this case has no relation with the
patch and discussion.