> On Jan 7, 2019, at 11:31 AM, Pavel Labath via lldb-dev > <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 <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 >> 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@lists.llvm.org <mailto:lldb-dev@lists.llvm.org> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev > <http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev>
_______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
