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

[Joel Brobecker <brobecker@adacore.com>]

> 2009-03-04  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-02-04  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
>        * solib-display.exp: New file.
>        * solib-display-main.c: New file.
>        * solib-display-lib.c: New file.

All pre-approved after taking the minor comments below into account.

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

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

> +/* 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)

> Index: solib.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib.c,v
> retrieving revision 1.111
> diff -u -p -u -r1.111 solib.c
> --- solib.c	21 Feb 2009 16:14:49 -0000	1.111
> +++ solib.c	4 Mar 2009 19:17:56 -0000
> @@ -999,8 +999,13 @@ sharedlibrary_command (char *args, int f
>  void
>  no_shared_libraries (char *ignored, int from_tty)
>  {
> -  objfile_purge_solibs ();
> +  /* 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.  */

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

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.

What do you think?

-- 
Joel