| Message ID | affc672d13100eb0c743b8c7ce88c47a104161bf.1453405861.git.jpoimboe@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 21/01/2016 23:49, Josh Poimboeuf wrote: > With some configs, gcc doesn't inline test_cc(). When that happens, it > doesn't create a stack frame before inserting the call instruction. > This breaks frame pointer convention if CONFIG_FRAME_POINTER is enabled > and can result in a bad stack trace. > > Force a stack frame to be created if CONFIG_FRAME_POINTER is enabled by > listing the stack pointer as an output operand for the inline asm > statement. If an __always_inline allocation works, that would be better. Paolo > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > Cc: Gleb Natapov <gleb@kernel.org> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: kvm@vger.kernel.org > --- > arch/x86/kvm/emulate.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index aa4d726..7dba65a 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -972,11 +972,13 @@ static int em_bsr_c(struct x86_emulate_ctxt *ctxt) > static u8 test_cc(unsigned int condition, unsigned long flags) > { > u8 rc; > + register void *__sp asm(_ASM_SP); > void (*fop)(void) = (void *)em_setcc + 4 * (condition & 0xf); > > flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF; > asm("push %[flags]; popf; call *%[fastop]" > - : "=a"(rc) : [fastop]"r"(fop), [flags]"r"(flags)); > + : "=a"(rc), "+r"(__sp) > + : [fastop]"r"(fop), [flags]"r"(flags)); > return rc; > } > > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 22, 2016 at 11:05:06AM +0100, Paolo Bonzini wrote: > > > On 21/01/2016 23:49, Josh Poimboeuf wrote: > > With some configs, gcc doesn't inline test_cc(). When that happens, it > > doesn't create a stack frame before inserting the call instruction. > > This breaks frame pointer convention if CONFIG_FRAME_POINTER is enabled > > and can result in a bad stack trace. > > > > Force a stack frame to be created if CONFIG_FRAME_POINTER is enabled by > > listing the stack pointer as an output operand for the inline asm > > statement. > > If an __always_inline allocation works, that would be better. Yeah, that seems to work. I'll update the patch.
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index aa4d726..7dba65a 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -972,11 +972,13 @@ static int em_bsr_c(struct x86_emulate_ctxt *ctxt) static u8 test_cc(unsigned int condition, unsigned long flags) { u8 rc; + register void *__sp asm(_ASM_SP); void (*fop)(void) = (void *)em_setcc + 4 * (condition & 0xf); flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF; asm("push %[flags]; popf; call *%[fastop]" - : "=a"(rc) : [fastop]"r"(fop), [flags]"r"(flags)); + : "=a"(rc), "+r"(__sp) + : [fastop]"r"(fop), [flags]"r"(flags)); return rc; }
With some configs, gcc doesn't inline test_cc(). When that happens, it doesn't create a stack frame before inserting the call instruction. This breaks frame pointer convention if CONFIG_FRAME_POINTER is enabled and can result in a bad stack trace. Force a stack frame to be created if CONFIG_FRAME_POINTER is enabled by listing the stack pointer as an output operand for the inline asm statement. Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Gleb Natapov <gleb@kernel.org> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: kvm@vger.kernel.org --- arch/x86/kvm/emulate.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)