Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Cagney <cagney@gnu.org>
To: Joel Brobecker <brobecker@gnat.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [ob(ish)/committed] Fix SEGV in hppa_frame_cache
Date: Fri, 19 Mar 2004 00:09:00 -0000	[thread overview]
Message-ID: <40491E6B.1000900@gnu.org> (raw)
In-Reply-To: <20040305050546.GC1226@gnat.com>

> I was happily testing what I hoped would be the latest version of
> the next/step patch replacing a complex condition by a frame ID
> comparison, when I discovered that it caused a few problems on
> HP/UX... But of course, HP/UX just got frame-ified!
> 
> Here is a description of the first problem I looked into:
> 
>         (in gdb.base)
>         % gdb
>         (gdb) file coremaker
>         (gdb) core-file corefile
>         (gdb) up
>         *** SEGV ***
> 
> Ooops!
> 
> What happens is that we hit the following code in hppa_frame_cache():
> 
>         /* Yow! */
>         u = find_unwind_entry (frame_func_unwind (next_frame));
>         if (!u)
>           return;

That 'll learn me for using the HP compiler :-/

> Unfortunately, that return causes the return value to be undefined.
> And we later crash while trying to dereference this undefined value
> in hppa_frame_this_id().
> 
> So I fixed it with the attached patch. This fixed 8 tests.
> I didn't commit it to the 6.1 branch yet, as I wanted to wait for
> Andrew's comments first. Don't want to disturb the branch too much.

For HP/UX, I think we'll be doing frequent pull-ups - this will be just 
the first of many bugs :-(  Definitly go ahead.

> There is also something that bothers me. If I understand this code well,
> it looks like we are going to abort the unwinding as soon as we hit a
> frame for which we can't find an associated function. Is that correct?

Yes, its taken straight from hppa_frame_find_saved_regs.

 > That would be very unfortunate, especially after we manage to install
 > the next/step patch I was testing; Once this patch is installed, the
 > chances us GDB trying to unwind from an unknown location will be more
 > important, no? If we don't know how to find our way out of there, then
 > the next/step machinery will be weakened. Andrew, if you confirm my
 > understanding is correct, I'll try to see if we can do better.

The code just needs to be sufficient to unwind one level of PC/FP - so 
that the caller can be found.  Having some of the frame saved pc code 
before the abort would improve things.  As for the SP:

You must be looking for a challenge, from frame_chain:

       if (!u)
         {
           /* We could find this information by examining prologues.  I 
don't
              think anyone has actually written any tools (not even "strip")
              which leave them out of an executable, so maybe this is a moot
              point.  */
           /* ??rehrauer: Actually, it's quite possible to stepi your 
way into
              code that doesn't have unwind entries.  For example, 
stepping into
              the dynamic linker will give you a PC that has none. 
Thus, I've
              disabled this warning. */
#if 0
           warning ("Unable to find unwind for PC 0x%x -- Help!", 
get_frame_pc (t
mp_frame));
#endif
           return (CORE_ADDR) 0;
         }

Andrew



WARNING: multiple messages have this Message-ID
From: Andrew Cagney <cagney@gnu.org>
To: Joel Brobecker <brobecker@gnat.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [ob(ish)/committed] Fix SEGV in hppa_frame_cache
Date: Sat, 06 Mar 2004 00:42:00 -0000	[thread overview]
Message-ID: <40491E6B.1000900@gnu.org> (raw)
Message-ID: <20040306004200.FZysBHNqQWLvGv0XYrDivOapjjGyS2ZB8prw5L5S7Es@z> (raw)
In-Reply-To: <20040305050546.GC1226@gnat.com>

> I was happily testing what I hoped would be the latest version of
> the next/step patch replacing a complex condition by a frame ID
> comparison, when I discovered that it caused a few problems on
> HP/UX... But of course, HP/UX just got frame-ified!
> 
> Here is a description of the first problem I looked into:
> 
>         (in gdb.base)
>         % gdb
>         (gdb) file coremaker
>         (gdb) core-file corefile
>         (gdb) up
>         *** SEGV ***
> 
> Ooops!
> 
> What happens is that we hit the following code in hppa_frame_cache():
> 
>         /* Yow! */
>         u = find_unwind_entry (frame_func_unwind (next_frame));
>         if (!u)
>           return;

That 'll learn me for using the HP compiler :-/

> Unfortunately, that return causes the return value to be undefined.
> And we later crash while trying to dereference this undefined value
> in hppa_frame_this_id().
> 
> So I fixed it with the attached patch. This fixed 8 tests.
> I didn't commit it to the 6.1 branch yet, as I wanted to wait for
> Andrew's comments first. Don't want to disturb the branch too much.

For HP/UX, I think we'll be doing frequent pull-ups - this will be just 
the first of many bugs :-(  Definitly go ahead.

> There is also something that bothers me. If I understand this code well,
> it looks like we are going to abort the unwinding as soon as we hit a
> frame for which we can't find an associated function. Is that correct?

Yes, its taken straight from hppa_frame_find_saved_regs.

 > That would be very unfortunate, especially after we manage to install
 > the next/step patch I was testing; Once this patch is installed, the
 > chances us GDB trying to unwind from an unknown location will be more
 > important, no? If we don't know how to find our way out of there, then
 > the next/step machinery will be weakened. Andrew, if you confirm my
 > understanding is correct, I'll try to see if we can do better.

The code just needs to be sufficient to unwind one level of PC/FP - so 
that the caller can be found.  Having some of the frame saved pc code 
before the abort would improve things.  As for the SP:

You must be looking for a challenge, from frame_chain:

       if (!u)
         {
           /* We could find this information by examining prologues.  I 
don't
              think anyone has actually written any tools (not even "strip")
              which leave them out of an executable, so maybe this is a moot
              point.  */
           /* ??rehrauer: Actually, it's quite possible to stepi your 
way into
              code that doesn't have unwind entries.  For example, 
stepping into
              the dynamic linker will give you a PC that has none. 
Thus, I've
              disabled this warning. */
#if 0
           warning ("Unable to find unwind for PC 0x%x -- Help!", 
get_frame_pc (t
mp_frame));
#endif
           return (CORE_ADDR) 0;
         }

Andrew



  parent reply	other threads:[~2004-03-06  0:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-03-05  5:05 Joel Brobecker
2004-03-19  0:09 ` Joel Brobecker
2004-03-19  0:09 ` Andrew Cagney [this message]
2004-03-06  0:42   ` Andrew Cagney
2004-03-10 20:09   ` Joel Brobecker
2004-03-19  0:09     ` Joel Brobecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=40491E6B.1000900@gnu.org \
    --to=cagney@gnu.org \
    --cc=brobecker@gnat.com \
    --cc=gdb-patches@sources.redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox