[lldb-dev] [Reproducers] SBReproducer RFC

Tue Jan 8 09:28:59 PST 2019

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/d3d796b0/attachment.html>