Re: GDB 8.0 release/branching 2017-03-20 update

[Yao Qi <qiyaoltc@gmail.com>]

"Metzger, Markus T" <markus.t.metzger@intel.com> writes:

Hi, Markus,
Thanks for your input.

>>   1. BtraceInstruction.sal is not necessary.  It can be got via
>>   gdb.find_pc_line(BtraceInstruction.pc).
>
> I asked Tim to put this into the Instruction object in order to support
> future extensions.
>
> At the moment, we decode from live memory, and there, you're right
> that we can get the SAL from the PC.  If we wanted to support exec()
> or dlclose() in the future, the trace would refer to instructions that are
> no longer available in memory and GDB would not know about them.
>
> Having users get the SAL from the Instruction object gives us more
> freedom to model this.  We could, for example, decode from binary
> files and leave GDB to represent the current program state.  Not sure
> whether this would suffice - probably not.  But it sounded like a good
> idea to have users get such information via the Instruction object from
> the beginning.  That's also why the Instruction object has a data function
> to get the raw bytes of an instruction rather than having users read
> it from memory.
>
>
>>   2. BtraceInstruction has some attributes (pc, data, decoded, and size,)
>>   which are not btrace related.
>>   They should be moved to a new class gdb.Instruction and BtraceInstruction
>>   extends it.
>> 
>>   Instruction
>>   {
>>     pc, data, decode, size;
>>   };
>
> That makes sense if we're considering a Python API for GDB's disassembler.
> I would extend it to include SAL for the above reasons so users don't need
> to know from where they got the Instruction object.

IMO, SAL only makes sense to RecordInstruction.  I don't see why must put
SAL into Instruction.

>
> And we would extend it for record targets to add the speculative execution
> indication and a means for interleaving decode errors with decoded instructions.
>

Can we also extend Instruction for record target to add SAL there?  Then
it becomes,

  Instruction
  {
    pc, data, decode, size;
  };

  RecordInstruction : Instruction
  {
    sal;
    is_speculative;
  };

> I'm wondering how much of this discussion is really a question of sub-classing
> and how much of it is about documentation.  Would I even be able to tell the
> difference in my python scripts that use the API?

All the user visible stuff should be covered in this discussion.  We
need to mention that "gdb.RecordInstruction extends gdb.Instruction" in
the document, so that we don't have to document gdb.Instruction
attributes again in gdb.RecordInstruction.

>
> The duck typing argument makes sense to me and I'm not familiar enough
> with implementing Python to have an opinion on how this should be done.
> Do we have an expert on this in the GDB community?
>

The duck typing is about writing python code, but I raise my concerns in
this thread from the view of controlling the API *interface* for
different record methods by means of inheritance.  With or without
inheritance, the python script still can do duck typing, no problem, but
we will end up having completely different ways of organizing these
interface.  Suppose we have three record methods, "btrace", "full" and
"bar", in current approach, we need to document all the attributes for
these three methods, although some of them are in common.  Why don't we
put these common attributes into a base class, and only document the
base class *once*?

Further, with such base class, we can guarantee that the client python
scripts only access base class attributes are right even with the new
record method may be added in the future.  In current approach, we write
such python code "print(hex(i.pc), i.sal, i.decoded)" just because
BtraceInstruction and FullInstruction *happen* to have these
attributes.  What if a new record method "bar" have instruction trace
which doesn't have attribute "decoded"?

>
>>   3. Why does Record have an attribute ptid?
>>   Other record method may have process-wide record/trace.  I'd like to put
>>   common attributes in Record, and extend Record for each specific record
>>   method.
>> 
>>   Record
>>   {
>>     method, format;
>>     instruction_history;
>>   };
>
> Agreed.  The ptid seems to be an implementation detail.  Tim, would you be
> OK to remove it?
>
> I would further put the function-call history into the Record base-class.  It is
> merely another view that can be computed from the instruction-history.
> Record targets that don't support this view can return None.

I like this idea.

>
> Also the replay_position is common to all record targets that support reverse/
> replay.  And record targets that don't will always remain at the end of the
> recorded trace.  I don't see why a record target would not be able to provide
> it.  Currently, all record targets support it.
>
> If you and Tim agree with this, we won't need a btrace sub-class, anymore and
> this should be a simple renaming.

Anything can make these APIs more general are great!

-- 
Yao (齐尧)