Re: [PATCH 0/1] Integrate GNU poke in GDB

["Jose E. Marchesi" <jose.marchesi@oracle.com>]

Hi Simon.
Thanks for the feedback.

>> A few notes:
>> 
>> - The poke support is optional, and built if specified with
>>   --enable-poke at configure time.
>
> If I look at this reference [1]:
>
>      --enable-*/--disable-* arguments
>
> 	The arguments starting with --enable- prefix are usually used to
> 	enable features of the program. They usually add or remove
> 	dependencies only if they are needed for that particular feature
> 	being enabled or disabled.
>
>     --with-*/--without-* arguments
>
> 	The arguments starting with --with- prefix are usually used to
> 	add or remove dependencies on external projects. These might add
> 	or remove features from the project.
>
> I don't find this super clear, as we could always see it both ways.  I
> want to enable the support for running Python code, so I use
> --enable-python, which has the side-effect of pulling libpython.  Or I
> want to make my project depend on libpython, so I use --with-python,
> which has the side-effect of adding the "can run Python code feature" in
> my project.
>
> But most of our dependencies on external libs are expressed with
> "--with", so I'd go with "--with-poke".

Ok, I will change it :)

>> 
>> - Eventually we will probably want to ship some prewritten Poke
>>   code in a pickle gdb.pk.  Would $pkddatadir/poke/ be a good
>>   location for Poke code distributed with GDB?
>
> So, /usr/share/poke?
>
> Would gdb.pk contain some poke code that gdb would use itself, or is the
> goal to make that poke code available to other libpoke users (like the
> poke cli)?
>
> If it's code that is only meant to be used by gdb, it would make sense
> to have it in GDB's own data dir (/usr/share/gdb/poke).  A bit like gdb
> has some Python code it uses for itself, in /usr/share/gdb/python.

Yeah that makes sense, /usr/share/poke for the first case, and
/usr/share/gdb/poke in the second case.

>> 
>> And a few questions:
>> 
>> - Where am I supposed to cleanup and shut down the incremental
>>   compiler when GDB exits?  I looked but I cannot find a
>>   counterpart to _initialize_FOO, like _finalize_FOO.
>
> See make_final_cleanup.  Or, you can use an std::unique_ptr
> specialization that calls your finalize function on destruction.  If
> that object is global (or static within a function), it will be
> destructed when the program (GDB) exits.  There was a discussion about
> exactly this but for debuginfod recently:
>
> https://sourceware.org/pipermail/gdb-patches/2021-May/178578.html

That unique_ptr approach should work.  Will take a look to that
discussion and DTRT.

>> - There are many global parameters that can be configured in the
>>   poke incremental compiler: the numeration base used when
>>   printing values, the global endianness, a cut-off value for how
>>   many array elements are printed, and the like.  Reasonable
>>   defaults for them are set in `start_poke', but I would like the
>>   GDB user to be able to change these settings.  I could add a
>>   `poke-set SETTING [VALUE]' command, but I was wondering whether
>>   there is a better way, like a global register of settings
>>   already in GDB?
>
> Without more knowledge of how this all works, I'd say that
>
>    set poke PARAM VALUE
>    show poke PARAM
>
> would be more GDB-ish.

Yes that's what I had in mind :)

> Is there a way for GDB to list all poke's parameters at startup, so it
> could register them all as a "set/show" command in GDB?

Yes, that is certainly possible.

> As a result, the user could do "set poke <TAB><TAB>" to see all
> possible parameters, which I would find really nice.

Definitely!

> Although if you think we might ever need to add some GDB parameters that
> are not poke parameters, but parameters about how GDB uses/integrates
> poke, then there is a chance of clash in the "set poke" namespace.  In
> that case, we could put all poke's parameters under
>
>     set poke parameter PARAM VALUE
>     show poke parameter PARAM
>
> This way, we could have distinct "set poke parameter foo ..." and "set
> poke foo ...".

Where is the interface to add the settings?

>> - I am using target_{read,write}_raw_memory to access the
>>   target's memory.  What is the difference between the raw and
>>   non-raw accessors?
>
> From target_read_raw_memory's doc:
>
> /* Like target_read_memory, but specify explicitly that this is a read
>    from the target's raw memory.  That is, this read bypasses the
>    dcache, breakpoint shadowing, etc.  */
>
>
> So, it bypasses a cache in GDB (not sure why that's important).  But
> most importantly, the non-raw version will hide the software breakpoint
> instructions, it will replace them with the original memory contents,
> whereas the raw version will not.
>
> Let's says that the user decodes some memory where a breakpoint is, using
> target_read_raw_memory will return the memory contents with the
> breakpoint instruction.  I think that most of the time, the user will
> actually want to decode the original contents, so using
> target_read_memory would be better.
>
> And if you write memory with target_write_raw_memory, I guess you can
> end up overwriting a breakpoint, so that's not good.  So I'd probably
> use target_write_memory.

Ok, I will change to use the non-raw versions.

>> - How can I demangle C++ identifiers?  And how can I detect when
>>   a given type is a C++ one that needs demangling?
>
> I think that we usually rely on the language of the compilation unit, as
> given by DWARF, to know that a symbol has a C++ mangled name and needs
> demangling.
>
> But otherwise, see the gdb_demangle function.

Will do.
Thanks!