[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett created this revision. Herald added subscribers: kristof.beyls, mgorny. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This adds a specific unwind plan for AArch64 Linux sigreturn frames. Previously we assumed t

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett added a reviewer: omjavaid. DavidSpickett added inline comments. Herald added a subscriber: JDevlieghere. Comment at: lldb/source/Symbol/AArch64UnwindInfo.cpp:40 + // [1] + // https://github.com/torvalds/linux/blob/master/arch/arm64/kernel/signal.c + int32_t offs

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

omjavaid accepted this revision. omjavaid added a comment. This revision is now accepted and ready to land. This look great. Thanks for doing the legit fix. @mgorny This is something you might wanna implement/test on BSD. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

labath requested changes to this revision. labath added a reviewer: jasonmolenda. labath added a comment. This revision now requires changes to proceed. I don't think this is a scalable way to fix this, and should get more discussion. Jason what do you make of this? Repository: rG LLVM Github

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

labath added a comment. I'm sorry for terse response. I wanted to ensure this gets more attention, but I wasn't able to look at this straight away. Let me try to elaborate. We generally try to avoid platform-specific code in the core libraries. The unwind code is one of the biggest offenders he

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett added a comment. > Creating a new Platform entry point to provide the unwind plan (or somehow > refactoring the GetTrapHandlerSymbolNames so that it can also provide an > unwind plan) Makes sense, I'll try that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett added inline comments. Comment at: lldb/source/Symbol/AArch64UnwindInfo.cpp:58 + unwind_plan_sp->SetSourcedFromCompiler(eLazyBoolYes); + unwind_plan_sp->SetUnwindPlanValidAtAllInstructions(eLazyBoolNo); + unwind_plan_sp->SetUnwindPlanForSignalTrap(eLazyBoolYes);

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett updated this revision to Diff 382287. DavidSpickett added a comment. Set ValidAtAllInstructions. Move building unwind plan into PlatformLinux and add a platform method to lookup unwind plans for signal handlers. I'm still checking architecture in PlatformLinux but it does move the c

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett added inline comments. Comment at: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp:258 + UnwindPlanSP unwind_plan_sp; + if (name != "__kernel_rt_sigreturn") +return unwind_plan_sp; PlatformLinux lists 3 handlers: * _sigtramp * __kernel_rt

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett updated this revision to Diff 382354. DavidSpickett added a comment. Set SP not FP based on the sigcontext. Before, the SP of the frame before sigcontext would be incorrect. (noticed while testing alt signal stacks) This was already done in the libunwind change but I got them mixed

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

labath added a comment. Thanks for doing this. Just a couple of comments inline. Comment at: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp:288 + + unwind_plan_sp = std::make_shared(eRegisterKindGeneric); + unwind_plan_sp->AppendRow(row); I have a feeli

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

labath added inline comments. Comment at: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp:295 + unwind_plan_sp->SetUnwindPlanForSignalTrap(eLazyBoolYes); + unwind_plan_sp->SetRegisterKind(eRegisterKindDWARF); + Ah now I see that it worked because of this.

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett updated this revision to Diff 382575. DavidSpickett added a comment. Turns out comparing thread.get_thread_frames() doesn't work because you end up getting references to the same frames. Instead save the info we want to compare. One obvious reason this tactic should have failed is t

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett added a comment. > Thanks for doing this. Just a couple of comments inline. Will respond to these shortly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112069/new/ https://reviews.llvm.org/D112069 __

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

labath added a comment. Since the signal context contains a copy of all registers, maybe you should be setting all of them. The other registers will not be used for unwinding, but they will enable to user to inspect the registers in the stack frames above the signal handler. One way to demonstr

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett updated this revision to Diff 382590. DavidSpickett added a comment. Use generic register names and don't set type to DWARF. Pass triple instead of machine and use isAArch64. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112069/new/

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett updated this revision to Diff 382591. DavidSpickett added a comment. Update comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112069/new/ https://reviews.llvm.org/D112069 Files: lldb/include/lldb/Target/Platform.h lldb/sourc

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett added a comment. > One way to demonstrate (and test) this would be to have a bit of inline asm, > which sets all registers to known values, and then raises a signal (SIGILL is > probably the easiest to raise from asm). I will give that a go, probably as a separate test case. Rep

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett updated this revision to Diff 386108. DavidSpickett added a comment. - Set locations for all general purpose registers - Set register type to DWARF due to that - Add a new test that sets known register values and compares them between signal catch and handler - Remove changes to han

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett added a comment. I'll at least make the floating point/SVE changes children of this review since it'll add some complexity. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112069/new/ https://reviews.llvm.org/D112069 _

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett added inline comments. Comment at: lldb/test/API/linux/aarch64/unwind_signal/main.c:23 + SETREG("24") SETREG("25") SETREG("26") SETREG("27") + SETREG("28") SETREG("29") SETREG("30") /* 31 is xzr/sp */ + /* clang-format on */ In my tes

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. In D112069#3120951 , @DavidSpickett wrote: > - Set locations for all general purpose registers > - Set register type to DWARF due to that > - Add a ne

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett updated this revision to Diff 386466. DavidSpickett added a comment. - Set x registers to a different pattern in the signal handler - Don't save frames before and after, just check for the right pattern when unwinding out of the handler to sigill() - Set register kind when we first

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

This revision was automatically updated to reflect the committed changes. Closed by commit rG9db2541d4c30: [lldb][AArch64] Add UnwindPlan for Linux sigreturn (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112069/new/ http

[Lldb-commits] [PATCH] D112069: [lldb][AArch64] Add UnwindPlan for Linux sigreturn

DavidSpickett added a comment. Thanks for the help, really helped get a high quality fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112069/new/ https://reviews.llvm.org/D112069 ___ lldb-commits maili