realmode test fixes for clang

Clang prefers to use a "mempcy" (or equivalent) to copy the "regs" structure. This doesn't work in 16-bit mode, as it will end up copying over half the number of bytes. GCC performs a field-by-field copy of the structure, so force clang to do the same thing.

Signed-off-by Bill Wendling <[email protected]>

---
 x86/realmode.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

x86/realmode.c +3 -0

index 303d093..cf45fd6 100644
@@ -117,6 +117,19 @@ struct regs {
        u32 eip, eflags;
 };
 
+#define COPY_REG(name, dst, src) (dst).name = (src).name
+#define COPY_REGS(dst, src)                            \
+       COPY_REG(eax, dst, src);                        \

x86/realmode.c +3 -4

 struct table_descr {
        u16 limit;
        void *base;

@@ -148,11 +161,11 @@ static void exec_in_big_real_mode(struct insn_desc *insn)
        extern u8 test_insn[], test_insn_end[];
 
        for (i = 0; i < insn->len; ++i)
-           test_insn[i] = ((u8 *)(unsigned long)insn->ptr)[i];
+               test_insn[i] = ((u8 *)(unsigned long)insn->ptr)[i];
        for (; i < test_insn_end - test_insn; ++i)
                test_insn[i] = 0x90; // nop
 
-       save = inregs;
+       COPY_REGS(save, inregs);
        asm volatile(
                "lgdtl %[gdt_descr] \n\t"
                "mov %%cr0, %[tmp] \n\t"

@@ -196,7 +209,7 @@ static void exec_in_big_real_mode(struct insn_desc *insn)
                : [gdt_descr]"m"(gdt_descr), [bigseg]"r"((short)16)
                : "cc", "memory"
                );
-       outregs = save;
+       COPY_REGS(outregs, save);
 }
 
 #define R_AX 1
--

2.23.0.700.g56cf767bdb-goog

Clang prefers to use a "mempcy" (or equivalent) to copy the "regs" structure. This doesn't work in 16-bit mode, as it will end up copying over half the number of bytes. GCC performs a field-by-field copy of the structure, so force clang to do the same thing.

Signed-off-by Bill Wendling <[email protected]>

---
 x86/realmode.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

x86/realmode.c +16 -3

index 303d093..cf45fd6 100644
@@ -117,6 +117,19 @@ struct regs {
 	u32 eip, eflags;
 };
 
+#define COPY_REG(name, dst, src) (dst).name = (src).name
+#define COPY_REGS(dst, src)				\
+	COPY_REG(eax, dst, src);			\
+	COPY_REG(ebx, dst, src);			\
+	COPY_REG(ecx, dst, src);			\
+	COPY_REG(edx, dst, src);			\
+	COPY_REG(esi, dst, src);			\
+	COPY_REG(edi, dst, src);			\
+	COPY_REG(esp, dst, src);			\
+	COPY_REG(ebp, dst, src);			\
+	COPY_REG(eip, dst, src);			\
+	COPY_REG(eflags, dst, src)
+
 struct table_descr {
 	u16 limit;
 	void *base;

@@ -148,11 +161,11 @@ static void exec_in_big_real_mode(struct insn_desc *insn)
 	extern u8 test_insn[], test_insn_end[];
 
 	for (i = 0; i < insn->len; ++i)
-	    test_insn[i] = ((u8 *)(unsigned long)insn->ptr)[i];
+		test_insn[i] = ((u8 *)(unsigned long)insn->ptr)[i];
 	for (; i < test_insn_end - test_insn; ++i)
 		test_insn[i] = 0x90; // nop
 
-	save = inregs;
+	COPY_REGS(save, inregs);
 	asm volatile(
 		"lgdtl %[gdt_descr] \n\t"
 		"mov %%cr0, %[tmp] \n\t"

@@ -196,7 +209,7 @@ static void exec_in_big_real_mode(struct insn_desc *insn)
 		: [gdt_descr]"m"(gdt_descr), [bigseg]"r"((short)16)
 		: "cc", "memory"
 		);
-	outregs = save;
+	COPY_REGS(outregs, save);
 }
 
 #define R_AX 1

It's fragile to try to retrieve the stack pointer by taking the address of a variable on the stack. For instance, clang reserves more stack space than gcc here, indicating that the variable may not be at the start of the stack. Instead of relying upon this to work, retrieve the "%rbp" value, which contains the value of "%rsp" before stack allocation.

Signed-off-by Bill Wendling <[email protected]>

---
 x86/realmode.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

x86/realmode.c +0 -0

index cf45fd6..7c89dd1 100644
@@ -518,11 +518,12 @@ extern void retf_imm(void);
 

x86/realmode.c +0 -1

        MK_INSN(call1, "mov $test_function, %eax \n\t"
                       "call *%eax\n\t");
--

2.23.0.700.g56cf767bdb-goog

Quoted 24 lines · Bill Wendling

It's fragile to try to retrieve the stack pointer by taking the address of a variable on the stack. For instance, clang reserves more stack space than gcc here, indicating that the variable may not be at the start of the stack. Instead of relying upon this to work, retrieve the "%rbp" value, which contains the value of "%rsp" before stack allocation.

Signed-off-by Bill Wendling <[email protected]>

---
 x86/realmode.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

x86/realmode.c +0 -1

index cf45fd6..7c89dd1 100644
@@ -518,11 +518,12 @@ extern void retf_imm(void);
 
 static void test_call(void)
 {
-       u32 esp[16];
        u32 addr;
 

x86/realmode.c extern void retf_imm(void); · line 526

Quoted 5 lines

x86/realmode.c +3 -1

        inregs = (struct regs){ 0 };
-       inregs.esp = (u32)esp;
+
+       // At this point the original stack pointer is in %ebp.
+       asm volatile ("mov %%ebp, %0" : "=rm"(inregs.esp));

I don't think we should assume the existence of frame pointers. Moreover, I think %esp is actually the value that should be saved here, regardless of how large the current frame is.

Quoted 5 lines

x86/realmode.c +0 -1

        MK_INSN(call1, "mov $test_function, %eax \n\t"
                       "call *%eax\n\t");
--

2.23.0.700.g56cf767bdb-goog

Never mind. After taking a closer look, esp[] is meant to provide stack space for the code under test, but inregs.esp should point to the top of this stack rather than the bottom. This is apparently a long-standing bug, similar to the one Avi fixed for test_long_jmp() in commit 4aa22949 ("realmode: fix esp in long jump test").

For consistency with test_long_jmp, I'd suggest changing the inregs.esp assignment to:

       inregs.esp = (u32)(esp+16);

Note that you absolutely must preserve the esp[] array!

esp needs to point at the end of the stack, or it will corrupt memory.

Signed-off-by Bill Wendling <[email protected]>

x86/realmode.c static void test_call(void) · line 523

Quoted 10 lines · Bill Wendling

x86/realmode.c +1 -1

index 41b8592..f318910 100644
@@ -520,7 +520,7 @@ static void test_call(void)
      u32 addr;
 
      inregs = (struct regs){ 0 };
-     inregs.esp = (u32)esp;
+     inregs.esp = (u32)(esp+16);

Applied with

+ inregs.esp = (u32)&esp[ARRAY_SIZE(esp)];

Paolo