Since clang does not push pc and sp in function prologues, the current
implementation of unwind_frame does not work. By using the previous
frame's lr/fp instead of saved pc/sp we get valid unwinds on clang-built
kernels.
The bounds check on next frame pointer must be changed as well since
there are 8 less bytes between frames.
This fixes /proc/<pid>/stack.
Link: https://github.com/ClangBuiltLinux/linux/issues/912
Signed-off-by: Nathan Huckleberry <[email protected]>
---
arch/arm/kernel/stacktrace.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
index cc726afea023..76ea4178a55c 100644
--- a/arch/arm/kernel/stacktrace.c
+++ b/arch/arm/kernel/stacktrace.c
@@ -22,6 +22,19 @@
* A simple function epilogue looks like this:
* ldm sp, {fp, sp, pc}
*
+ * When compiled with clang, pc and sp are not pushed. A simple function
+ * prologue looks like this when built with clang:
+ *
+ * stmdb {..., fp, lr}
+ * add fp, sp, #x
+ * sub sp, sp, #y
+ *
+ * A simple function epilogue looks like this when built with clang:
+ *
+ * sub sp, fp, #x
+ * ldm {..., fp, pc}
+ *
+ *
* Note that with framepointer enabled, even the leaf functions have the same
* prologue and epilogue, therefore we can ignore the LR value in this case.
*/
@@ -34,6 +47,16 @@ int notrace unwind_frame(struct stackframe *frame)
low = frame->sp;
high = ALIGN(low, THREAD_SIZE);
+#ifdef CONFIG_CC_IS_CLANG
+ /* check current frame pointer is within bounds */
+ if (fp < low + 4 || fp > high - 4)
+ return -EINVAL;
+
+ frame->sp = frame->fp;
+ frame->fp = *(unsigned long *)(fp);
+ frame->pc = frame->lr;
+ frame->lr = *(unsigned long *)(fp + 4);
+#else
/* check current frame pointer is within bounds */
if (fp < low + 12 || fp > high - 4)
return -EINVAL;
@@ -42,6 +65,7 @@ int notrace unwind_frame(struct stackframe *frame)
frame->fp = *(unsigned long *)(fp - 12);
frame->sp = *(unsigned long *)(fp - 8);
frame->pc = *(unsigned long *)(fp - 4);
+#endif
return 0;
}
--
2.27.0.290.gba653c62da-goog
On Wed, Jun 17, 2020 at 12:36 AM 'Nathan Huckleberry' via Clang Built
Linux <[email protected]> wrote:
>
> Since clang does not push pc and sp in function prologues, the current
> implementation of unwind_frame does not work. By using the previous
> frame's lr/fp instead of saved pc/sp we get valid unwinds on clang-built
> kernels.
>
> The bounds check on next frame pointer must be changed as well since
> there are 8 less bytes between frames.
>
> This fixes /proc/<pid>/stack.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/912
> Signed-off-by: Nathan Huckleberry <[email protected]>
Just a small nit.
Please label your patch with: "arm/stacktrace: ..." or "arm: stacktrace: ..."
git log --oneline arch/arm/kernel/stacktrace.c
...is your friend :-).
- Sedat -
> ---
> arch/arm/kernel/stacktrace.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
> index cc726afea023..76ea4178a55c 100644
> --- a/arch/arm/kernel/stacktrace.c
> +++ b/arch/arm/kernel/stacktrace.c
> @@ -22,6 +22,19 @@
> * A simple function epilogue looks like this:
> * ldm sp, {fp, sp, pc}
> *
> + * When compiled with clang, pc and sp are not pushed. A simple function
> + * prologue looks like this when built with clang:
> + *
> + * stmdb {..., fp, lr}
> + * add fp, sp, #x
> + * sub sp, sp, #y
> + *
> + * A simple function epilogue looks like this when built with clang:
> + *
> + * sub sp, fp, #x
> + * ldm {..., fp, pc}
> + *
> + *
> * Note that with framepointer enabled, even the leaf functions have the same
> * prologue and epilogue, therefore we can ignore the LR value in this case.
> */
> @@ -34,6 +47,16 @@ int notrace unwind_frame(struct stackframe *frame)
> low = frame->sp;
> high = ALIGN(low, THREAD_SIZE);
>
> +#ifdef CONFIG_CC_IS_CLANG
> + /* check current frame pointer is within bounds */
> + if (fp < low + 4 || fp > high - 4)
> + return -EINVAL;
> +
> + frame->sp = frame->fp;
> + frame->fp = *(unsigned long *)(fp);
> + frame->pc = frame->lr;
> + frame->lr = *(unsigned long *)(fp + 4);
> +#else
> /* check current frame pointer is within bounds */
> if (fp < low + 12 || fp > high - 4)
> return -EINVAL;
> @@ -42,6 +65,7 @@ int notrace unwind_frame(struct stackframe *frame)
> frame->fp = *(unsigned long *)(fp - 12);
> frame->sp = *(unsigned long *)(fp - 8);
> frame->pc = *(unsigned long *)(fp - 4);
> +#endif
>
> return 0;
> }
> --
> 2.27.0.290.gba653c62da-goog
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200616223633.73971-1-nhuck%40google.com.
On Tue, Jun 16, 2020 at 3:36 PM 'Nathan Huckleberry' via Clang Built
Linux <[email protected]> wrote:
>
> Since clang does not push pc and sp in function prologues, the current
> implementation of unwind_frame does not work. By using the previous
> frame's lr/fp instead of saved pc/sp we get valid unwinds on clang-built
> kernels.
>
> The bounds check on next frame pointer must be changed as well since
> there are 8 less bytes between frames.
>
> This fixes /proc/<pid>/stack.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/912
> Signed-off-by: Nathan Huckleberry <[email protected]>
Thanks for the patch, Nathan! When I looked into this, I found the
latest ARM AAPCS [0] specifically says (with `it` referring to `a
platform`: "It may permit the frame pointer register to be used as a
general-purpose callee-saved register, but provide a platform-specific
mechanism for external agents to reliably locate the chain of frame
records." While it's good that's acknowledged in the documentation,
the current wording is relaxed in order to not force current
implementations to converge. This has the unfortunate side effect of
making finding the frame pointer toolchain dependendent, hence this
patch and your previous commit 6dc5fd93b2f1 ("ARM: 8900/1:
UNWINDER_FRAME_POINTER implementation for Clang"). Being more
specific in the documentation would force at least one implementation
to change, and I think that would also imply an ABI break. So I can
see it both ways, though I still would prefer that the language pin
this down, even if we had to change LLVM. Just providing additional
context for folks on the thread.
This should also have a reported by tag from Miles, in v2.
Reported-by: Miles Chen <[email protected]>
Miles mentioned to me that he tested it, but maybe Miles can confirm
that publicly on-list via an explicit Tested-by: tag?
This would be useful for us to have in stable; otherwise we'll have to
carry out of tree in Android and CrOS, which I'd rather not do. Via
Documentation/process/stable-kernel-rules.rst, if you add this tag to
V2, that will greatly simplify submitting this to stable:
Cc: [email protected]
Miles also showed me the behavior of this patch for different kernel
versions, which varies anywhere from empty or single entry traces to
panics, so this is pretty important that this works for Clang builds.
[0] https://static.docs.arm.com/ihi0042/i/aapcs32.pdf
> ---
> arch/arm/kernel/stacktrace.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
> index cc726afea023..76ea4178a55c 100644
> --- a/arch/arm/kernel/stacktrace.c
> +++ b/arch/arm/kernel/stacktrace.c
> @@ -22,6 +22,19 @@
> * A simple function epilogue looks like this:
> * ldm sp, {fp, sp, pc}
> *
> + * When compiled with clang, pc and sp are not pushed. A simple function
> + * prologue looks like this when built with clang:
> + *
> + * stmdb {..., fp, lr}
> + * add fp, sp, #x
> + * sub sp, sp, #y
> + *
> + * A simple function epilogue looks like this when built with clang:
> + *
> + * sub sp, fp, #x
> + * ldm {..., fp, pc}
> + *
> + *
> * Note that with framepointer enabled, even the leaf functions have the same
> * prologue and epilogue, therefore we can ignore the LR value in this case.
> */
> @@ -34,6 +47,16 @@ int notrace unwind_frame(struct stackframe *frame)
> low = frame->sp;
> high = ALIGN(low, THREAD_SIZE);
>
> +#ifdef CONFIG_CC_IS_CLANG
> + /* check current frame pointer is within bounds */
> + if (fp < low + 4 || fp > high - 4)
The patch LGTM; maybe Russell or Catalin could triple check this
bounds check? Assuming there's no issue, you can include on a v2 my
reviewed by:
Reviewed-by: Nick Desaulniers <[email protected]>
I'd probably wait the remainder of a week before sending a v2 to
collect additional feedback. Thank you again.
> + return -EINVAL;
> +
> + frame->sp = frame->fp;
> + frame->fp = *(unsigned long *)(fp);
> + frame->pc = frame->lr;
> + frame->lr = *(unsigned long *)(fp + 4);
> +#else
> /* check current frame pointer is within bounds */
> if (fp < low + 12 || fp > high - 4)
> return -EINVAL;
> @@ -42,6 +65,7 @@ int notrace unwind_frame(struct stackframe *frame)
> frame->fp = *(unsigned long *)(fp - 12);
> frame->sp = *(unsigned long *)(fp - 8);
> frame->pc = *(unsigned long *)(fp - 4);
> +#endif
>
> return 0;
> }
> --
> 2.27.0.290.gba653c62da-goog
>
> --
--
Thanks,
~Nick Desaulniers
On Thu, Jun 18, 2020 at 1:22 PM Nick Desaulniers
<[email protected]> wrote:
>
> On Tue, Jun 16, 2020 at 3:36 PM 'Nathan Huckleberry' via Clang Built
> Linux <[email protected]> wrote:
> >
> > Since clang does not push pc and sp in function prologues, the current
> > implementation of unwind_frame does not work. By using the previous
> > frame's lr/fp instead of saved pc/sp we get valid unwinds on clang-built
> > kernels.
> >
> > The bounds check on next frame pointer must be changed as well since
> > there are 8 less bytes between frames.
> >
> > This fixes /proc/<pid>/stack.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/912
> > Signed-off-by: Nathan Huckleberry <[email protected]>
>
> Thanks for the patch, Nathan! When I looked into this, I found the
> latest ARM AAPCS [0] specifically says (with `it` referring to `a
> platform`: "It may permit the frame pointer register to be used as a
> general-purpose callee-saved register, but provide a platform-specific
> mechanism for external agents to reliably locate the chain of frame
> records." While it's good that's acknowledged in the documentation,
> the current wording is relaxed in order to not force current
> implementations to converge. This has the unfortunate side effect of
> making finding the frame pointer toolchain dependendent, hence this
> patch and your previous commit 6dc5fd93b2f1 ("ARM: 8900/1:
> UNWINDER_FRAME_POINTER implementation for Clang"). Being more
> specific in the documentation would force at least one implementation
> to change, and I think that would also imply an ABI break. So I can
> see it both ways, though I still would prefer that the language pin
> this down, even if we had to change LLVM. Just providing additional
> context for folks on the thread.
>
> This should also have a reported by tag from Miles, in v2.
>
> Reported-by: Miles Chen <[email protected]>
>
> Miles mentioned to me that he tested it, but maybe Miles can confirm
> that publicly on-list via an explicit Tested-by: tag?
>
> This would be useful for us to have in stable; otherwise we'll have to
> carry out of tree in Android and CrOS, which I'd rather not do. Via
> Documentation/process/stable-kernel-rules.rst, if you add this tag to
> V2, that will greatly simplify submitting this to stable:
> Cc: [email protected]
>
> Miles also showed me the behavior of this patch for different kernel
s/this patch/this function before this patch is applied/
> versions, which varies anywhere from empty or single entry traces to
> panics, so this is pretty important that this works for Clang builds.
>
> [0] https://static.docs.arm.com/ihi0042/i/aapcs32.pdf
>
> > ---
> > arch/arm/kernel/stacktrace.c | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
> > index cc726afea023..76ea4178a55c 100644
> > --- a/arch/arm/kernel/stacktrace.c
> > +++ b/arch/arm/kernel/stacktrace.c
> > @@ -22,6 +22,19 @@
> > * A simple function epilogue looks like this:
> > * ldm sp, {fp, sp, pc}
> > *
> > + * When compiled with clang, pc and sp are not pushed. A simple function
> > + * prologue looks like this when built with clang:
> > + *
> > + * stmdb {..., fp, lr}
> > + * add fp, sp, #x
> > + * sub sp, sp, #y
> > + *
> > + * A simple function epilogue looks like this when built with clang:
> > + *
> > + * sub sp, fp, #x
> > + * ldm {..., fp, pc}
> > + *
> > + *
> > * Note that with framepointer enabled, even the leaf functions have the same
> > * prologue and epilogue, therefore we can ignore the LR value in this case.
> > */
> > @@ -34,6 +47,16 @@ int notrace unwind_frame(struct stackframe *frame)
> > low = frame->sp;
> > high = ALIGN(low, THREAD_SIZE);
> >
> > +#ifdef CONFIG_CC_IS_CLANG
> > + /* check current frame pointer is within bounds */
> > + if (fp < low + 4 || fp > high - 4)
>
> The patch LGTM; maybe Russell or Catalin could triple check this
> bounds check? Assuming there's no issue, you can include on a v2 my
> reviewed by:
>
> Reviewed-by: Nick Desaulniers <[email protected]>
>
> I'd probably wait the remainder of a week before sending a v2 to
> collect additional feedback. Thank you again.
>
> > + return -EINVAL;
> > +
> > + frame->sp = frame->fp;
> > + frame->fp = *(unsigned long *)(fp);
> > + frame->pc = frame->lr;
> > + frame->lr = *(unsigned long *)(fp + 4);
> > +#else
> > /* check current frame pointer is within bounds */
> > if (fp < low + 12 || fp > high - 4)
> > return -EINVAL;
> > @@ -42,6 +65,7 @@ int notrace unwind_frame(struct stackframe *frame)
> > frame->fp = *(unsigned long *)(fp - 12);
> > frame->sp = *(unsigned long *)(fp - 8);
> > frame->pc = *(unsigned long *)(fp - 4);
> > +#endif
> >
> > return 0;
> > }
> > --
> > 2.27.0.290.gba653c62da-goog
> >
> > --
>
> --
> Thanks,
> ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
On Wed, Jun 17, 2020 at 06:23:48AM +0200, Sedat Dilek wrote:
> On Wed, Jun 17, 2020 at 12:36 AM 'Nathan Huckleberry' via Clang Built
> Linux <[email protected]> wrote:
> >
> > Since clang does not push pc and sp in function prologues, the current
> > implementation of unwind_frame does not work. By using the previous
> > frame's lr/fp instead of saved pc/sp we get valid unwinds on clang-built
> > kernels.
> >
> > The bounds check on next frame pointer must be changed as well since
> > there are 8 less bytes between frames.
> >
> > This fixes /proc/<pid>/stack.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/912
> > Signed-off-by: Nathan Huckleberry <[email protected]>
>
> Just a small nit.
>
> Please label your patch with: "arm/stacktrace: ..." or "arm: stacktrace: ..."
Convention is to use "ARM: ..." for arch/arm.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Thu, 2020-06-18 at 13:22 -0700, Nick Desaulniers wrote:
> On Tue, Jun 16, 2020 at 3:36 PM 'Nathan Huckleberry' via Clang Built
> Linux <[email protected]> wrote:
> >
> > Since clang does not push pc and sp in function prologues, the current
> > implementation of unwind_frame does not work. By using the previous
> > frame's lr/fp instead of saved pc/sp we get valid unwinds on clang-built
> > kernels.
> >
> > The bounds check on next frame pointer must be changed as well since
> > there are 8 less bytes between frames.
> >
> > This fixes /proc/<pid>/stack.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/912
> > Signed-off-by: Nathan Huckleberry <[email protected]>
>
> Thanks for the patch, Nathan! When I looked into this, I found the
> latest ARM AAPCS [0] specifically says (with `it` referring to `a
> platform`: "It may permit the frame pointer register to be used as a
> general-purpose callee-saved register, but provide a platform-specific
> mechanism for external agents to reliably locate the chain of frame
> records." While it's good that's acknowledged in the documentation,
> the current wording is relaxed in order to not force current
> implementations to converge. This has the unfortunate side effect of
> making finding the frame pointer toolchain dependendent, hence this
> patch and your previous commit 6dc5fd93b2f1 ("ARM: 8900/1:
> UNWINDER_FRAME_POINTER implementation for Clang"). Being more
> specific in the documentation would force at least one implementation
> to change, and I think that would also imply an ABI break. So I can
> see it both ways, though I still would prefer that the language pin
> this down, even if we had to change LLVM. Just providing additional
> context for folks on the thread.
>
> This should also have a reported by tag from Miles, in v2.
>
> Reported-by: Miles Chen <[email protected]>
>
> Miles mentioned to me that he tested it, but maybe Miles can confirm
> that publicly on-list via an explicit Tested-by: tag?
Thanks for the patch.
Tested-by: Miles Chen <[email protected]>
>
> This would be useful for us to have in stable; otherwise we'll have to
> carry out of tree in Android and CrOS, which I'd rather not do. Via
> Documentation/process/stable-kernel-rules.rst, if you add this tag to
> V2, that will greatly simplify submitting this to stable:
> Cc: [email protected]
>
> Miles also showed me the behavior of this patch for different kernel
> versions, which varies anywhere from empty or single entry traces to
> panics, so this is pretty important that this works for Clang builds.
>
> [0] https://static.docs.arm.com/ihi0042/i/aapcs32.pdf
>
> > ---
> > arch/arm/kernel/stacktrace.c | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
> > index cc726afea023..76ea4178a55c 100644
> > --- a/arch/arm/kernel/stacktrace.c
> > +++ b/arch/arm/kernel/stacktrace.c
> > @@ -22,6 +22,19 @@
> > * A simple function epilogue looks like this:
> > * ldm sp, {fp, sp, pc}
> > *
> > + * When compiled with clang, pc and sp are not pushed. A simple function
> > + * prologue looks like this when built with clang:
> > + *
> > + * stmdb {..., fp, lr}
> > + * add fp, sp, #x
> > + * sub sp, sp, #y
> > + *
> > + * A simple function epilogue looks like this when built with clang:
> > + *
> > + * sub sp, fp, #x
> > + * ldm {..., fp, pc}
> > + *
> > + *
> > * Note that with framepointer enabled, even the leaf functions have the same
> > * prologue and epilogue, therefore we can ignore the LR value in this case.
> > */
> > @@ -34,6 +47,16 @@ int notrace unwind_frame(struct stackframe *frame)
> > low = frame->sp;
> > high = ALIGN(low, THREAD_SIZE);
> >
> > +#ifdef CONFIG_CC_IS_CLANG
> > + /* check current frame pointer is within bounds */
> > + if (fp < low + 4 || fp > high - 4)
>
> The patch LGTM; maybe Russell or Catalin could triple check this
> bounds check? Assuming there's no issue, you can include on a v2 my
> reviewed by:
>
> Reviewed-by: Nick Desaulniers <[email protected]>
>
> I'd probably wait the remainder of a week before sending a v2 to
> collect additional feedback. Thank you again.
>
> > + return -EINVAL;
> > +
> > + frame->sp = frame->fp;
> > + frame->fp = *(unsigned long *)(fp);
> > + frame->pc = frame->lr;
> > + frame->lr = *(unsigned long *)(fp + 4);
> > +#else
> > /* check current frame pointer is within bounds */
> > if (fp < low + 12 || fp > high - 4)
> > return -EINVAL;
> > @@ -42,6 +65,7 @@ int notrace unwind_frame(struct stackframe *frame)
> > frame->fp = *(unsigned long *)(fp - 12);
> > frame->sp = *(unsigned long *)(fp - 8);
> > frame->pc = *(unsigned long *)(fp - 4);
> > +#endif
> >
> > return 0;
> > }
> > --
> > 2.27.0.290.gba653c62da-goog
> >
> > --
>
Since clang does not push pc and sp in function prologues, the current
implementation of unwind_frame does not work. By using the previous
frame's lr/fp instead of saved pc/sp we get valid unwinds on clang-built
kernels.
The bounds check on next frame pointer must be changed as well since
there are 8 less bytes between frames.
This fixes /proc/<pid>/stack.
Link: https://github.com/ClangBuiltLinux/linux/issues/912
Reported-by: Miles Chen <[email protected]>
Tested-by: Miles Chen <[email protected]>
Cc: [email protected]
Reviewed-by: Nick Desaulniers <[email protected]>
Signed-off-by: Nathan Huckleberry <[email protected]>
---
arch/arm/kernel/stacktrace.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
index cc726afea023..76ea4178a55c 100644
--- a/arch/arm/kernel/stacktrace.c
+++ b/arch/arm/kernel/stacktrace.c
@@ -22,6 +22,19 @@
* A simple function epilogue looks like this:
* ldm sp, {fp, sp, pc}
*
+ * When compiled with clang, pc and sp are not pushed. A simple function
+ * prologue looks like this when built with clang:
+ *
+ * stmdb {..., fp, lr}
+ * add fp, sp, #x
+ * sub sp, sp, #y
+ *
+ * A simple function epilogue looks like this when built with clang:
+ *
+ * sub sp, fp, #x
+ * ldm {..., fp, pc}
+ *
+ *
* Note that with framepointer enabled, even the leaf functions have the same
* prologue and epilogue, therefore we can ignore the LR value in this case.
*/
@@ -34,6 +47,16 @@ int notrace unwind_frame(struct stackframe *frame)
low = frame->sp;
high = ALIGN(low, THREAD_SIZE);
+#ifdef CONFIG_CC_IS_CLANG
+ /* check current frame pointer is within bounds */
+ if (fp < low + 4 || fp > high - 4)
+ return -EINVAL;
+
+ frame->sp = frame->fp;
+ frame->fp = *(unsigned long *)(fp);
+ frame->pc = frame->lr;
+ frame->lr = *(unsigned long *)(fp + 4);
+#else
/* check current frame pointer is within bounds */
if (fp < low + 12 || fp > high - 4)
return -EINVAL;
@@ -42,6 +65,7 @@ int notrace unwind_frame(struct stackframe *frame)
frame->fp = *(unsigned long *)(fp - 12);
frame->sp = *(unsigned long *)(fp - 8);
frame->pc = *(unsigned long *)(fp - 4);
+#endif
return 0;
}
--
2.27.0.212.ge8ba1cc988-goog
Hi Nathan,
Would you please submit this to Russell's patch tracking system:
https://www.arm.linux.org.uk/developer/patches/
login -> add new patch.
On Mon, Jul 6, 2020 at 11:27 AM Nathan Huckleberry <[email protected]> wrote:
>
> Since clang does not push pc and sp in function prologues, the current
> implementation of unwind_frame does not work. By using the previous
> frame's lr/fp instead of saved pc/sp we get valid unwinds on clang-built
> kernels.
>
> The bounds check on next frame pointer must be changed as well since
> there are 8 less bytes between frames.
>
> This fixes /proc/<pid>/stack.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/912
> Reported-by: Miles Chen <[email protected]>
> Tested-by: Miles Chen <[email protected]>
> Cc: [email protected]
> Reviewed-by: Nick Desaulniers <[email protected]>
> Signed-off-by: Nathan Huckleberry <[email protected]>
> ---
> arch/arm/kernel/stacktrace.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
> index cc726afea023..76ea4178a55c 100644
> --- a/arch/arm/kernel/stacktrace.c
> +++ b/arch/arm/kernel/stacktrace.c
> @@ -22,6 +22,19 @@
> * A simple function epilogue looks like this:
> * ldm sp, {fp, sp, pc}
> *
> + * When compiled with clang, pc and sp are not pushed. A simple function
> + * prologue looks like this when built with clang:
> + *
> + * stmdb {..., fp, lr}
> + * add fp, sp, #x
> + * sub sp, sp, #y
> + *
> + * A simple function epilogue looks like this when built with clang:
> + *
> + * sub sp, fp, #x
> + * ldm {..., fp, pc}
> + *
> + *
> * Note that with framepointer enabled, even the leaf functions have the same
> * prologue and epilogue, therefore we can ignore the LR value in this case.
> */
> @@ -34,6 +47,16 @@ int notrace unwind_frame(struct stackframe *frame)
> low = frame->sp;
> high = ALIGN(low, THREAD_SIZE);
>
> +#ifdef CONFIG_CC_IS_CLANG
> + /* check current frame pointer is within bounds */
> + if (fp < low + 4 || fp > high - 4)
> + return -EINVAL;
> +
> + frame->sp = frame->fp;
> + frame->fp = *(unsigned long *)(fp);
> + frame->pc = frame->lr;
> + frame->lr = *(unsigned long *)(fp + 4);
> +#else
> /* check current frame pointer is within bounds */
> if (fp < low + 12 || fp > high - 4)
> return -EINVAL;
> @@ -42,6 +65,7 @@ int notrace unwind_frame(struct stackframe *frame)
> frame->fp = *(unsigned long *)(fp - 12);
> frame->sp = *(unsigned long *)(fp - 8);
> frame->pc = *(unsigned long *)(fp - 4);
> +#endif
>
> return 0;
> }
> --
> 2.27.0.212.ge8ba1cc988-goog
>
--
Thanks,
~Nick Desaulniers