On Tue, Jan 8, 2019 at 8:27 AM Frédéric Riss <fr...@apple.com> wrote:
> > > > On Jan 8, 2019, at 1:25 AM, Pavel Labath <pa...@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@lists.llvm.org <mailto:lldb-dev@lists.llvm.org>> wrote: > >>> > >>> On 07/01/2019 19:26, Jonas Devlieghere wrote: > >>>> On Mon, Jan 7, 2019 at 1:40 AM Pavel Labath <pa...@labath.sk <mailto: > pa...@labath.sk><mailto:pa...@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 > >
_______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
