RE: [PATCH v5] gdb: add a symbol* argument to get_return_value

["Six, Lancelot" <Lancelot.Six@amd.com>]

[AMD Official Use Only]

> > +  /* gdb.Value object of the function finished by this breakpoint.
> > +
> > +     nullptr if no debug information was available or return type was 
> > + VOID.  */
> >    PyObject *function_value;
>
> Actually I would mention the "nullptr if no debug..." part in func_symbol's documentation as well, since in the code we do check whether func_symbol is nullptr.
>
> LGTM with that fixed.
>
> And I have a question I didn't bring up before, because I don't want to increase the scope of this patch, but I am asking just for the sake of discussing.  Do you know why we wrapped return_type, and now func_symbol, in Python objects?  Any reason why we don't save the struct type / struct symbol pointer directly?  To protect against the underlying object disappearing while the FinishBreakpoint exists, maybe?

I guess it is out of consistency (but am really unsure). The return_value field is exposed as part of the python API (see bpfinishpy_get_returnvalue[1]), exposing the other fields is just a matter of adding the get function.  I also did ask this myself but since all fields of finish_breakpoint_object were not exposed I thought not exposing the symbol either was OK.  Having a quick look at the log, it seems that this did not change for quite some time (~10 years) so I am not sure we will get a definitive answer.

Lancelot

[1] https://www.sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/python/py-finishbreakpoint.c;h=03bd49345060fa48dec07298af3e4b4d50d775fa;hb=refs/heads/master#l63

>
> Simon
>