diff mbox

linux-user: Use correct alignment for long long on i386 guests

Message ID 1469707079-9049-1-git-send-email-peter.maydell@linaro.org
State New
Headers show

Commit Message

Peter Maydell July 28, 2016, 11:57 a.m. UTC
For i386, the ABI specifies that 'long long' (8 byte values)
need only be 4 aligned, but we were requiring them to be
8-aligned. This meant we were laying out the target_epoll_event
structure wrongly. Add a suitable ifdef to abitypes.h to
specify the i386-specific alignment requirement.

Reported-by: Icenowy Zheng <icenowy@aosc.xyz>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/exec/user/abitypes.h | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Laurent Vivier July 28, 2016, 7:19 p.m. UTC | #1
Le 28/07/2016 à 13:57, Peter Maydell a écrit :
> For i386, the ABI specifies that 'long long' (8 byte values)
> need only be 4 aligned, but we were requiring them to be
> 8-aligned. This meant we were laying out the target_epoll_event
> structure wrongly. Add a suitable ifdef to abitypes.h to
> specify the i386-specific alignment requirement.
> 
> Reported-by: Icenowy Zheng <icenowy@aosc.xyz>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/exec/user/abitypes.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/exec/user/abitypes.h b/include/exec/user/abitypes.h
> index a09d6c6..ba18860 100644
> --- a/include/exec/user/abitypes.h
> +++ b/include/exec/user/abitypes.h
> @@ -15,6 +15,10 @@
>  #define ABI_LLONG_ALIGNMENT 2
>  #endif
>  
> +#if defined(TARGET_I386) && !defined(TARGET_X86_64)
> +#define ABI_LLONG_ALIGNMENT 4
> +#endif
> +
>  #ifndef ABI_SHORT_ALIGNMENT
>  #define ABI_SHORT_ALIGNMENT 2
>  #endif
> 

Why the following program from commit

    c2e3dee linux-user: Define target alignment size

int main(void)
{
    printf("alignof(short) %ld\n", __alignof__(short));
    printf("alignof(int) %ld\n", __alignof__(int));
    printf("alignof(long) %ld\n", __alignof__(long));
    printf("alignof(long long) %ld\n", __alignof__(long long));
}


gives me:

alignof(short) 2
alignof(int) 4
alignof(long) 4
alignof(long long) 8

?

I compile it on x86_64 in 32bit mode:

cc -m32 -o align align.c
file align
align: ELF 32-bit LSB executable, Intel 80386, version 1 (SYSV),
dynamically linked, interpreter /lib/ld-linux.so.2, for GNU/Linux
2.6.32, BuildID[sha1]=f5312f2334462b48fd9764e78a845879ff317b94, not stripped

Thanks,
Laurent
Peter Maydell July 28, 2016, 8:38 p.m. UTC | #2
On 28 July 2016 at 20:19, Laurent Vivier <laurent@vivier.eu> wrote:
> Why the following program from commit
>
>     c2e3dee linux-user: Define target alignment size
>
> int main(void)
> {
>     printf("alignof(short) %ld\n", __alignof__(short));
>     printf("alignof(int) %ld\n", __alignof__(int));
>     printf("alignof(long) %ld\n", __alignof__(long));
>     printf("alignof(long long) %ld\n", __alignof__(long long));
> }
>
>
> gives me:
>
> alignof(short) 2
> alignof(int) 4
> alignof(long) 4
> alignof(long long) 8
>
> ?

Because gcc __alignof__ gives you the maximum preferred
alignment, not the minimum alignment. If you want to know
what the alignment used for setting struct alignment is,
use C99 alignof from stdalign.h and/or cross-check by
using sizeof() on a struct with the type in it:

orth$ uname -a
Linux orth 3.16.0-4-686-pae #1 SMP Debian 3.16.7-ckt20-1+deb8u3
(2016-01-17) i686 GNU/Linux
orth$ cat /tmp/zz9.c
#include <stdio.h>
#include <stdalign.h>

struct epoll_event {
  int events;
  unsigned long long data;
};

#define PRINTALIGN(T) \
    printf(#T ": sizeof %zd __alignof__ %zd _Alignof %zd alignof %zd\n", \
    sizeof(T), __alignof__(T), _Alignof(T), alignof(T))

int main(void) {
    PRINTALIGN(unsigned long long);
    PRINTALIGN(struct epoll_event);
    return 0;
}
orth$ gcc -g -Wall -o /tmp/zz9 /tmp/zz9.c
orth$ /tmp/zz9
unsigned long long: sizeof 8 __alignof__ 8 _Alignof 4 alignof 4
struct epoll_event: sizeof 12 __alignof__ 4 _Alignof 4 alignof 4

Also watch out for broken clang versions:

orth$ clang -g -Wall -o /tmp/zz9clang /tmp/zz9.c
orth$ /tmp/zz9clang
unsigned long long: sizeof 8 __alignof__ 8 _Alignof 8 alignof 8
struct epoll_event: sizeof 12 __alignof__ 4 _Alignof 4 alignof 4
orth$ clang --version
Debian clang version 3.5.0-10 (tags/RELEASE_350/final) (based on LLVM 3.5.0)
Target: i386-pc-linux-gnu
Thread model: posix

(that's fixed in later clang versions)

thanks
-- PMM
Laurent Vivier July 29, 2016, 12:13 a.m. UTC | #3
Le 28/07/2016 à 13:57, Peter Maydell a écrit :
> For i386, the ABI specifies that 'long long' (8 byte values)
> need only be 4 aligned, but we were requiring them to be
> 8-aligned. This meant we were laying out the target_epoll_event
> structure wrongly. Add a suitable ifdef to abitypes.h to
> specify the i386-specific alignment requirement.
> 
> Reported-by: Icenowy Zheng <icenowy@aosc.xyz>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/exec/user/abitypes.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/exec/user/abitypes.h b/include/exec/user/abitypes.h
> index a09d6c6..ba18860 100644
> --- a/include/exec/user/abitypes.h
> +++ b/include/exec/user/abitypes.h
> @@ -15,6 +15,10 @@
>  #define ABI_LLONG_ALIGNMENT 2
>  #endif
>  
> +#if defined(TARGET_I386) && !defined(TARGET_X86_64)
> +#define ABI_LLONG_ALIGNMENT 4
> +#endif
> +
>  #ifndef ABI_SHORT_ALIGNMENT
>  #define ABI_SHORT_ALIGNMENT 2
>  #endif
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
diff mbox

Patch

diff --git a/include/exec/user/abitypes.h b/include/exec/user/abitypes.h
index a09d6c6..ba18860 100644
--- a/include/exec/user/abitypes.h
+++ b/include/exec/user/abitypes.h
@@ -15,6 +15,10 @@ 
 #define ABI_LLONG_ALIGNMENT 2
 #endif
 
+#if defined(TARGET_I386) && !defined(TARGET_X86_64)
+#define ABI_LLONG_ALIGNMENT 4
+#endif
+
 #ifndef ABI_SHORT_ALIGNMENT
 #define ABI_SHORT_ALIGNMENT 2
 #endif