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.


On Thu, Mar 5, 2009 at 12:04 PM, Joel Brobecker <brobecker@adacore.com> wrote:

>> +  if (d->block != NULL &&
>> +      solib_address (d->block->startaddr) == solib->so_name)
>
> Minor style issue: The "&&" should be at the start of the next line.

Sorry 'bout that.

>
>> +       gdb_assert (elts[i + 3].opcode == OP_VAR_VALUE);
>
> Is this really necessary? I don't believe we have this sort
> of assertion anywhere else.

Ok, removed.

>> +/* display_chain items point to blocks and expressions. Some expressions in
>
> One space is missing after the period (I feel really sorry about
> perstering anyone about this, but this is the GCS)

Sorry about missing this one ...

>> +  /* The order of the two routines below is important: clear_solib
>> +     will notify observers, and at least clear_dangling_display_expressions
>> +     observer needs access to objfiles associated with solibs being
>> +     cleared.  */
>
> I'm relunctant to have comments mention the name of a routine as
> an example. I think we can get away from it by staying general:
>
>   /* The order of the two routines below is important: clear_solib
>      notifies the solib_unloaded observers, and some of these observes
>      might need access to their associated objfiles.  Therefore,
>      we cannot purge the solibs' objfiles before clear_solib has
>      been called.  */

Sounds good, comment updated.

>> +if ![runto_main] then {
>> +    fail "Can't run to main (2)"
>> +    return 0
>> +}
>> +
>> +gdb_test "stepi" "3: c_global = 43\\r\\n2: b_global = 42\\r\\n1: a_global = 41" "after rerun"
>
> I am guessing that the "stepi" test is really there because
> you couldn't get the runto_main output, and therefore couldn't
> verify it.

Correct.

> I suggest a different approach:
>
>  | # Start the program, we should land in the program main procedure
>  | if { [gdb_start_cmd] < 0 } {
>  |     fail "Can't run to main"
>  |     return -1
>  | }
>  |
>  | gdb_test "" \
>  |          "first \\(\\) at .*first.adb.*" \
>  |          "start first"
>
> The second gdb_test should allow you to verify that the debugger
> displays your variables correctly.

Looks good.

Attached is the patch I just committed.

Thanks,
-- 
Paul Pluzhnikov

ChangeLog:
2009-03-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* printcmd.c (do_one_display): Reparse exp_string.
	(display_uses_solib_p): New function.
	(clear_dangling_display_expressions): New function.
	(_initialize_printcmd): Add observer.
	* solib.c (no_shared_libraries): Swap order of calls to
	clear_solib and objfile_purge_solibs.
	
testsuite/ChangeLog:
2009-03-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* solib-display.exp: New file.
	* solib-display-main.c: New file.
	* solib-display-lib.c: New file.

Attachment: gdb-display-crash-20090305.txt
Description: Text document


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