Re: [kvm-unit-tests PATCH v2 0/4] Better backtraces for leaf functions

Mathias Krause
2025-11-18T01:33:21Z

[-- Attachment #1: Type: text/plain, Size: 4248 bytes --]

On 17.11.25 23:19, Sean Christopherson wrote:
> On Sat, Nov 15, 2025, Mathias Krause wrote:
>> On 14.11.25 19:25, Sean Christopherson wrote:
>>> On Mon, 15 Sep 2025 23:54:28 +0200, Mathias Krause wrote:
>>>> This is v2 of [1], trying to enhance backtraces involving leaf
>>>> functions.
>>>>
>>>> This version fixes backtraces on ARM and ARM64 as well, as ARM currently
>>>> fails hard for leaf functions lacking a proper stack frame setup, making
>>>> it dereference invalid pointers. ARM64 just skips frames, much like x86
>>>> does.
>>>>
>>>> [...]
>>>
>>> Applied to kvm-x86 next, thanks!
>>
>> Thanks a lot, Sean!
>>
>>> P.S. This also prompted me to get pretty_print_stacks.py working in my
>>> environment, so double thanks!
>>
>> Haha, you're welcome! :D
>>
>>>
>>> [1/4] Makefile: Provide a concept of late CFLAGS
>>> https://github.com/kvm-x86/kvm-unit-tests/commit/816fe2d45aed
>>> [2/4] x86: Better backtraces for leaf functions
>>> https://github.com/kvm-x86/kvm-unit-tests/commit/f01ea38a385a
>
> Spoke too soon :-(
>
> The x86 change breaks the realmode test. I didn't try hard to debug, as that
> test is brittle, e.g. see https://lore.kernel.org/all/20240604143507.1041901-1-pbonzini@redhat.com.

Gee! 16bit code at its finest.

So I looked at it and this seems to be a bug in gcc because it emits a
5-byte NOP to make room for a 3-byte call. Actually, it doesn't even
emit a NOP, it just spits out some bytes:

minipli@bell:~$ diff -u pg.s nop_mcount.s
--- pg.s 2025-11-18 01:34:34.884472417 +0100
+++ nop_mcount.s 2025-11-18 01:34:05.716096290 +0100
@@ -11,7 +11,7 @@
.cfi_offset 5, -8
movl %esp, %ebp
.cfi_def_cfa_register 5
-1: call mcount
+1: .byte 0x0f, 0x1f, 0x44, 0x00, 0x00
movl 8(%ebp), %eax
addl %eax, %eax
popl %ebp

The issue is, while that is a perfectly fine NOPL for 32-bit and 64-bit,
it's not for 16-bit. The 16-bit version gets treated as a 4-byte NOPW
with a left-over zero byte which messes with all the following opcodes
and causes issues like you observed.

However, gcc is probably not fully to blame, as it cannot know that it's
generating 16-bit code. That piece is hidden in a asm(".code16gcc")
statement.

>
> I can't for the life of me figure out how to get Makefile variables to do what I
> want, so for now I'm going to drop the x86 change so as not to block the rest of
> the stuff I've got applied.
>
> I'll keep the rest applied, so just resubmit the x86 patch against kvm-x86/next.
>

> FWIW, conceptually I think we want something like this:
>
> diff --git a/x86/Makefile.common b/x86/Makefile.common
> index be18a77a..65e41578 100644
> --- a/x86/Makefile.common
> +++ b/x86/Makefile.common
> @@ -44,6 +44,7 @@ COMMON_CFLAGS += -O1
> KEEP_FRAME_POINTER := y
>
> ifneq ($(KEEP_FRAME_POINTER),)
> +ifneq ($(no_profiling),y)
> # Fake profiling to force the compiler to emit a frame pointer setup also in
> # leaf function (-mno-omit-leaf-frame-pointer doesn't work, unfortunately).
> #
> @@ -53,6 +54,7 @@ ifneq ($(KEEP_FRAME_POINTER),)
> # during compilation makes this do "The Right Thing."
> LATE_CFLAGS += $(call cc-option, -pg -mnop-mcount, "")
> endif
> +endif
>
> FLATLIBS = lib/libcflat.a
>
> @@ -120,6 +122,7 @@ $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o $(SRCDIR)/$(TEST_DIR)/realmode.
> $(LD) -m elf_i386 -nostdlib -o $@ \
> -T $(SRCDIR)/$(TEST_DIR)/realmode.lds $(filter %.o, $^)
>
> +$(TEST_DIR)/realmode.o: no_profiling = y
> $(TEST_DIR)/realmode.o: bits = $(realmode_bits)

Kind of, yes. Just with the $(no_profiling) part embedded into
LATE_CFLAGS. However, as that gets forcibly evaluated early (on
purpose!), that can't work.

I tried to play with $(filter-out ...,$(CFLAGS)) but that didn't work
out either. What does work, however, is to disable -mnop-mcount and make
it a call again. This just needs a stub mcount() and the realmode test
is working again.

I see that kvm-x86/next as of now (e403d30a8210 ("x86/emulator: Treat
DR6_BUS_LOCK as writable if CPU has BUS_LOCK_DETECT")) still has
f01ea38a385a ("x86: Better backtraces for leaf functions"), so I just
created a fixup commit (attached). Feel free to squash it into
f01ea38a385a or yell at if you want something else.

Thanks,
Mathias

[-- Attachment #2: 0001-x86-Prevent-realmode-test-code-instrumentation-with-.patch --]
[-- Type: text/x-patch, Size: 2392 bytes --]

From d2ac15881b794a4e3932c49a97278d0426f0aa67 Mon Sep 17 00:00:00 2001
From: Mathias Krause <minipli@grsecurity.net>
Date: Tue, 18 Nov 2025 02:14:27 +0100
Subject: [kvm-unit-tests PATCH] x86: Prevent realmode test code
instrumentation with nop-mcount

Commit f01ea38a385a ("x86: Better backtraces for leaf functions") made
use of '-pg -mnop-mcount' to provide a lightweight way to force leaf
functions to emit a proper prologue for the backtracing code. However,
-mnop-mcount doesn't play well with 16-bit code generation for C code.
gcc happily emits a 5-byte NOP that transmutes to a 4-byte NOP followed
by a zero byte when executed in real mode, wrecking all code that
follows.

Fix that by selectively disabling '-mnop-mcount' for realmode.c, making
it call mcount(), which is provided as a stub function.

Reported-by: Sean Christopherson <seanjc@google.com>
Fixes: f01ea38a385a ("x86: Better backtraces for leaf functions")
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
---
x86/Makefile.common | 5 ++++-
x86/realmode.c | 3 +++
2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/x86/Makefile.common b/x86/Makefile.common
index be18a77a779e..6b59f26a3d2c 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -51,7 +51,9 @@ ifneq ($(KEEP_FRAME_POINTER),)
# We need to defer the cc-option test until -fno-pic or -no-pie have been
# added to CFLAGS as -mnop-mcount needs it. The lazy evaluation of CFLAGS
# during compilation makes this do "The Right Thing."
-LATE_CFLAGS += $(call cc-option, -pg -mnop-mcount, "")
+NOP_PGFLAGS := -pg -mnop-mcount
+LATE_CFLAGS += $(call cc-option, $(NOP_PGFLAGS), "")
+NO_NOP_MCOUNT = $(if $(filter $(NOP_PGFLAGS),$(LATE_CFLAGS)),-mno-nop-mcount)
endif

FLATLIBS = lib/libcflat.a
@@ -121,6 +123,7 @@ $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o $(SRCDIR)/$(TEST_DIR)/realmode.
-T $(SRCDIR)/$(TEST_DIR)/realmode.lds $(filter %.o, $^)

$(TEST_DIR)/realmode.o: bits = $(realmode_bits)
+$(TEST_DIR)/realmode.o: CFLAGS += $(NO_NOP_MCOUNT)

$(TEST_DIR)/access_test.$(bin): $(TEST_DIR)/access.o

diff --git a/x86/realmode.c b/x86/realmode.c
index 7a4423ec6098..0a7104d4cff4 100644
--- a/x86/realmode.c
+++ b/x86/realmode.c
@@ -23,6 +23,9 @@ void test_function(void);
asm(
"test_function: \n\t"
"mov $0x1234, %eax \n\t"
+ "ret\n\t"
+ /* mcount() stub */
+ "mcount:\n\t"
"ret"
);

--
2.47.3