This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Fix a crash when displaying variables from shared library.


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]