[lldb-dev] [Reproducers] SBReproducer RFC

Tue Jan 8 12:57:42 PST 2019

Before I got around to coding this up I realized you can't take the address
of constructors in C++, so the function address won't work as an
identifier.

On Tue, Jan 8, 2019 at 9:28 AM Jonas Devlieghere <jonas at devlieghere.com>
wrote:

> On Tue, Jan 8, 2019 at 8:27 AM Frédéric Riss <friss at apple.com> wrote:
>
>>
>>
>> > On Jan 8, 2019, at 1:25 AM, Pavel Labath <pavel at labath.sk> wrote:
>> >
>> > On 07/01/2019 22:45, Frédéric Riss wrote:
>> >>> On Jan 7, 2019, at 11:31 AM, Pavel Labath via lldb-dev <
>> lldb-dev at lists.llvm.org <mailto:lldb-dev at lists.llvm.org>> wrote:
>> >>>
>> >>> On 07/01/2019 19:26, Jonas Devlieghere wrote:
>> >>>> On Mon, Jan 7, 2019 at 1:40 AM Pavel Labath <pavel at labath.sk
>> <mailto:pavel at labath.sk><mailto:pavel at labath.sk>> wrote:
>> >>>>    I've been thinking about how could this be done better, and the
>> best
>> >>>>    (though not ideal) way I came up with is using the functions
>> address as
>> >>>>    the key. That's guaranteed to be unique everywhere. Of course, you
>> >>>>    cannot serialize that to a file, but since you already have a
>> central
>> >>>>    place where you list all intercepted functions (to register their
>> >>>>    replayers), that place can be also used to assign unique integer
>> IDs to
>> >>>>    these functions. So then the idea would be that the SB_RECORD
>> macro
>> >>>>    takes the address of the current function, that gets converted to
>> an ID
>> >>>>    in the lookup table, and the ID gets serialized.
>> >>>> It sound like you would generate the indices at run-time. How would
>> that work with regards to the the reverse mapping?
>> >>> In the current implementation, SBReplayer::Init contains a list of
>> all intercepted methods, right? Each of the SB_REGISTER calls takes two
>> arguments: The method name, and the replay implementation.
>> >>>
>> >>> I would change that so that this macro takes three arguments:
>> >>> - the function address (the "runtime" ID)
>> >>> - an integer (the "serialized" ID)
>> >>> - the replay implementation
>> >>>
>> >>> This creates a link between the function address and the serialized
>> ID. So when, during capture, a method calls SB_RECORD_ENTRY and passes in
>> the function address, that address can be looked up and translated to an ID
>> for serialization.
>> >>>
>> >>> The only thing that would need to be changed is to have
>> SBReplayer::Init execute during record too (which probably means it
>> shouldn't be called SBReplayer, but whatever..), so that the ID mapping is
>> also available when capturing.
>> >>>
>> >>> Does that make sense?
>> >> I think I understand what you’re explaining, and the mapping side of
>> things makes sense. But I’m concerned about the size and complexity of the
>> SB_RECORD macro that will need to be written. IIUC, those would need to
>> take the address of the current function and the prototype, which is a lot
>> of cumbersome text to type. It seems like having a specialized tool to
>> generate those would be nice, but once you have a tool you also don’t need
>> all this complexity, do you?
>> >> Fred
>> >
>> > Yes, if the tool generates the IDs for you and checks that the macro
>> invocations are correct, then you don't need the function prototype.
>> However, that tool also doesn't come for free: Somebody has to write it,
>> and it adds complexity in the form of an extra step in the build process.
>>
>> Definitely agreed, the complexity has to be somewhere.
>>
>> > My point is that this extended macro could provide all the
>> error-checking benefits of this tool. It's a tradeoff, of course, and the
>> cost here is a more complex macro invocation. I think the choice here is
>> mostly down to personal preference of whoever implements this. However, if
>> I was implementing this, I'd go for an extended macro, because I don't find
>> the extra macro complexity to be too much. For example, this should be the
>> macro invocation for SBData::SetDataFromDoubleArray:
>> >
>> > SB_RECORD(bool, SBData, SetDataFromDoubleArray, (double *, size_t),
>> >    array, array_len);
>>
>> Yeah, this doesn’t seem so bad. For some reason I imagined it much more
>> verbose. Note that a verification tool that checks that every SB method is
>> instrumented correctly would still be nice (but it can come as a follow-up).
>>
>
> It sounds like this should work but we should try it out to be sure. I'll
> rework the prototype to use the function address and update the thread with
> my findings. I also like the idea of using templates to generate the
> parsers so I'll try that as well.
>
>
>> > It's a bit long, but it's not that hard to type, and all of this
>> information should be present on the previous line, where
>> SBData::SetDataFromDoubleArray is defined (I deliberately made the macro
>> argument order match the function definition syntax).
>> >
>> > And this approach can be further tweaked. For instance, if we're
>> willing to take the hit of having "weird" function definitions, then we can
>> avoid the repetition altogether, and make the macro define the function too:
>> >
>> > SB_METHOD2(bool, SBData, SetDataFromDoubleArray, double *, array,
>> >    size_t, array_len, {
>> >  // Method body
>> > })
>>
>> I personally don’t like this.
>>
>> Fred
>>
>> > This would also enable you to automatically capture method return value
>> for the "object" results.
>> >
>> > pl
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20190108/18e92d95/attachment.html>