Thread Specific Architectures And Python Unwinder API

Thread Specific Architectures And Python Unwinder API

[Andrew Burgess]

Hi Luis,

While working on something else I was looking at the Python Unwinder API
code, and I suspect that the thread-specific architectures support might
(currently) break the Python Unwinder support.

If it is, then I think the fix is pretty simple, but before I posted it,
I wondered if you could confirm that things are indeed, currently
broken.

Attached at the end of this email is a Python unwinder.  You'll need to
supply your own test program that makes use of sve/sme, and thus uses
thread-specific architectures.

What you'll need to do is:

  $ gdb -q test_file_that_uses_sve_sme
  Reading symbols from .... etc ...
  (gdb) source ./unwinder.py
  (gdb) break function_where_a_thread_specific_arch_will_be_in_use
  Breakpoint 1 at ... etc ...
  (gdb) run
  Starting program: ... etc ...

Now at this point, when you stop, you should see at least one instance
of the banner:
  
  ***********************************
  * Have executed the test unwinder *
  ***********************************

being printed, probably more.  As you step though the function you
should see more instances of the banner being printed.

To reveal the bug then it is important that when GDB stops in
function_where_a_thread_specific_arch_will_be_in_use, the per-thread
gdbarch that it creates _must_ be different from the inferior wide,
top-level gdbarch.

If you don't see the banner then my suspicion is correct, and the Python
Unwinder API was broken when the thread-specific architecture support
was added.

The problem (I think) is that the Python unwinder is registered on each
gdbarch as a result of the gdb::observers::architecture_changed.notify()
call in set_target_gdbarch (though Simon has recently moved, or is about
to move, this code, but not in a way that will fix this problem).  Thus,
the Python unwinder is only registered on the top-level (inferior wide)
gdbarch, not on each of the different per-thread architectures.

I have a patch here that removes the 'architecture_changed' observer,
and replaces it with a 'new_architecture' observer, which I propose to
move up into gdbarch_find_by_info.

Thanks,
Andrew

---

from gdb.unwinder import Unwinder, FrameId

class test_unwinder(Unwinder):
    def __init__(self):
        super().__init__("test_unwinder")

    def __call__(self,pending_frame):
        print("")
        print("***********************************")
        print("* Have executed the test unwinder *")
        print("***********************************")
        return None

gdb.unwinder.register_unwinder(None, test_unwinder(), replace=True)

Re: Thread Specific Architectures And Python Unwinder API

[Luis Machado]

Hi Andrew,

On 10/11/23 09:47, Andrew Burgess wrote:
> 
> Hi Luis,
> 
> While working on something else I was looking at the Python Unwinder API
> code, and I suspect that the thread-specific architectures support might
> (currently) break the Python Unwinder support.
> 
> If it is, then I think the fix is pretty simple, but before I posted it,
> I wondered if you could confirm that things are indeed, currently
> broken.
> 
> Attached at the end of this email is a Python unwinder.  You'll need to
> supply your own test program that makes use of sve/sme, and thus uses
> thread-specific architectures.
> 
> What you'll need to do is:
> 
>   $ gdb -q test_file_that_uses_sve_sme
>   Reading symbols from .... etc ...
>   (gdb) source ./unwinder.py
>   (gdb) break function_where_a_thread_specific_arch_will_be_in_use
>   Breakpoint 1 at ... etc ...
>   (gdb) run
>   Starting program: ... etc ...
> 
> Now at this point, when you stop, you should see at least one instance
> of the banner:
>   
>   ***********************************
>   * Have executed the test unwinder *
>   ***********************************
> 
> being printed, probably more.  As you step though the function you
> should see more instances of the banner being printed.
> 
> To reveal the bug then it is important that when GDB stops in
> function_where_a_thread_specific_arch_will_be_in_use, the per-thread
> gdbarch that it creates _must_ be different from the inferior wide,
> top-level gdbarch.
> 
> If you don't see the banner then my suspicion is correct, and the Python
> Unwinder API was broken when the thread-specific architecture support
> was added.
> 
> The problem (I think) is that the Python unwinder is registered on each
> gdbarch as a result of the gdb::observers::architecture_changed.notify()
> call in set_target_gdbarch (though Simon has recently moved, or is about
> to move, this code, but not in a way that will fix this problem).  Thus,
> the Python unwinder is only registered on the top-level (inferior wide)
> gdbarch, not on each of the different per-thread architectures.
> 
> I have a patch here that removes the 'architecture_changed' observer,
> and replaces it with a 'new_architecture' observer, which I propose to
> move up into gdbarch_find_by_info.
> 
> Thanks,
> Andrew
> 
> ---
> 
> from gdb.unwinder import Unwinder, FrameId
> 
> class test_unwinder(Unwinder):
>     def __init__(self):
>         super().__init__("test_unwinder")
> 
>     def __call__(self,pending_frame):
>         print("")
>         print("***********************************")
>         print("* Have executed the test unwinder *")
>         print("***********************************")
>         return None
> 
> gdb.unwinder.register_unwinder(None, test_unwinder(), replace=True)
> 

