[Pixman] [PATCH 1/2] utils.c: On x86-32 unalign the stack before calling test_function

[Søren Sandmann]

Søren Sandmann <sandmann at cs.au.dk> writes:

> The trampoline is marked noinline to work around what appears to be a
> bug in GCC/OpenMP (and possibly later), where the test_function
> pointer is not correctly passed to the assembly if the function is
> inlined.

Turns out this was just because I had forgotten to mark the caller-save
registers as clobbered.


Søren


>From 764b97dd6016fa9f0286a64d7440a7de42b4818d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=B8ren=20Sandmann=20Pedersen?= <ssp at redhat.com>
Date: Wed, 2 Oct 2013 14:38:16 -0400
Subject: [PATCH 2/3] utils.c: On x86-32 unalign the stack before calling test_function

GCC when compiling with -msse2 and -mssse3 will assume that the stack
is aligned to 16 bytes even on x86-32 and accordingly issue movdqa
instructions for stack allocated variables.

But despite what GCC thinks, the standard ABI on x86-32 only requires
a 4-byte aligned stack. This is true at least on Windows, but there
also was (and maybe still is) Linux code in the wild that assumed
this. When such code calls into pixman and hits something compiled
with -msse2, we get a segfault from the unaligned movdqas.

Pixman has worked around this issue in the past with the gcc attribute
"force_align_arg_pointer" but the problem has resurfaced now in

    https://bugs.freedesktop.org/show_bug.cgi?id=68300

because pixman_composite_glyphs() is missing this attribute.

This patch makes fuzzer_test_main() call the test_function through a
trampoline, which, on x86-32, has a bit of assembly that deliberately
avoids aligning the stack to 16 bytes as GCC normally expects. The
result is that glyph-test now crashes.

V2: Mark caller-save registers as clobbered, rather than using
noinline on the trampoline.
---
 test/utils.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/test/utils.c b/test/utils.c
index 0cd982e..281f6b4 100644
--- a/test/utils.c
+++ b/test/utils.c
@@ -641,6 +641,32 @@ draw_checkerboard (pixman_image_t *image,
     }
 }
 
+static uint32_t
+call_test_function (uint32_t    (*test_function)(int testnum, int verbose),
+		    int		testnum,
+		    int		verbose)
+{
+    uint32_t retval;
+
+#if defined (__GNUC__) && (defined (__i386) || defined (__i386__))
+    __asm__ (
+	/* Deliberately avoid aligning the stack to 16 bytes */
+	"pushl	%1\n\t"
+	"pushl	%2\n\t"
+	"call	*%3\n\t"
+	"addl	$8, %%esp\n\t"
+	: "=a" (retval)
+	: "r" (verbose),
+	  "r" (testnum),
+	  "r" (test_function)
+	: "edx", "ecx"); /* caller save registers */
+#else
+    retval = test_function (testnum, verbose);
+#endif
+
+    return retval;
+}
+
 /*
  * A function, which can be used as a core part of the test programs,
  * intended to detect various problems with the help of fuzzing input
@@ -710,7 +736,9 @@ fuzzer_test_main (const char *test_name,
     else if (argc >= 2)
     {
 	n2 = atoi (argv[1]);
-	checksum = test_function (n2, 1);
+
+	checksum = call_test_function (test_function, n2, 1);
+
 	printf ("%d: checksum=%08X\n", n2, checksum);
 	return 0;
     }
@@ -726,7 +754,7 @@ fuzzer_test_main (const char *test_name,
 #endif
     for (i = n1; i <= n2; i++)
     {
-	uint32_t crc = test_function (i, 0);
+	uint32_t crc = call_test_function (test_function, i, 0);
 	if (verbose)
 	    printf ("%d: %08X\n", i, crc);
 	checksum += crc;
-- 
1.7.11.7