labath added a comment. In D66250#1632187 <https://reviews.llvm.org/D66250#1632187>, @jasonmolenda wrote:
> In D66250#1631200 <https://reviews.llvm.org/D66250#1631200>, @labath wrote: > > > I have one high-level question. Who is generating the code inside the > > "trampoline" object file? Couldn't we have him generate proper unwind > > information so that the regular unwind machinery "just works"? I imagine > > all it would take is a couple of well-placed `.cfi` assembler directives... > > > The ABI plugin creates the trampoline by writing bytes directly IIRC. The > trampoline has to save the entire register context to the stack before it > calls the jitted function that does the comparison and possibly breaks. Then > the trampoline restores all the register context, executes any replaced > functions, and then jumps directly back into the original function. > > The trampoline has to be expressed in terms of assembly (or hardcoded bytes) > because we're not using the normal calling conventions - we have to spill all > registers so the jitted function doesn't overwrite a caller-spilled register > and corrupt the original user function that we're patching in to. Also, it > is not CALLed from the original user function, it is JMPed to, so there's no > way for the unwinder to get from the trampoline function back to the original > user function by looking at the stack. > > The question is then, why do we have to JMP to the trampoline? This > simplifies running the replaced instructions in the trampoline, because the > stack pointer and frame pointer have the original user function's values when > the replaced instructions are executed at the end of the trampoline. Using a > CALL to get from the original function to the trampoline solves the unwinder > problem, but we're still going to need to express the trampoline in assembly > terms to save & restore all the registers, and have the replaced instructions > execute. > > On non-x86 architectures, like Aarch64, doing a function call to the > trampoline would mean we need to spill the $lr value to the stack in the > original user function before calling the trampoline so we don't overwrite > the value; that's a larger number of instructions that need to be shifted > into the trampoline to execute. > > Of course the only reason we have this ObjectFile plugin is for the > unwinder's benefit. If we used a CALL instruction to get to the trampoline, > and we sent the assembly through the clang FE as a series of asm()s to jit > it, instead of writing the bytes down, the unwinder would get the unwind > information as we do a normal jitted function -- you're right, cfi directives > to point to the saved register context would be sufficient. > > But because we're jumping directly to the trampoline, and jumping back from > the trampoline, we need to have a hand-written unwind plan that has the > return address hard coded in it for the unwinder to work. > > Is having a small ObjectFile plugin for this purpose a bad idea? It seems > like the right approach, but maybe I'm not seeing the issue. The > CreateInstance method returns false always, so when we iterate looking for an > ObjectFile plugin, we will call this method but it will return false right > away. I wouldn't say creating a new ObjectFile class (*) is necessarily a "bad" idea. I am just wondering whether it is possible to implement this feature without introducing special knowledge about it to other parts of the code, as that has a lot of benefits (increasing coverage for the existing code paths that you do end up using, keeping the feature self-contained, etc.). I understand the reason for doing jumps, and I totally agree with that. And I agree that in this case the unwinder would need some additional help to figure out the return address. However, we already have a mechanism for telling the unwinder how to unwind from tricky frames -- the eh_frame section. And this kind of unwind is easily expressible using eh_frame data -- you could say that via something like `DW_CFA_val_expression(%rip, DW_OP_const8u $WHATEVER)`. What you're doing here is extremely similar to the signal hander trampolines. These are also written in assembly and have hand-coded eh_frame data which precisely describes where the signal hander will return, and where are all the registers saved. Its unwind rule is something like row[0]: 0: CFA=DW_OP_breg7 +160, DW_OP_deref => rax=[DW_OP_breg7 +144] rdx=[DW_OP_breg7 +136] rcx=[DW_OP_breg7 +152] rbx=[DW_OP_breg7 +128] rsi=[DW_OP_breg7 +112] rdi=[DW_OP_breg7 +104] rbp=[DW_OP_breg7 +120] rsp=[DW_OP_breg7 +160] r8=[DW_OP_breg7 +40] r9=[DW_OP_breg7 +48] r10=[DW_OP_breg7 +56] r11=[DW_OP_breg7 +64] r12=[DW_OP_breg7 +72] r13=[DW_OP_breg7 +80] r14=[DW_OP_breg7 +88] r15=[DW_OP_breg7 +96] rip=[DW_OP_breg7 +168] These kinds of rules can be "easily" generated by an appropriate `.cfi_escape` directive, similar to how he it's done in some of our tests https://github.com/llvm-mirror/lldb/blob/master/lit/Unwind/Inputs/eh-frame-dwarf-unwind.s. I am hoping this assembly, can be passed down the regular codegen pipeline together with the rest of the breakpoint condition expression. Probably the simplest way to do that would be via a top-level `asm` expression like so https://godbolt.org/z/9nRD8k. Then, if I understand correctly, we are already able to read debug information from the expressions we inject into the process (so that we're able to debug them). If that's the case, then I'm hoping that the unwind information we inject this way would be automatically picked up and the unwinder would "just work" -- if not, there's an opportunity to improve debugging of jitted expressions while doing this. At the same time, the knowledge of how to unwind from these trampolines remains closely coupled with the code for the actual trampoline. What do you think about that? (*) I am here deliberately avoid thing the word "plugin", because this class is so tightly coupled with the trampoline code (and it will probably get even more coupled once you start to teach the unwinder how to restore all of the other registers), that it makes no sense to use it for anything else. It that sense it is not really a "plugin", and (if we do go down this path) I don't think it should get it's own plugin directory, but should rather live some place very close to the place that creates these trampolines. 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