I'll give this a try and will let you know. But I suspect it might be broken indeed. Not all the gdb layers
support this concept of per-thread gdbarch.

Re: Thread Specific Architectures And Python Unwinder API

[Luis Machado]

On 10/11/23 11:06, Luis Machado wrote:
> Hi Andrew,
> 
> On 10/11/23 09:47, Andrew Burgess wrote:
>>
>> Hi Luis,
>>
>> While working on something else I was looking at the Python Unwinder API
>> code, and I suspect that the thread-specific architectures support might
>> (currently) break the Python Unwinder support.
>>
>> If it is, then I think the fix is pretty simple, but before I posted it,
>> I wondered if you could confirm that things are indeed, currently
>> broken.
>>
>> Attached at the end of this email is a Python unwinder.  You'll need to
>> supply your own test program that makes use of sve/sme, and thus uses
>> thread-specific architectures.
>>
>> What you'll need to do is:
>>
>>   $ gdb -q test_file_that_uses_sve_sme
>>   Reading symbols from .... etc ...
>>   (gdb) source ./unwinder.py
>>   (gdb) break function_where_a_thread_specific_arch_will_be_in_use
>>   Breakpoint 1 at ... etc ...
>>   (gdb) run
>>   Starting program: ... etc ...
>>
>> Now at this point, when you stop, you should see at least one instance
>> of the banner:
>>   
>>   ***********************************
>>   * Have executed the test unwinder *
>>   ***********************************
>>
>> being printed, probably more.  As you step though the function you
>> should see more instances of the banner being printed.
>>
>> To reveal the bug then it is important that when GDB stops in
>> function_where_a_thread_specific_arch_will_be_in_use, the per-thread
>> gdbarch that it creates _must_ be different from the inferior wide,
>> top-level gdbarch.
>>
>> If you don't see the banner then my suspicion is correct, and the Python
>> Unwinder API was broken when the thread-specific architecture support
>> was added.

I've confirmed I don't see the banner. So this seems to be broken.

Is the fix to set the python unwinder hook elsewhere? Or maybe save that
information when we're trying to pick a different gdbarch?

Re: Thread Specific Architectures And Python Unwinder API

[Andrew Burgess]

Luis Machado <luis.machado@arm.com> writes:

