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

[Kai Tietz <ktietz70 at googlemail dot 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