labath added a comment.

In D66250#1638338 <https://reviews.llvm.org/D66250#1638338>, @jasonmolenda 
wrote:

> In D66250#1633455 <https://reviews.llvm.org/D66250#1633455>, @clayborg wrote:
>
> > Why are we not just using ObjectFileJIT? I am guessing these breakpoint 
> > expressions create one of these by compiling the breakpoint expression and 
> > JIT'ing it just like any other expression. If this is the case, then why do 
> > we need to create a ObjectFileTrampoline? Seems like we could add .cfi 
> > directives to the assembly we use for the breakpoint condition function so 
> > that we can unwind out of it?
>
>
> I hadn't looked at ObjectFileJIT, we should look at whether we can that 
> existing plugin.  I agree they're conceptually doing the same thing, but one 
> difference is that the trampoline is only a function, it's not a mach-o 
> binary that we've written into memory.  I think it would be possible to 
> represent the trampoline unwind plan in eh_frame using a DW_OP_addr encoding 
> for the caller's pc value, but we would have to manually write out the bytes 
> of the eh_frame header, CIE and FDE entries, then the DW_OP_addr.  I think 
> it's easier to stuff in an UnwindPlan with this unwind rule added manually.  
> Using the existing eh_frame format is laudable but I think it hand-writing 
> the binary format into memory and then reading it back out again would be 
> less maintainable IMO.


Setting the return address is one thing, but I'm not sure it stops there... For 
instance, I would expect you'll want to also set the "restore" rules for the GP 
registers too, so that their values come out "right" when you select the 
original frame in the debugger. We could do something special there too, but if 
we go the assembly route (which I believe has a lot of other advantages too), 
then all we'd need is to add some .cfi directives into the asm, and the 
eh_frame would fall out automatically..



================
Comment at: lldb/source/Target/ABI.cpp:10
 #include "lldb/Target/ABI.h"
+#include "Plugins/ObjectFile/Trampoline/ObjectFileTrampoline.h"
+#include "lldb/Core/Module.h"
----------------
jasonmolenda wrote:
> mib wrote:
> > labath wrote:
> > > If this is going to be something that is called directly from core lldb 
> > > code, then it not a "plugin" by any stretch of imagination. I think we 
> > > should put this file some place else.
> > Any suggestion on where to put it ?
> Maybe in source/Symbol along with the ObjectFile.cpp base class.  I agree 
> with Pavel that this isn't a plugin; it subclasses ObjectFile so it probably 
> has to implement the plugin methods, but it's not something that will ever be 
> created by iterating through the active ObjectFile plugins or anything like 
> that.
Yeah, either that, or next to the place that actually uses it (which would be 
`Target/ABI` right now). I think you have to implement some plugin methods 
(like `GetPluginName`, which is fine), but I am pretty sure you don't have the 
implement the CreateInstance stuff or register yourself with the plugin manager 
(as we will never be creating the objects this way).

That said, I'm still not convinced we really need this class in the first 
place. I would like to explore the possiblity of injecting the trampoline code 
as inline asm into the compiled expression. That way, it should automatically 
become a part of the resuling ObjectFileJIT, and it will solve a couple of 
other problems along the way (e.g., the "how to allocate memory in a 
target-independent manner" discussion in the other thread).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66250/new/

https://reviews.llvm.org/D66250



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to