[PATCH] Lookup the JIT descriptor symbol first to avoid finding the PLT entry of the breakpoint in the wrong object file.

[PATCH] Lookup the JIT descriptor symbol first to avoid finding the PLT entry of the breakpoint in the wrong object file.

[Yichao Yu]

Fix 20633
---
 gdb/jit.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 158d6d8..d6eb800 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -1051,18 +1051,21 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
   if (ps_data->objfile == NULL)
     {
       /* Lookup the registration symbol.  If it is missing, then we
-	 assume we are not attached to a JIT.  */
-      reg_symbol = lookup_minimal_symbol_and_objfile (jit_break_name);
-      if (reg_symbol.minsym == NULL
-	  || BMSYMBOL_VALUE_ADDRESS (reg_symbol) == 0)
-	return 1;
+	 assume we are not attached to a JIT.
+	 Lookup the descriptor first since looking up the breakpoint might
+	 return a PLT entry in the wrong file.  */
 
-      desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL,
-					   reg_symbol.objfile);
+      desc_symbol = lookup_minimal_symbol_and_objfile (jit_descriptor_name);
       if (desc_symbol.minsym == NULL
 	  || BMSYMBOL_VALUE_ADDRESS (desc_symbol) == 0)
 	return 1;
 
+      reg_symbol = lookup_minimal_symbol (jit_break_name, NULL,
+					  desc_symbol.objfile);
+      if (reg_symbol.minsym == NULL
+	  || BMSYMBOL_VALUE_ADDRESS (reg_symbol) == 0)
+	return 1;
+
       objf_data = get_jit_objfile_data (reg_symbol.objfile);
       objf_data->register_code = reg_symbol.minsym;
       objf_data->descriptor = desc_symbol.minsym;
-- 
2.10.2

Re: [PATCH] Lookup the JIT descriptor symbol first to avoid finding the PLT entry of the breakpoint in the wrong object file.

[Yichao Yu]

On Thu, Jan 26, 2017 at 3:24 PM, Yichao Yu <yyc1992@gmail.com> wrote:
> Fix 20633


..... I don't use git send-mail very often and missed the chance to
type more comments....

The link to the bug report is
https://sourceware.org/bugzilla/show_bug.cgi?id=20633 . I think
ideally the interface should find all the global and local symbols and
breakpoint on all of them since there isn't an agreement on who will
provide it AFAICT (and for example,a process can load multiple copies
of statically linked LLVM each providing a jit debugging interface).
However, I'm not really sure what's the right function to use for that
and it'll probably be a much bigger change so I went for this smaller
change that fixes the original issue.

