Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@gnat.com>
To: gdb-patches@sources.redhat.com
Subject: [ob(ish)/committed] Fix SEGV in hppa_frame_cache
Date: Fri, 05 Mar 2004 05:05:00 -0000	[thread overview]
Message-ID: <20040305050546.GC1226@gnat.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]

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;

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.

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?
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.

2004-03-04  J. Brobecker  <brobecker@gnat.com>

        * hppa-tdep.c (hppa_frame_cache): Avoid undefined return value.

Comments? Ok for the branch? (already committed in HEAD as, err,
obvious - well, at least we don't crash anymore :-)

Thanks,
-- 
Joel

[-- Attachment #2: hppa-tdep.c.diff --]
[-- Type: text/plain, Size: 537 bytes --]

Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.129
diff -u -p -r1.129 hppa-tdep.c
--- hppa-tdep.c	27 Feb 2004 21:47:53 -0000	1.129
+++ hppa-tdep.c	5 Mar 2004 04:46:32 -0000
@@ -4617,7 +4617,7 @@ hppa_frame_cache (struct frame_info *nex
   /* Yow! */
   u = find_unwind_entry (frame_func_unwind (next_frame));
   if (!u)
-    return;
+    return (*this_cache);
 
   /* Turn the Entry_GR field into a bitmask.  */
   saved_gr_mask = 0;

WARNING: multiple messages have this Message-ID
From: Joel Brobecker <brobecker@gnat.com>
To: gdb-patches@sources.redhat.com
Subject: [ob(ish)/committed] Fix SEGV in hppa_frame_cache
Date: Fri, 19 Mar 2004 00:09:00 -0000	[thread overview]
Message-ID: <20040305050546.GC1226@gnat.com> (raw)
Message-ID: <20040319000900._ogOb7kmP7YpvUddIfoDZ-SxxNzZ8-mIprA0SbJ8Z5w@z> (raw)

[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]

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;

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.

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?
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.

2004-03-04  J. Brobecker  <brobecker@gnat.com>

        * hppa-tdep.c (hppa_frame_cache): Avoid undefined return value.

Comments? Ok for the branch? (already committed in HEAD as, err,
obvious - well, at least we don't crash anymore :-)

Thanks,
-- 
Joel

[-- Attachment #2: hppa-tdep.c.diff --]
[-- Type: text/plain, Size: 537 bytes --]

Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.129
diff -u -p -r1.129 hppa-tdep.c
--- hppa-tdep.c	27 Feb 2004 21:47:53 -0000	1.129
+++ hppa-tdep.c	5 Mar 2004 04:46:32 -0000
@@ -4617,7 +4617,7 @@ hppa_frame_cache (struct frame_info *nex
   /* Yow! */
   u = find_unwind_entry (frame_func_unwind (next_frame));
   if (!u)
-    return;
+    return (*this_cache);
 
   /* Turn the Entry_GR field into a bitmask.  */
   saved_gr_mask = 0;

             reply	other threads:[~2004-03-05  5:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-03-05  5:05 Joel Brobecker [this message]
2004-03-19  0:09 ` Andrew Cagney
2004-03-06  0:42   ` Andrew Cagney
2004-03-10 20:09   ` Joel Brobecker
2004-03-19  0: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=20040305050546.GC1226@gnat.com \
    --to=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