Re: [PATCH] gdb: trivial segfault fix in tui

[Doug Evans <dje@google.com>]

On Tue, Aug 14, 2012 at 4:03 PM, Hal Ashburner <hal@ashburner.info> wrote:
> Dear Hafiz,
>
> Thank you for letting me know this.
> I've added a ChangeLog entry to the patch. I hope this is done the correct way.
>
> By way of trivia, I fixed this bug because I hit it more than once.
>
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 81c03ee..e1a080f 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +2012-08-15  Hal Ashburner <hal@ashburner.info>
> +
> +   * tui/tui-source.c: Check for null pointer to prevent segfault.
> +
>  2012-08-10  Sergio Durigan Junior  <sergiodj@redhat.com>
>
>     * linespec.c (find_methods): Remove unused variables `i1' and
> diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
> index 9ba9b1d..0c94aed 100644
> --- a/gdb/tui/tui-source.c
> +++ b/gdb/tui/tui-source.c
> @@ -334,11 +334,13 @@ tui_show_symtab_source (struct gdbarch *gdbarch,
> struct symtab *s,
>  int
>  tui_source_is_displayed (char *fname)
>  {
> -  return (TUI_SRC_WIN->generic.content_in_use
> -     && (filename_cmp (((struct tui_win_element *)
> -                (tui_locator_win_info_ptr ())->
> -                content[0])->which_element.locator.file_name,
> -               fname) == 0));
> +  if (tui_locator_win_info_ptr()->content)
> +      return (TUI_SRC_WIN->generic.content_in_use
> +          && (filename_cmp (((struct tui_win_element *)
> +                     (tui_locator_win_info_ptr ())->
> +                     content[0])->which_element.locator.file_name,
> +                    fname) == 0));
> +  return 0;
>  }

It seems tui is quite fragile with respect to window resizes.
I was able to get a segv here for similar reasons, but I didn't write
down the steps I did to trigger it. :-(

tui-source.c:
                        element->which_element.source.is_exec_point =
                          (filename_cmp (((struct tui_win_element *)

locator->content[0])->which_element.locator.file_name,
                                         s->filename) == 0

tui is also a bit, umm, weird.

tui_free_window has this:

      generic_win = tui_locator_win_info_ptr ();
      if (generic_win != (struct tui_gen_win_info *) NULL)
        {
          tui_delete_win (generic_win->handle);
          generic_win->handle = (WINDOW *) NULL;
        }

but tui_locator_win_info_ptr will never return NULL.

struct tui_gen_win_info *
tui_locator_win_info_ptr (void)
{
  return &_locator;
}

Blech!

In the end I suspect there's a better fix, but I don't know tui well enough.
I'd wait to see if someone else does.

Also, I'd add a comment explaining *why* the test for content != NULL,
and I'd rewrite it as:

  return (TUI_SRC_WIN->generic.content_in_use
              && tui_locator_win_info_ptr()->content != NULL
              && ...);

The style is more consistent this way.