[RFC] Is inside_entry_file useful and needed?

[RFC] Is inside_entry_file useful and needed?

[Corinna Vinschen]

Hi,

the reason I'm going to discuss the inside_entry_file() function is that
I had trouble with the asm-source.exp tests on both platforms I currently
care about, Cygwin and XStormy16, one native, one embedded target.

Both targets have the same failures in the asm-source.exp tests:

(gdb) FAIL: gdb.asm/asm-source.exp: bt ALL in foo2
(gdb) FAIL: gdb.asm/asm-source.exp: bt 2 in foo2
(gdb) FAIL: gdb.asm/asm-source.exp: bt 3 in foo3
(gdb) FAIL: gdb.asm/asm-source.exp: finish from foo3
(gdb) FAIL: gdb.asm/asm-source.exp: info source asmsrc2.s
(gdb) FAIL: gdb.asm/asm-source.exp: info line
(gdb) FAIL: gdb.asm/asm-source.exp: next over foo3

After some debugging it turned out that the cause for these failures is
a misbehaving of the frame code due to the return value of inside_entry_file().

This function returns "true" for all functions implemented in asmsrc1.s
for obvious reasons - the assembler test is not linked with an actual
startup file like crt0.o.

Due to the return code of inside_entry_file(), get_prev_frame() misbehaves
in the first place.  It returns NULL prematurely, no chance left to
ask the target dependent code if a prev frame might exist.

The second place where it goes wrong is frame_chain_valid(), called from
legacy_get_prev_frame().  Here the same happens again, the inside_entry_file()
function is called before the target dependent code might express another
opinion about the validity of the frame chain.

In the case of XStorm16, everything's ok when removing the calls to
inside_entry_file().  Cygwin, on the other hand still is wrong. 5
out of these 7 error are still left on a Cygwin box.  Why?

Digging a bit more I found that once more inside_entry_file() was the
culprit.  This time in i386_frame_chain().  That function ends with:

  if (! inside_entry_file (get_frame_pc (frame)))
    return read_memory_unsigned_integer (get_frame_base (frame), 4);
  return 0;

which invariably fails to return the correct address in the above case.

For testing purposes I've changed the above three lines to just one:

  return read_memory_unsigned_integer (get_frame_base (frame), 4);

Now everythings ok for Cygwin as well.  With these three changes, both
targets behave fine and no regressions are introduced in the testsuite.


Q1: Is there actually a target which depends on the inside_entry_file()
    function?

Q2: If Q1 has to be answered with "yes", I'd like to propose the introduction
    of a predicate which can be set by the target code to disable the
    inside_entry_file() function as already noted in two comments in the
    gdb source code:

frame.c::get_prev_frame():

  /* If we're inside the entry file, it isn't valid.  Don't apply this
     test to a dummy frame - dummy frame PC's typically land in the
     entry file.  Don't apply this test to the sentinel frame.
     Sentinel frames should always be allowed to unwind.  */
  /* NOTE: drow/2002-12-25: should there be a way to disable this
     check?  It assumes a single small entry file, and the way some
     debug readers (e.g.  dbxread) figure out which object is the
     entry file is somewhat hokey.  */
  /* NOTE: cagney/2003-01-10: If there is a way of disabling this test
     then it should probably be moved to before the ->prev_p test,
     above.  */

blockframe.c::frame_chain_valid():

  /* If we're inside the entry file, it isn't valid.  */
  /* NOTE/drow 2002-12-25: should there be a way to disable this check?  It
     assumes a single small entry file, and the way some debug readers (e.g.
     dbxread) figure out which object is the entry file is somewhat hokey.  */


Thoughts?

Corinna

-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.
mailto:vinschen@redhat.com

Re: [RFC] Is inside_entry_file useful and needed?

[Andrew Cagney]

> Hi,
> 
> the reason I'm going to discuss the inside_entry_file() function is that
> I had trouble with the asm-source.exp tests on both platforms I currently
> care about, Cygwin and XStormy16, one native, one embedded target.

Please note that these tests work on i386 GNU/Linux (and d10v).

> Both targets have the same failures in the asm-source.exp tests:
> 
> (gdb) FAIL: gdb.asm/asm-source.exp: bt ALL in foo2
> (gdb) FAIL: gdb.asm/asm-source.exp: bt 2 in foo2
> (gdb) FAIL: gdb.asm/asm-source.exp: bt 3 in foo3
> (gdb) FAIL: gdb.asm/asm-source.exp: finish from foo3 
> (gdb) FAIL: gdb.asm/asm-source.exp: info source asmsrc2.s
> (gdb) FAIL: gdb.asm/asm-source.exp: info line
> (gdb) FAIL: gdb.asm/asm-source.exp: next over foo3
> 
> After some debugging it turned out that the cause for these failures is
> a misbehaving of the frame code due to the return value of inside_entry_file().
> 
> This function returns "true" for all functions implemented in asmsrc1.s
> for obvious reasons - the assembler test is not linked with an actual
> startup file like crt0.o.

