Re: Fwd: [patch]: Fix crash in objc and breakpoints

[Pedro Alves <pedro at codesourcery dot com>]

On Thursday 18 February 2010 19:32:11, Kai Tietz wrote:

>        * source.c (line_info): Initialize pspace by default
>        current_program_space.
>        * frame.c (find_frame_sal): Likewise.
>        * linespec.c (decode_line_2): Likewise.
>        (decode_objc): Likewise.
> 
> Ok for apply?

Sorry, not yet.

The patch is mangled and I can't apply it as is,
but I'll try to make an effort to read it anyway.

Could you please, please also use (cvs) diff's -p switch
for future patches?  It makes diffs so much more readable.
Thanks.


> 
> Index: src/gdb/frame.c
> ===================================================================
> --- src.orig/gdb/frame.c        2010-01-29 16:28:43.000000000 +0100
> +++ src/gdb/frame.c     2010-02-18 10:49:42.745803800 +0100
> @@ -1857,6 +1857,8 @@
>           the call site is.  Do not pretend to.  This is jarring, but
>           we can't do much better.  */
>        sal->pc = get_frame_pc (frame);
> +      /* Initialize pspace by default.  */
> +      sal->pspace = current_program_space;
> 

Did you try frame->pspace?

>       return;
>     }
> Index: src/gdb/linespec.c
> ===================================================================
> --- src.orig/gdb/linespec.c     2010-02-18 10:41:31.000000000 +0100
> +++ src/gdb/linespec.c  2010-02-18 10:52:50.980178800 +0100
> @@ -513,7 +513,9 @@
>   while (i < nelts)
>     {
>       init_sal (&return_values.sals[i]);       /* Initialize to zeroes.
> */
> +      return_values.sals[i].pspace = current_program_space;
>       init_sal (&values.sals[i]);
> +      values.sals[i].pspace = current_program_space;
>       if (sym_arr[i] && SYMBOL_CLASS (sym_arr[i]) == LOC_BLOCK)
>        values.sals[i] = find_function_start_sal (sym_arr[i],
> funfirstline);
>       i++;

Sorry, I don't like these workarounds.  Why was this needed? 
There must be a code path that didn't set the pspace on a
valid sal.  What was it?

I think you're patching this:

 i = 0;
  while (i < nelts)
    {
      init_sal (&return_values.sals[i]);	/* Initialize to zeroes.  */
      init_sal (&values.sals[i]);
      if (sym_arr[i] && SYMBOL_CLASS (sym_arr[i]) == LOC_BLOCK)
	values.sals[i] = find_function_start_sal (sym_arr[i], funfirstline);
      i++;
    }

and find_function_start_sal should be returning a sal with
the correct pspace, so this must be about the sals that
aren't LOC_BLOCK?  What are those?  Below there's this

 while (i < nelts)
        {
          if (sym_arr[i] && SYMBOL_CLASS (sym_arr[i]) == LOC_BLOCK)
            {
...
            }
          else
            printf_unfiltered (_("?HERE\n"));
          i++;

Are we hitting this?  Sounds like something similar to PR10966.  Does
the workaround that has been applied on the branch for this PR happen
to fix this?

> @@ -1206,6 +1208,7 @@
>                                                   &current_target);
> 
>          init_sal (&values.sals[0]);
> +         values.sals[0].pspace = current_program_space;
>          values.sals[0].pc = pc;
>        }
>       return values;



> Index: src/gdb/source.c
> ===================================================================
> --- src.orig/gdb/source.c       2010-01-12 16:54:43.000000000 +0100
> +++ src/gdb/source.c    2010-02-18 10:46:36.183303800 +0100
> @@ -1467,6 +1467,7 @@
>   int i;
> 
>   init_sal (&sal);             /* initialize to zeroes */
> +  sal.pspace = current_program_space; /* initialize as default.  */

Likewise.  Isn't this a problem with the `if (arg == 0)' branch below
only?  It looks like it.

> 
>   if (arg == 0)
>     {
> 
> 

Do you have a testcase (doesn't have to be in .exp form, just
something I could try) for these issues yet?  You seem to
be covering different problems with the same patch.

-- 
Pedro Alves