Not sure how this should be tested....... =(

> ---
>  gdb/jit.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/jit.c b/gdb/jit.c
> index 158d6d8..d6eb800 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -1051,18 +1051,21 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
>    if (ps_data->objfile == NULL)
>      {
>        /* Lookup the registration symbol.  If it is missing, then we
> -        assume we are not attached to a JIT.  */
> -      reg_symbol = lookup_minimal_symbol_and_objfile (jit_break_name);
> -      if (reg_symbol.minsym == NULL
> -         || BMSYMBOL_VALUE_ADDRESS (reg_symbol) == 0)
> -       return 1;
> +        assume we are not attached to a JIT.
> +        Lookup the descriptor first since looking up the breakpoint might
> +        return a PLT entry in the wrong file.  */
>
> -      desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL,
> -                                          reg_symbol.objfile);
> +      desc_symbol = lookup_minimal_symbol_and_objfile (jit_descriptor_name);
>        if (desc_symbol.minsym == NULL
>           || BMSYMBOL_VALUE_ADDRESS (desc_symbol) == 0)
>         return 1;
>
> +      reg_symbol = lookup_minimal_symbol (jit_break_name, NULL,
> +                                         desc_symbol.objfile);
> +      if (reg_symbol.minsym == NULL
> +         || BMSYMBOL_VALUE_ADDRESS (reg_symbol) == 0)
> +       return 1;
> +
>        objf_data = get_jit_objfile_data (reg_symbol.objfile);
>        objf_data->register_code = reg_symbol.minsym;
>        objf_data->descriptor = desc_symbol.minsym;
> --
> 2.10.2
>

Re: [PATCH] Lookup the JIT descriptor symbol first to avoid finding the PLT entry of the breakpoint in the wrong object file.

[Yichao Yu]

Ping. Can anyone please review this? Or is there a better place to post this?

On Thu, Jan 26, 2017 at 3:31 PM, Yichao Yu <yyc1992@gmail.com> wrote:
> On Thu, Jan 26, 2017 at 3:24 PM, Yichao Yu <yyc1992@gmail.com> wrote:
>> Fix 20633
>
>
> ..... I don't use git send-mail very often and missed the chance to
> type more comments....
>
> The link to the bug report is
> https://sourceware.org/bugzilla/show_bug.cgi?id=20633 . I think
> ideally the interface should find all the global and local symbols and
> breakpoint on all of them since there isn't an agreement on who will
> provide it AFAICT (and for example,a process can load multiple copies
> of statically linked LLVM each providing a jit debugging interface).
> However, I'm not really sure what's the right function to use for that
> and it'll probably be a much bigger change so I went for this smaller
> change that fixes the original issue.
>
> Not sure how this should be tested....... =(
>
>> ---
>>  gdb/jit.c | 17 ++++++++++-------
>>  1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/gdb/jit.c b/gdb/jit.c
>> index 158d6d8..d6eb800 100644
>> --- a/gdb/jit.c
>> +++ b/gdb/jit.c
>> @@ -1051,18 +1051,21 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
>>    if (ps_data->objfile == NULL)
>>      {
>>        /* Lookup the registration symbol.  If it is missing, then we
>> -        assume we are not attached to a JIT.  */
>> -      reg_symbol = lookup_minimal_symbol_and_objfile (jit_break_name);
>> -      if (reg_symbol.minsym == NULL
>> -         || BMSYMBOL_VALUE_ADDRESS (reg_symbol) == 0)
>> -       return 1;
>> +        assume we are not attached to a JIT.
>> +        Lookup the descriptor first since looking up the breakpoint might
>> +        return a PLT entry in the wrong file.  */
>>
>> -      desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL,
>> -                                          reg_symbol.objfile);
>> +      desc_symbol = lookup_minimal_symbol_and_objfile (jit_descriptor_name);
>>        if (desc_symbol.minsym == NULL
>>           || BMSYMBOL_VALUE_ADDRESS (desc_symbol) == 0)
>>         return 1;
>>
>> +      reg_symbol = lookup_minimal_symbol (jit_break_name, NULL,
>> +                                         desc_symbol.objfile);
>> +      if (reg_symbol.minsym == NULL
>> +         || BMSYMBOL_VALUE_ADDRESS (reg_symbol) == 0)
>> +       return 1;
>> +
>>        objf_data = get_jit_objfile_data (reg_symbol.objfile);
>>        objf_data->register_code = reg_symbol.minsym;
>>        objf_data->descriptor = desc_symbol.minsym;
>> --
>> 2.10.2
>>

Re: [PATCH] Lookup the JIT descriptor symbol first to avoid finding the PLT entry of the breakpoint in the wrong object file.

[Yichao Yu]

Ping again.

On Sun, Feb 5, 2017 at 7:46 PM, Yichao Yu <yyc1992@gmail.com> wrote:
> Ping. Can anyone please review this? Or is there a better place to post this?
>
> On Thu, Jan 26, 2017 at 3:31 PM, Yichao Yu <yyc1992@gmail.com> wrote:
>> On Thu, Jan 26, 2017 at 3:24 PM, Yichao Yu <yyc1992@gmail.com> wrote:
>>> Fix 20633
>>
>>
>> ..... I don't use git send-mail very often and missed the chance to
>> type more comments....
>>
>> The link to the bug report is
>> https://sourceware.org/bugzilla/show_bug.cgi?id=20633 . I think
>> ideally the interface should find all the global and local symbols and
>> breakpoint on all of them since there isn't an agreement on who will
>> provide it AFAICT (and for example,a process can load multiple copies
>> of statically linked LLVM each providing a jit debugging interface).
>> However, I'm not really sure what's the right function to use for that
>> and it'll probably be a much bigger change so I went for this smaller
>> change that fixes the original issue.
>>
>> Not sure how this should be tested....... =(
>>
>>> ---
>>>  gdb/jit.c | 17 ++++++++++-------
>>>  1 file changed, 10 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/gdb/jit.c b/gdb/jit.c
>>> index 158d6d8..d6eb800 100644
>>> --- a/gdb/jit.c
>>> +++ b/gdb/jit.c
>>> @@ -1051,18 +1051,21 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
>>>    if (ps_data->objfile == NULL)
>>>      {
>>>        /* Lookup the registration symbol.  If it is missing, then we
>>> -        assume we are not attached to a JIT.  */
>>> -      reg_symbol = lookup_minimal_symbol_and_objfile (jit_break_name);
>>> -      if (reg_symbol.minsym == NULL
>>> -         || BMSYMBOL_VALUE_ADDRESS (reg_symbol) == 0)
>>> -       return 1;
>>> +        assume we are not attached to a JIT.
>>> +        Lookup the descriptor first since looking up the breakpoint might
>>> +        return a PLT entry in the wrong file.  */
>>>
>>> -      desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL,
>>> -                                          reg_symbol.objfile);
>>> +      desc_symbol = lookup_minimal_symbol_and_objfile (jit_descriptor_name);
>>>        if (desc_symbol.minsym == NULL
>>>           || BMSYMBOL_VALUE_ADDRESS (desc_symbol) == 0)
>>>         return 1;
>>>
>>> +      reg_symbol = lookup_minimal_symbol (jit_break_name, NULL,
>>> +                                         desc_symbol.objfile);
>>> +      if (reg_symbol.minsym == NULL
>>> +         || BMSYMBOL_VALUE_ADDRESS (reg_symbol) == 0)
>>> +       return 1;
>>> +
>>>        objf_data = get_jit_objfile_data (reg_symbol.objfile);
>>>        objf_data->register_code = reg_symbol.minsym;
>>>        objf_data->descriptor = desc_symbol.minsym;
>>> --
>>> 2.10.2
>>>

Re: [PATCH] Lookup the JIT descriptor symbol first to avoid finding the PLT entry of the breakpoint in the wrong object file.

[Yichao Yu]

And ping again.

On Fri, Feb 24, 2017 at 2:00 PM, Yichao Yu <yyc1992@gmail.com> wrote:
> Ping again.
>
> On Sun, Feb 5, 2017 at 7:46 PM, Yichao Yu <yyc1992@gmail.com> wrote:
>> Ping. Can anyone please review this? Or is there a better place to post this?
>>
>> On Thu, Jan 26, 2017 at 3:31 PM, Yichao Yu <yyc1992@gmail.com> wrote:
>>> On Thu, Jan 26, 2017 at 3:24 PM, Yichao Yu <yyc1992@gmail.com> wrote:
>>>> Fix 20633
>>>
>>>
>>> ..... I don't use git send-mail very often and missed the chance to
>>> type more comments....
>>>
>>> The link to the bug report is
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=20633 . I think
>>> ideally the interface should find all the global and local symbols and
>>> breakpoint on all of them since there isn't an agreement on who will
>>> provide it AFAICT (and for example,a process can load multiple copies
>>> of statically linked LLVM each providing a jit debugging interface).
>>> However, I'm not really sure what's the right function to use for that
>>> and it'll probably be a much bigger change so I went for this smaller
>>> change that fixes the original issue.
>>>
>>> Not sure how this should be tested....... =(
>>>
>>>> ---
>>>>  gdb/jit.c | 17 ++++++++++-------
>>>>  1 file changed, 10 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/gdb/jit.c b/gdb/jit.c
>>>> index 158d6d8..d6eb800 100644
>>>> --- a/gdb/jit.c
>>>> +++ b/gdb/jit.c
>>>> @@ -1051,18 +1051,21 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
>>>>    if (ps_data->objfile == NULL)
>>>>      {
>>>>        /* Lookup the registration symbol.  If it is missing, then we
>>>> -        assume we are not attached to a JIT.  */
>>>> -      reg_symbol = lookup_minimal_symbol_and_objfile (jit_break_name);
>>>> -      if (reg_symbol.minsym == NULL
>>>> -         || BMSYMBOL_VALUE_ADDRESS (reg_symbol) == 0)
>>>> -       return 1;
>>>> +        assume we are not attached to a JIT.
>>>> +        Lookup the descriptor first since looking up the breakpoint might
>>>> +        return a PLT entry in the wrong file.  */
>>>>
>>>> -      desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL,
>>>> -                                          reg_symbol.objfile);
>>>> +      desc_symbol = lookup_minimal_symbol_and_objfile (jit_descriptor_name);
>>>>        if (desc_symbol.minsym == NULL
>>>>           || BMSYMBOL_VALUE_ADDRESS (desc_symbol) == 0)
>>>>         return 1;
>>>>
>>>> +      reg_symbol = lookup_minimal_symbol (jit_break_name, NULL,
>>>> +                                         desc_symbol.objfile);
>>>> +      if (reg_symbol.minsym == NULL
>>>> +         || BMSYMBOL_VALUE_ADDRESS (reg_symbol) == 0)
>>>> +       return 1;
>>>> +
>>>>        objf_data = get_jit_objfile_data (reg_symbol.objfile);
>>>>        objf_data->register_code = reg_symbol.minsym;
>>>>        objf_data->descriptor = desc_symbol.minsym;
>>>> --
>>>> 2.10.2
>>>>