> On 10/11/23 11:06, Luis Machado wrote:
>> Hi Andrew,
>> 
>> On 10/11/23 09:47, Andrew Burgess wrote:
>>>
>>> Hi Luis,
>>>
>>> While working on something else I was looking at the Python Unwinder API
>>> code, and I suspect that the thread-specific architectures support might
>>> (currently) break the Python Unwinder support.
>>>
>>> If it is, then I think the fix is pretty simple, but before I posted it,
>>> I wondered if you could confirm that things are indeed, currently
>>> broken.
>>>
>>> Attached at the end of this email is a Python unwinder.  You'll need to
>>> supply your own test program that makes use of sve/sme, and thus uses
>>> thread-specific architectures.
>>>
>>> What you'll need to do is:
>>>
>>>   $ gdb -q test_file_that_uses_sve_sme
>>>   Reading symbols from .... etc ...
>>>   (gdb) source ./unwinder.py
>>>   (gdb) break function_where_a_thread_specific_arch_will_be_in_use
>>>   Breakpoint 1 at ... etc ...
>>>   (gdb) run
>>>   Starting program: ... etc ...
>>>
>>> Now at this point, when you stop, you should see at least one instance
>>> of the banner:
>>>   
>>>   ***********************************
>>>   * Have executed the test unwinder *
>>>   ***********************************
>>>
>>> being printed, probably more.  As you step though the function you
>>> should see more instances of the banner being printed.
>>>
>>> To reveal the bug then it is important that when GDB stops in
>>> function_where_a_thread_specific_arch_will_be_in_use, the per-thread
>>> gdbarch that it creates _must_ be different from the inferior wide,
>>> top-level gdbarch.
>>>
>>> If you don't see the banner then my suspicion is correct, and the Python
>>> Unwinder API was broken when the thread-specific architecture support
>>> was added.
>
> I've confirmed I don't see the banner. So this seems to be broken.
>
> Is the fix to set the python unwinder hook elsewhere? Or maybe save that
> information when we're trying to pick a different gdbarch?

I'll post my patch for this later today.  But I'm going to propose
changing the 'architecture_changed' observable into a 'new_architecture'
observable (which is called in a different place).  As such, every new
architecture will have the Python unwinder registered correctly.

I just need to update the commit message with some details of this issue
as justification for the change, and I'll post it -- it's a pretty small
change.  I'll CC you on the new thread.

Thanks,
Andrew

Re: Thread Specific Architectures And Python Unwinder API

[Luis Machado]

On 10/12/23 14:02, Andrew Burgess wrote:
> Luis Machado <luis.machado@arm.com> writes:
> 
>> On 10/11/23 11:06, Luis Machado wrote:
>>> Hi Andrew,
>>>
>>> On 10/11/23 09:47, Andrew Burgess wrote:
>>>>
>>>> Hi Luis,
>>>>
>>>> While working on something else I was looking at the Python Unwinder API
>>>> code, and I suspect that the thread-specific architectures support might
>>>> (currently) break the Python Unwinder support.
>>>>
>>>> If it is, then I think the fix is pretty simple, but before I posted it,
>>>> I wondered if you could confirm that things are indeed, currently
>>>> broken.
>>>>
>>>> Attached at the end of this email is a Python unwinder.  You'll need to
>>>> supply your own test program that makes use of sve/sme, and thus uses
>>>> thread-specific architectures.
>>>>
>>>> What you'll need to do is:
>>>>
>>>>   $ gdb -q test_file_that_uses_sve_sme
>>>>   Reading symbols from .... etc ...
>>>>   (gdb) source ./unwinder.py
>>>>   (gdb) break function_where_a_thread_specific_arch_will_be_in_use
>>>>   Breakpoint 1 at ... etc ...
>>>>   (gdb) run
>>>>   Starting program: ... etc ...
>>>>
>>>> Now at this point, when you stop, you should see at least one instance
>>>> of the banner:
>>>>   
>>>>   ***********************************
>>>>   * Have executed the test unwinder *
>>>>   ***********************************
>>>>
>>>> being printed, probably more.  As you step though the function you
>>>> should see more instances of the banner being printed.
>>>>
>>>> To reveal the bug then it is important that when GDB stops in
>>>> function_where_a_thread_specific_arch_will_be_in_use, the per-thread
>>>> gdbarch that it creates _must_ be different from the inferior wide,
>>>> top-level gdbarch.
>>>>
>>>> If you don't see the banner then my suspicion is correct, and the Python
>>>> Unwinder API was broken when the thread-specific architecture support
>>>> was added.
>>
>> I've confirmed I don't see the banner. So this seems to be broken.
>>
>> Is the fix to set the python unwinder hook elsewhere? Or maybe save that
>> information when we're trying to pick a different gdbarch?
> 
> I'll post my patch for this later today.  But I'm going to propose
> changing the 'architecture_changed' observable into a 'new_architecture'
> observable (which is called in a different place).  As such, every new
> architecture will have the Python unwinder registered correctly.
> 
> I just need to update the commit message with some details of this issue
> as justification for the change, and I'll post it -- it's a pretty small
> change.  I'll CC you on the new thread.

