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

[Kai Tietz <ktietz70@googlemail.com>]

Hello Pedro,

2010/2/22 Pedro Alves <pedro@codesourcery.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.
>
Ok, I use in general quilt for patches, but of course I can sent them
in future by using cvs -p for diff.

>>
>> 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?

Yes, I did. But it hadn't solved the issue.

>>       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?

The patch in PR/10966 looks similar but didn't solved the issue, For
some of those place I added a default initialization of sal's pspace
are maybe superflous, but I saw the issue just remaining for obj-c.

I tested that the necessary patches to solve my issue are in frame.c
(find_frame_sal) and linespec.c (decode_line_2), The other seems to be
not really necessary.

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

Sadly the test-scenario for this issue is a bit big. I'll try to make
a testcase for it, but AFAI had seen it happens for me with Obj-C and
shared object files. I hope it isn't a Windows specific thing, but I
wouldn't assume so.

Kai


-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination