1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] multiboot: make tests work with clang

Anatol Pomozov posted 1 patch 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170823192244.18914-1-anatol.pomozov@gmail.com
Test FreeBSD passed
Test checkpatch passed
Test docker passed
Test s390x passed
tests/multiboot/Makefile | 2 +-
tests/multiboot/libc.c   | 9 +++++++++
tests/multiboot/libc.h   | 2 ++
3 files changed, 12 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] multiboot: make tests work with clang
Posted by Anatol Pomozov 6 years, 8 months ago 
 * explicitly disable SSE as clang enables it by default for 32bit code
 * add memset() implementation. Clang complains that the function is
   absent, gcc seems provides default built-in.
---
 tests/multiboot/Makefile | 2 +-
 tests/multiboot/libc.c   | 9 +++++++++
 tests/multiboot/libc.h   | 2 ++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
index 856e76682a..7fadbca33a 100644
--- a/tests/multiboot/Makefile
+++ b/tests/multiboot/Makefile
@@ -1,5 +1,5 @@
 CC=gcc
-CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin
+CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin -mno-sse
 ASFLAGS=-m32
 
 LD=ld
diff --git a/tests/multiboot/libc.c b/tests/multiboot/libc.c
index 6df9bda96d..512fccd7fa 100644
--- a/tests/multiboot/libc.c
+++ b/tests/multiboot/libc.c
@@ -33,6 +33,15 @@ void* memcpy(void *dest, const void *src, int n)
 
     return dest;
 }
+void *memset(void *s, int c, size_t n)
+{
+    size_t i;
+    char *d = s;
+    for (i = 0; i < n; i++) {
+        *d++ = c;
+    }
+    return s;
+}
 
 static void print_char(char c)
 {
diff --git a/tests/multiboot/libc.h b/tests/multiboot/libc.h
index 04c9922c27..44b71350dd 100644
--- a/tests/multiboot/libc.h
+++ b/tests/multiboot/libc.h
@@ -36,6 +36,7 @@ typedef signed short int16_t;
 typedef signed char int8_t;
 
 typedef uint32_t uintptr_t;
+typedef uint32_t size_t;
 
 
 /* stdarg.h */
@@ -58,5 +59,6 @@ static inline void outb(uint16_t port, uint8_t data)
 
 void printf(const char *fmt, ...);
 void* memcpy(void *dest, const void *src, int n);
+void *memset(void *s, int c, size_t n);
 
 #endif
-- 
2.14.1.342.g6490525c54-goog
Re: [Qemu-devel] [PATCH] multiboot: make tests work with clang
Posted by Philippe Mathieu-Daudé 6 years, 8 months ago 
Hi Anatol,

On 08/23/2017 04:22 PM, Anatol Pomozov wrote:
>   * explicitly disable SSE as clang enables it by default for 32bit code
>   * add memset() implementation. Clang complains that the function is
>     absent, gcc seems provides default built-in.
> ---
>   tests/multiboot/Makefile | 2 +-
>   tests/multiboot/libc.c   | 9 +++++++++
>   tests/multiboot/libc.h   | 2 ++
>   3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
> index 856e76682a..7fadbca33a 100644
> --- a/tests/multiboot/Makefile
> +++ b/tests/multiboot/Makefile
> @@ -1,5 +1,5 @@
>   CC=gcc
> -CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin
> +CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin -mno-sse

What about using -march=i586 instead? so no need to disable each next 
features clang decide to default enable.

Even cleaner IMHO, use -march=pentium in CFLAGS and add "-cpu pentium" 
in run_test.sh so we are sure the generated code matches, what do you think?

Regards,

Phil.

>   ASFLAGS=-m32
>   
>   LD=ld
> diff --git a/tests/multiboot/libc.c b/tests/multiboot/libc.c
> index 6df9bda96d..512fccd7fa 100644
> --- a/tests/multiboot/libc.c
> +++ b/tests/multiboot/libc.c
> @@ -33,6 +33,15 @@ void* memcpy(void *dest, const void *src, int n)
>   
>       return dest;
>   }
> +void *memset(void *s, int c, size_t n)
> +{
> +    size_t i;
> +    char *d = s;
> +    for (i = 0; i < n; i++) {
> +        *d++ = c;
> +    }
> +    return s;
> +}
>   
>   static void print_char(char c)
>   {
> diff --git a/tests/multiboot/libc.h b/tests/multiboot/libc.h
> index 04c9922c27..44b71350dd 100644
> --- a/tests/multiboot/libc.h
> +++ b/tests/multiboot/libc.h
> @@ -36,6 +36,7 @@ typedef signed short int16_t;
>   typedef signed char int8_t;
>   
>   typedef uint32_t uintptr_t;
> +typedef uint32_t size_t;
>   
>   
>   /* stdarg.h */
> @@ -58,5 +59,6 @@ static inline void outb(uint16_t port, uint8_t data)
>   
>   void printf(const char *fmt, ...);
>   void* memcpy(void *dest, const void *src, int n);
> +void *memset(void *s, int c, size_t n);
>   
>   #endif
>
Re: [Qemu-devel] [PATCH] multiboot: make tests work with clang
Posted by Anatol Pomozov 6 years, 8 months ago 
On Wed, Aug 23, 2017 at 4:44 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Hi Anatol,
>
> On 08/23/2017 04:22 PM, Anatol Pomozov wrote:
>>
>>   * explicitly disable SSE as clang enables it by default for 32bit code
>>   * add memset() implementation. Clang complains that the function is
>>     absent, gcc seems provides default built-in.
>> ---
>>   tests/multiboot/Makefile | 2 +-
>>   tests/multiboot/libc.c   | 9 +++++++++
>>   tests/multiboot/libc.h   | 2 ++
>>   3 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
>> index 856e76682a..7fadbca33a 100644
>> --- a/tests/multiboot/Makefile
>> +++ b/tests/multiboot/Makefile
>> @@ -1,5 +1,5 @@
>>   CC=gcc
>> -CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc
>> -fno-builtin
>> +CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc
>> -fno-builtin -mno-sse
>
>
> What about using -march=i586 instead? so no need to disable each next
> features clang decide to default enable.
>
> Even cleaner IMHO, use -march=pentium in CFLAGS and add "-cpu pentium" in
> run_test.sh so we are sure the generated code matches, what do you think?

It sounds reasonable. Just sent an updated patch. Works with gcc 6.3
and clang 3.8.
[Qemu-devel] [PATCH] multiboot: make tests work with clang
Posted by Anatol Pomozov 6 years, 8 months ago 
 * clang 3.8 enables SSE even for 32bit code. Generate code for pentium
   CPU to make sure no new instructions are used.
 * add memset() implementation. Clang implements array zeroing in
   print_num() via memset() function call.
---
 tests/multiboot/Makefile    | 2 +-
 tests/multiboot/libc.c      | 9 +++++++++
 tests/multiboot/libc.h      | 2 ++
 tests/multiboot/run_test.sh | 1 +
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
index 856e76682a..2e015409dc 100644
--- a/tests/multiboot/Makefile
+++ b/tests/multiboot/Makefile
@@ -1,5 +1,5 @@
 CC=gcc
-CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin
+CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin -march=pentium
 ASFLAGS=-m32
 
 LD=ld
diff --git a/tests/multiboot/libc.c b/tests/multiboot/libc.c
index 6df9bda96d..512fccd7fa 100644
--- a/tests/multiboot/libc.c
+++ b/tests/multiboot/libc.c
@@ -33,6 +33,15 @@ void* memcpy(void *dest, const void *src, int n)
 
     return dest;
 }