Great. Thanks for the heads-up. Let me know if you need some more testing.

Re: Thread Specific Architectures And Python Unwinder API

[Andrew Burgess]

Luis Machado <luis.machado@arm.com> writes:

> On 10/12/23 14:02, Andrew Burgess wrote:
>> Luis Machado <luis.machado@arm.com> writes:
>> 
>>> On 10/11/23 11:06, Luis Machado wrote:
>>>> Hi Andrew,
>>>>
>>>> On 10/11/23 09:47, Andrew Burgess wrote:
>>>>>
>>>>> Hi Luis,
>>>>>
>>>>> While working on something else I was looking at the Python Unwinder API
>>>>> code, and I suspect that the thread-specific architectures support might
>>>>> (currently) break the Python Unwinder support.
>>>>>
>>>>> If it is, then I think the fix is pretty simple, but before I posted it,
>>>>> I wondered if you could confirm that things are indeed, currently
>>>>> broken.
>>>>>
>>>>> Attached at the end of this email is a Python unwinder.  You'll need to
>>>>> supply your own test program that makes use of sve/sme, and thus uses
>>>>> thread-specific architectures.
>>>>>
>>>>> What you'll need to do is:
>>>>>
>>>>>   $ gdb -q test_file_that_uses_sve_sme
>>>>>   Reading symbols from .... etc ...
>>>>>   (gdb) source ./unwinder.py
>>>>>   (gdb) break function_where_a_thread_specific_arch_will_be_in_use
>>>>>   Breakpoint 1 at ... etc ...
>>>>>   (gdb) run
>>>>>   Starting program: ... etc ...
>>>>>
>>>>> Now at this point, when you stop, you should see at least one instance
>>>>> of the banner:
>>>>>   
>>>>>   ***********************************
>>>>>   * Have executed the test unwinder *
>>>>>   ***********************************
>>>>>
>>>>> being printed, probably more.  As you step though the function you
>>>>> should see more instances of the banner being printed.
>>>>>
>>>>> To reveal the bug then it is important that when GDB stops in
>>>>> function_where_a_thread_specific_arch_will_be_in_use, the per-thread
>>>>> gdbarch that it creates _must_ be different from the inferior wide,
>>>>> top-level gdbarch.
>>>>>
>>>>> If you don't see the banner then my suspicion is correct, and the Python
>>>>> Unwinder API was broken when the thread-specific architecture support
>>>>> was added.
>>>
>>> I've confirmed I don't see the banner. So this seems to be broken.
>>>
>>> Is the fix to set the python unwinder hook elsewhere? Or maybe save that
>>> information when we're trying to pick a different gdbarch?
>> 
>> I'll post my patch for this later today.  But I'm going to propose
>> changing the 'architecture_changed' observable into a 'new_architecture'
>> observable (which is called in a different place).  As such, every new
>> architecture will have the Python unwinder registered correctly.
>> 
>> I just need to update the commit message with some details of this issue
>> as justification for the change, and I'll post it -- it's a pretty small
>> change.  I'll CC you on the new thread.
>
> Great. Thanks for the heads-up. Let me know if you need some more testing.

If you're keen, the series is now posted here:

  https://inbox.sourceware.org/gdb-patches/f5efeaed8c7150294024f5d2f6d98b748fd6bf56.1697120493.git.aburgess@redhat.com/T/#u

If you apply this, and then repeat the previous experiment (loading the
dummy Python unwinder), you should now see the banner being printed.

Thanks,
Andrew