Re: [PATCH 2/5] gdb/python: new gdb.architecture_names function

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2021-10-22 13:34, Andrew Burgess wrote:
> * Simon Marchi <simon.marchi@polymtl.ca> [2021-10-22 09:02:04 -0400]:
> 
>> Hi Andrew,
>>
>> Sorry to reply only after you have merged this.  I just have a question
>> about the API.
> 
> Not a problem, better to get this sorted if something isn't right.
> 
>>
>> On 2021-10-13 17:59, Andrew Burgess wrote:
>>> Add a new function to the Python API, gdb.architecture_names().  This
>>> function returns a list containing all of the supported architecture
>>> names within the current build of GDB.
>>>
>>> The values returned in this list are all of the possible values that
>>> can be returned from gdb.Architecture.name().
>>
>> Did you consider having a `gdb.architectures()` function, that returns a
>> list of gdb.Architecture objects?  And then, if you want the names, you
>> use gdb.Architecture.name:
>>
>>     for arch in gdb.architectures():
>> 	print(arch.name)
> 
> Except I don't believe this would work.  A gdb.Architecture goes 1:1
> with a gdbarch object, and we can have multiple gdbarch objects for
> the same underlying bfd architecture.  For example, if two targets
> have the same bfd-architecture, but different target descriptions,
> you'll get different gdbarch objects, and different gdb.Architecture
> objects.
> 
> So, in your above code you have to at least filter for duplicates.
> 
> Then, as I understand it, gdbarch objects are only created "on
> demand", so in a multi-arch GDB, if I pass an x86 ELF, I don't
> believe a risc-v gdb.Architecture is ever created.
> 
> So, in your above code, you'll only see the names of architectures
> that the user has exposed to GDB.
> 
> That doesn't mean a gdb.architectures() method wouldn't be useful in
> some other situation, I just don't think it did what I wanted - tell
> me all the architectures this GDB knows about.

Ok, the above makes sense.

> Maybe what I should do is add an architecture_created event, then I
> could do:
> 
>   def do_something(arch):
>     print(arch.name)
> 
>   for arch in gdb.architectures():
>     do_something(arch)
> 
>   def handler(event):
>     do_something(event.architecture)
> 
>   gdb.events.architecture_created.connect(handler)

I don't think that's necessary.

>> Being able to get the gdb.Architecture objects instead of just the name
>> sounds more flexible / future-proof to me.
> 
> I don't think I'm worried that adding this API will be something we
> regret.  gdb.Architecture already has a .name method, so being able to
> ask for the set of all possible names doesn't seem unreasonable.  For
> that reason, I'd like to keep the existing method, even if you think
> that the above would be a better API...

I'm convinced that what you add is ok.  The list of architecture names
isn't the list of all possible gdbarches (and therefore
gdb.Architecture), they are different things.

>> `gdb.breakpoints()`, not `gdb.breakpoint_nums()`.  But there is perhaps
>> a technical reason why this doesn't work or isn't a good idea.
> 
> The only different I see is that the names can exist before the
> corresponding architectures, and we can have multiple architectures
> per name, it would be like if gdb.breakpoint_nums() only returned the
> numbers 1 -> 10 because we could only have that many breakpoints, but,
> then we could actually have multiple breakpoints for each
> number...... OK, this analogy got weird...
> 
> I've added the above additional APIs to my todo list, and I'll try to
> get them implemented.

I don't think it's necessary, I think what you have is ok.

Simon