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 <jo...@devlieghere.com> wrote: > 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
