From: Joel Brobecker <brobecker@adacore.com>
To: Paul Pluzhnikov <ppluzhnikov@google.com>
Cc: Tom Tromey <tromey@redhat.com>,
Pedro Alves <pedro@codesourcery.com>,
gdb-patches@sourceware.org
Subject: Re: [patch] Fix a crash when displaying variables from shared library.
Date: Mon, 23 Feb 2009 01:47:00 -0000 [thread overview]
Message-ID: <20090223010759.GA30997@adacore.com> (raw)
In-Reply-To: <8ac60eac0902181458g39dfbce9k63c3329528b0aad5@mail.gmail.com>
Hello Paul,
> (gdb) r
>
> Breakpoint 1, main () at noshlib.c:11
> 11 return foo();
> warning: Unable to display "a_static": No symbol "a_static" in current context.
> warning: Unable to display "a_local": No symbol "a_local" in current context.
> 1: a_global = 0
Good catch!
> I've updated the patch to only clear displays which actually are
> about to become dangling, and extended the test to check that displays
> that refer to the main executable are not affected.
You forgot to include a ChangeLog...
> + /* Can't re-parse the expression. Disable this display item. */
Minor style issue: You need to have two spaces after each period
(one in the middle, and one at the end, before the "*/").
> +/* Answer 1 if "d" uses "solib" (and will become dangling when "solib"
> + is unloaded), otherwise answer 0. */
If you don't mind, I think using "Return 1" instead of "Answer 1" would
be more consistent with the other descriptions.
Another minor style correction: In GDB, we refer to the function
parameters by using their names in ALL_CAPS, and without the quotes.
So, in your case, you would write:
/* Return 1 if D uses SOLIB (and will become dangling [...] */
> + if (d->block != NULL
> + && addr_low <= d->block->startaddr && d->block->endaddr <= addr_high)
> + return 1;
I suggest you use solib_address instead of doing the check yourself.
As mentioned by Daniel in another thread, shared libraries on SVR4
systems occupy a contiguous address block, but this is not the case
of DLLs where the data and text sections might be separate.
I verified that solib_address should handle the DLL case.
> + for (i = 0; i < d->exp->nelts; i++)
> + {
> + union exp_element *elts = d->exp->elts;
> + if (elts[i].opcode == OP_VAR_VALUE)
I'm afraid this isn't going to work for more complex structures...
The problem is that you might be reading an undefined field of
union exp_element. Imagine for instance that you have an expression
that looks like this: "foo->bar".
At one point, you'll encounter the following elements:
[i ] -> STRUCTOP_PTR
[i+1] -> A string
[i+2] -> STRUCTOP_PTR
Iterating over the expression, you'll ignore the element at index i,
and then check the opcode of the element at i+1, which is the wrong
field of the enum to access in this case...
I can't think of a way of doing what you're trying to do off the top
of my head. I'll have to think about it a little more. Perhaps others
will have suggestions... Or perhaps we'll have to attack the problem
with a different angle, I'm not very familiar with how "display"
expressions are handled...
> +gdb_test "run" "3: c_global = 43\\r\\nwarning: .*b_global.*\\r\\n1: a_global = 41" "after rebuild"
Can this be changed to use either one of the runto routines, or maybe
gdb_run_cmd if one of the above doesn't work in this case?
> +gdb_test "run" "6: a_static = 46\\r\\n4: main_global = 44\\r\\n.*"
Same here.
--
Joel
next prev parent reply other threads:[~2009-02-23 1:08 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-02-05 3:03 Paul Pluzhnikov
2009-02-06 21:38 ` Tom Tromey
2009-02-07 2:37 ` Paul Pluzhnikov
2009-02-11 1:46 ` Tom Tromey
2009-02-19 1:00 ` Paul Pluzhnikov
2009-02-19 7:52 ` Paul Pluzhnikov
2009-02-23 1:47 ` Joel Brobecker [this message]
2009-02-23 18:36 ` Paul Pluzhnikov
2009-03-03 2:31 ` Paul Pluzhnikov
2009-03-04 0:51 ` Tom Tromey
2009-03-04 19:26 ` Paul Pluzhnikov
2009-03-05 20:04 ` Joel Brobecker
2009-03-05 23:46 ` Paul Pluzhnikov
2009-03-06 3:06 ` Paul Pluzhnikov
2009-03-06 3:18 ` Paul Pluzhnikov
2009-03-06 17:48 ` Joel Brobecker
2009-03-06 18:31 ` Paul Pluzhnikov
2009-03-06 18:47 ` Joel Brobecker
2009-03-06 18:52 ` Paul Pluzhnikov
2009-03-06 22:06 ` Paul Pluzhnikov
2009-03-09 18:33 ` Joel Brobecker
2009-03-10 2:05 ` Paul Pluzhnikov
2009-03-10 14:31 ` Daniel Jacobowitz
2009-03-12 2:45 ` Paul Pluzhnikov
2009-03-20 20:32 ` Joel Brobecker
2009-03-20 20:53 ` Paul Pluzhnikov
2009-03-23 17:31 ` Joel Brobecker
2009-03-18 2:50 ` Pedro Alves
2009-03-18 3:24 ` [patch] Fix a crash when displaying variables from shared ?library Joel Brobecker
2009-03-18 4:06 ` Paul Pluzhnikov
2009-03-18 4:19 ` Pedro Alves
2009-03-18 6:54 ` Paul Pluzhnikov
2009-03-18 17:32 ` Pedro Alves
2009-02-06 21:53 ` [patch] Fix a crash when displaying variables from shared library Pedro Alves
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=20090223010759.GA30997@adacore.com \
--to=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=pedro@codesourcery.com \
--cc=ppluzhnikov@google.com \
--cc=tromey@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