[lldb-dev] [Reproducers] SBReproducer RFC

[Frédéric Riss via lldb-dev]

> 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'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