+void *memset(void *s, int c, size_t n)
+{
+    size_t i;
+    char *d = s;
+    for (i = 0; i < n; i++) {
+        *d++ = c;
+    }
+    return s;
+}
 
 static void print_char(char c)
 {
diff --git a/tests/multiboot/libc.h b/tests/multiboot/libc.h
index 04c9922c27..44b71350dd 100644
--- a/tests/multiboot/libc.h
+++ b/tests/multiboot/libc.h
@@ -36,6 +36,7 @@ typedef signed short int16_t;
 typedef signed char int8_t;
 
 typedef uint32_t uintptr_t;
+typedef uint32_t size_t;
 
 
 /* stdarg.h */
@@ -58,5 +59,6 @@ static inline void outb(uint16_t port, uint8_t data)
 
 void printf(const char *fmt, ...);
 void* memcpy(void *dest, const void *src, int n);
+void *memset(void *s, int c, size_t n);
 
 #endif
diff --git a/tests/multiboot/run_test.sh b/tests/multiboot/run_test.sh
index f04e35cbf0..38dcfef42c 100755
--- a/tests/multiboot/run_test.sh
+++ b/tests/multiboot/run_test.sh
@@ -29,6 +29,7 @@ run_qemu() {
     printf %b "\n\n=== Running test case: $kernel $* ===\n\n" >> test.log
 
     $QEMU \
+        -cpu pentium \
         -kernel $kernel \
         -display none \
         -device isa-debugcon,chardev=stdio \
-- 
2.14.1.342.g6490525c54-goog
Re: [Qemu-devel] [PATCH] multiboot: make tests work with clang
Posted by Philippe Mathieu-Daudé 6 years, 8 months ago 
Hi Anatol,

Next time use 'v2' for an updated patch (for the automated tracking 
tools), also no need to link it to the previous Message-Id it looks more 
messy and can get lost.

On 08/23/2017 09:01 PM, Anatol Pomozov wrote:
>   * clang 3.8 enables SSE even for 32bit code. Generate code for pentium
>     CPU to make sure no new instructions are used.
>   * add memset() implementation. Clang implements array zeroing in
>     print_num() via memset() function call.
> ---
>   tests/multiboot/Makefile    | 2 +-
>   tests/multiboot/libc.c      | 9 +++++++++
>   tests/multiboot/libc.h      | 2 ++
>   tests/multiboot/run_test.sh | 1 +
>   4 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
> index 856e76682a..2e015409dc 100644
> --- a/tests/multiboot/Makefile
> +++ b/tests/multiboot/Makefile
> @@ -1,5 +1,5 @@
>   CC=gcc
> -CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin
> +CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin -march=pentium

Now that I remark it, I think -ffreestanding is a better flag to 
multiboot than -fno-builtin.

>   ASFLAGS=-m32
>   
>   LD=ld
> diff --git a/tests/multiboot/libc.c b/tests/multiboot/libc.c
> index 6df9bda96d..512fccd7fa 100644
> --- a/tests/multiboot/libc.c
> +++ b/tests/multiboot/libc.c
> @@ -33,6 +33,15 @@ void* memcpy(void *dest, const void *src, int n)
>   
>       return dest;
>   }
> +void *memset(void *s, int c, size_t n)
> +{
> +    size_t i;
> +    char *d = s;
> +    for (i = 0; i < n; i++) {
> +        *d++ = c;
> +    }
> +    return s;
> +}
>   
>   static void print_char(char c)
>   {
> diff --git a/tests/multiboot/libc.h b/tests/multiboot/libc.h
> index 04c9922c27..44b71350dd 100644
> --- a/tests/multiboot/libc.h
> +++ b/tests/multiboot/libc.h
> @@ -36,6 +36,7 @@ typedef signed short int16_t;
>   typedef signed char int8_t;
>   
>   typedef uint32_t uintptr_t;
> +typedef uint32_t size_t;
>   

I'd also add here:

_Static_assert(sizeof(void *) == sizeof(uint32_t), "Expecting a 32-bit 
arch");

Anyway:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>   
>   /* stdarg.h */
> @@ -58,5 +59,6 @@ static inline void outb(uint16_t port, uint8_t data)
>   
>   void printf(const char *fmt, ...);
>   void* memcpy(void *dest, const void *src, int n);
> +void *memset(void *s, int c, size_t n);
>   
>   #endif
> diff --git a/tests/multiboot/run_test.sh b/tests/multiboot/run_test.sh
> index f04e35cbf0..38dcfef42c 100755
> --- a/tests/multiboot/run_test.sh
> +++ b/tests/multiboot/run_test.sh
> @@ -29,6 +29,7 @@ run_qemu() {
>       printf %b "\n\n=== Running test case: $kernel $* ===\n\n" >> test.log
>   
>       $QEMU \
> +        -cpu pentium \
>           -kernel $kernel \
>           -display none \
>           -device isa-debugcon,chardev=stdio \
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.