Message ID | 20200908122151.14025-1-martin@martin.st |
---|---|
State | Accepted |
Commit | bd6ecbe48ada79bb14cbb30ef8318495b5237790 |
Headers | show |
Series | [v2] libgcc: Expose the instruction pointer and stack pointer in SEH _Unwind_Backtrace | expand |
Hello, as noone seems to be able to review this patch, I will do so, even if this is no longer a task of mine. The patch itself is reasonable and seems to fix a pending issue we have on CFA support. I had already private discussion with Martin, and I would have loved to see a test-case illustrating this fix. But as Martin explained to me, there is no trivial testcase for this, so I would approve that patch. Regards, Kai Am Di., 8. Sept. 2020 um 14:24 Uhr schrieb Martin Storsjö <martin@martin.st>: > > Previously, the SEH version of _Unwind_Backtrace did unwind > the stack and call the provided callback function as intended, > but there was little the caller could do within the callback to > actually get any info about that particular level in the unwind. > > Set the ra and cfa pointers, which are used by _Unwind_GetIP > and _Unwind_GetCFA, to allow using these functions from the > callacb to inspect the state at each stack frame. > > 2020-09-08 Martin Storsjö <martin@martin.st> > > libgcc/Changelog: > * unwind-seh.c (_Unwind_Backtrace): Set the ra and cfa pointers > before calling the callback. > --- > libgcc/unwind-seh.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/libgcc/unwind-seh.c b/libgcc/unwind-seh.c > index 1a70180cfaa..275d782903a 100644 > --- a/libgcc/unwind-seh.c > +++ b/libgcc/unwind-seh.c > @@ -466,6 +466,11 @@ _Unwind_Backtrace(_Unwind_Trace_Fn trace, > &gcc_context.disp->HandlerData, > &gcc_context.disp->EstablisherFrame, NULL); > > + /* Set values that the callback can inspect via _Unwind_GetIP > + * and _Unwind_GetCFA. */ > + gcc_context.ra = ms_context.Rip; > + gcc_context.cfa = ms_context.Rsp; > + > /* Call trace function. */ > if (trace (&gcc_context, trace_argument) != _URC_NO_REASON) > return _URC_FATAL_PHASE1_ERROR; > -- > 2.17.1 >
On 9/8/20 12:21 PM, Martin Storsjö wrote: > Previously, the SEH version of _Unwind_Backtrace did unwind > the stack and call the provided callback function as intended, > but there was little the caller could do within the callback to > actually get any info about that particular level in the unwind. > > Set the ra and cfa pointers, which are used by _Unwind_GetIP > and _Unwind_GetCFA, to allow using these functions from the > callacb to inspect the state at each stack frame. > > 2020-09-08 Martin Storsjö <martin@martin.st> > > libgcc/Changelog: > * unwind-seh.c (_Unwind_Backtrace): Set the ra and cfa pointers > before calling the callback. > --- > libgcc/unwind-seh.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/libgcc/unwind-seh.c b/libgcc/unwind-seh.c > index 1a70180cfaa..275d782903a 100644 > --- a/libgcc/unwind-seh.c > +++ b/libgcc/unwind-seh.c > @@ -466,6 +466,11 @@ _Unwind_Backtrace(_Unwind_Trace_Fn trace, > &gcc_context.disp->HandlerData, > &gcc_context.disp->EstablisherFrame, NULL); > > + /* Set values that the callback can inspect via _Unwind_GetIP > + * and _Unwind_GetCFA. */ > + gcc_context.ra = ms_context.Rip; > + gcc_context.cfa = ms_context.Rsp; > + > /* Call trace function. */ > if (trace (&gcc_context, trace_argument) != _URC_NO_REASON) > return _URC_FATAL_PHASE1_ERROR; > Pushed to master branch, thanks.
On 11/2/20 5:05 AM, Kai Tietz via Gcc-patches wrote: > Hello, > > as noone seems to be able to review this patch, I will do so, even if > this is no longer a task of mine. > The patch itself is reasonable and seems to fix a pending issue we > have on CFA support. I had already private discussion with Martin, > and I would have loved to see a test-case illustrating this fix. But > as Martin explained to me, there is no trivial testcase for this, so I > would approve that patch. Thanks Kai. I'll get the bits installed momentarily. jeff
On 9/8/20 6:21 AM, Martin Storsjö wrote: > Previously, the SEH version of _Unwind_Backtrace did unwind > the stack and call the provided callback function as intended, > but there was little the caller could do within the callback to > actually get any info about that particular level in the unwind. > > Set the ra and cfa pointers, which are used by _Unwind_GetIP > and _Unwind_GetCFA, to allow using these functions from the > callacb to inspect the state at each stack frame. > > 2020-09-08 Martin Storsjö <martin@martin.st> > > libgcc/Changelog: > * unwind-seh.c (_Unwind_Backtrace): Set the ra and cfa pointers > before calling the callback. Ah, it's already been committed. Sorry for the delays and confusion. jeff
On 11/6/20 3:27 AM, Jeff Law wrote: > > On 11/2/20 5:05 AM, Kai Tietz via Gcc-patches wrote: >> Hello, >> >> as noone seems to be able to review this patch, I will do so, even if >> this is no longer a task of mine. >> The patch itself is reasonable and seems to fix a pending issue we >> have on CFA support. I had already private discussion with Martin, >> and I would have loved to see a test-case illustrating this fix. But >> as Martin explained to me, there is no trivial testcase for this, so I >> would approve that patch. > > Thanks Kai. I'll get the bits installed momentarily. > FYI I already pushed this recently to gcc master branch bd6ecbe48ada79bb14cbb30ef8318495b5237790, but did not reply earlier. Sorry about the confusion.
diff --git a/libgcc/unwind-seh.c b/libgcc/unwind-seh.c index 1a70180cfaa..275d782903a 100644 --- a/libgcc/unwind-seh.c +++ b/libgcc/unwind-seh.c @@ -466,6 +466,11 @@ _Unwind_Backtrace(_Unwind_Trace_Fn trace, &gcc_context.disp->HandlerData, &gcc_context.disp->EstablisherFrame, NULL); + /* Set values that the callback can inspect via _Unwind_GetIP + * and _Unwind_GetCFA. */ + gcc_context.ra = ms_context.Rip; + gcc_context.cfa = ms_context.Rsp; + /* Call trace function. */ if (trace (&gcc_context, trace_argument) != _URC_NO_REASON) return _URC_FATAL_PHASE1_ERROR;