> Due to the return code of inside_entry_file(), get_prev_frame() misbehaves
> in the first place.  It returns NULL prematurely, no chance left to
> ask the target dependent code if a prev frame might exist.
> 
> The second place where it goes wrong is frame_chain_valid(), called from
> legacy_get_prev_frame().  Here the same happens again, the inside_entry_file()
> function is called before the target dependent code might express another
> opinion about the validity of the frame chain.
> 
> In the case of XStorm16, everything's ok when removing the calls to
> inside_entry_file().  Cygwin, on the other hand still is wrong. 5
> out of these 7 error are still left on a Cygwin box.  Why?
> 
> Digging a bit more I found that once more inside_entry_file() was the
> culprit.  This time in i386_frame_chain().  That function ends with:
> 
>   if (! inside_entry_file (get_frame_pc (frame)))
>     return read_memory_unsigned_integer (get_frame_base (frame), 4);
>   return 0;
> 
> which invariably fails to return the correct address in the above case.
> 
> For testing purposes I've changed the above three lines to just one:
> 
>   return read_memory_unsigned_integer (get_frame_base (frame), 4);
> 
> Now everythings ok for Cygwin as well.  With these three changes, both
> targets behave fine and no regressions are introduced in the testsuite.
> 
> 
> Q1: Is there actually a target which depends on the inside_entry_file()
>     function?

There definitly was.  The code would have been added to fix a problem.

> 
> Q2: If Q1 has to be answered with "yes", I'd like to propose the introduction
>     of a predicate which can be set by the target code to disable the
>     inside_entry_file() function as already noted in two comments in the
>     gdb source code:

Please note that these two tests are not identical.

> frame.c::get_prev_frame():
> 
>   /* If we're inside the entry file, it isn't valid.  Don't apply this
>      test to a dummy frame - dummy frame PC's typically land in the
>      entry file.  Don't apply this test to the sentinel frame.
>      Sentinel frames should always be allowed to unwind.  */
>   /* NOTE: drow/2002-12-25: should there be a way to disable this
>      check?  It assumes a single small entry file, and the way some
>      debug readers (e.g.  dbxread) figure out which object is the
>      entry file is somewhat hokey.  */
>   /* NOTE: cagney/2003-01-10: If there is a way of disabling this test
>      then it should probably be moved to before the ->prev_p test,
>      above.  */

This thest terminates the unwind after there has been a stack frame in 
the startup file.  Remember, the d10v, which only relies on this test, 
works.

> blockframe.c::frame_chain_valid():
> 
>   /* If we're inside the entry file, it isn't valid.  */
>   /* NOTE/drow 2002-12-25: should there be a way to disable this check?  It
>      assumes a single small entry file, and the way some debug readers (e.g.
>      dbxread) figure out which object is the entry file is somewhat hokey.  */

This call terminates the unwind before there has been a stack frame in 
the startup file.

Is there something significnatly different between i386 GNU/Linux and, 
say, cygwin?

Andrew

Re: [RFC] Is inside_entry_file useful and needed?

[Corinna Vinschen]

On Thu, Mar 27, 2003 at 12:08:17PM -0500, Andrew Cagney wrote:
> >Hi,
> >
> >the reason I'm going to discuss the inside_entry_file() function is that
> >I had trouble with the asm-source.exp tests on both platforms I currently
> >care about, Cygwin and XStormy16, one native, one embedded target.
> 
> Please note that these tests work on i386 GNU/Linux (and d10v).

I don't doubt that they work on other targets but they apparently fail
due to the calls of the inside_entry_file() for these two targets.

Note that Linux doesn't need the inside_entry_file() function.  All
tests are still working as expected if all calls are dropped as in
my patch on gdb-patches.

> >frame.c::get_prev_frame():
> >[...]
> This thest terminates the unwind after there has been a stack frame in 
> the startup file.  Remember, the d10v, which only relies on this test, 
> works.

Please note that I don't say to remove the inside_entry_file() tests
entirely from gdb.  But it's obviously wrong to call this stuff inside
of the generic functionality.  This should remain in some way inside
of the target specific code.  Either by having a way to disable the calls
for a target or by having a architecture method which is called from
inside the inside_entry_file() function itself.

> Is there something significnatly different between i386 GNU/Linux and, 
> say, cygwin?

I'm not quite sure what you're up to.  There are a whole bunch of
differences probably, e. g. how applications (even assembler apps)
are linked.

Corinna

-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.
mailto:vinschen@redhat.com