[lldb-dev] [Reproducers] SBReproducer RFC

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

> On Jan 7, 2019, at 11:31 AM, Pavel Labath via lldb-dev <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 <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

>>    The part that bugs me about this is that taking the address of an
>>    overloaded function is extremely tedious (you have to write something
>>    like static_cast<function_prototype>(&SBFoo::Bar)). That would mean all
>>    of these things would have to be passed to the RECORD macro. OTOH, the
>>    upshot of this would be that the macro would now have sufficient
>>    information to perform pretty decent error checking on its invocation.
>>    Another nice about this could be that once you already have a prototype
>>    and an address of the function, it should be possible (with sufficient
>>    template-fu) to synthesize replay code for the function automatically,
>>    at least in the simple cases, which would avoid the repetitiveness of
>>    the current replay code. Together, this might obviate the need for any
>>    clang plugins or other funny build steps.
>> See my previous question. I see how the signature would help with decoding but still missing how you'd get the mapping.
>>    The second thing I noticed is the usage of pointers for identifying
>>    object. A pointer is good for that but only while the object it points
>>    to is alive. Once the object is gone, the pointer can (and most likely
>>    will) be reused. So, it sounds to me like you also need to track the
>>    lifetime of these objects. That may be as simple as intercepting
>>    constructor/destructor calls, but I haven't seen anything like that yet
>>    (though I haven't looked at all details of the patch).
>> This shouldn't be a problem. When a new object is created it will be recorded in the table with a new identifier.
> Ok, sounds good.
> 
>>    Tying into that is the recording of return values. It looks like the
>>    current RECORD_RETURN macro will record the address of the temporary
>>    object in the frame of the current function. However, that address will
>>    become invalid as soon as the function returns as the result object
>>    will
>>    be copied into a location specified by the caller as a part of the
>>    return processing. Are you handling this in any way?
>> We capture the temporary and the call to the copy-assignment constructor. This is not super efficient but it's the best we can do.
> 
> Ok, cool. I must have missed that part in the code.
> 
>>    The final thing, which I noticed is the lack of any sign of threading
>>    support. I'm not too worried about that, as that sounds like something
>>    that could be fitted into the existing framework incrementally, but it
>>    is something worth keeping in mind, as you're going to run into that
>>    pretty soon.
>> Yup, I've intentially ignored this for now.
> 
> Awasome.
> _______________________________________________
> lldb-dev mailing list
> lldb-dev at lists.llvm.org <mailto:lldb-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev <http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20190107/c3f70bf1/attachment-0001.html>