* [Qemu-devel] [PATCH] linux-user: remove useless padding in flock64 structure
@ 2018-05-01 23:53 Laurent Vivier
2018-05-02 3:27 ` Richard Henderson
2018-05-02 14:52 ` Laurent Vivier
0 siblings, 2 replies; 3+ messages in thread
From: Laurent Vivier @ 2018-05-01 23:53 UTC (permalink / raw)
To: qemu-devel; +Cc: Riku Voipio, Laurent Vivier
Since commit 8efb2ed5ec ("linux-user: Correct signedness of
target_flock l_start and l_len fields"), flock64 structure uses
abi_llong for l_start and l_len in place of "unsigned long long"
this should force them to be aligned accordingly to the target
rules. So we can remove the padding field and the QEMU_PACKED
attribute.
I have compared the result of the following program before and
after the change:
cat -> flock64_dump <<EOF
p/d sizeof(struct target_flock64)
p/d &((struct target_flock64 *)0)->l_type
p/d &((struct target_flock64 *)0)->l_whence
p/d &((struct target_flock64 *)0)->l_start
p/d &((struct target_flock64 *)0)->l_len
p/d &((struct target_flock64 *)0)->l_pid
quit
EOF
for file in build/all/*-linux-user/qemu-* ; do
echo $file
gdb -batch -nx -x flock64_dump $file 2> /dev/null
done
The sizeof() changes because we remove the QEMU_PACKED.
The new size is 32 (except for i386 and m68k) and this is
the real size of "struct flock64" on the target architecture.
The following architectures differ:
aarch64_be, aarch64, alpha, armeb, arm, cris, hppa, nios2, or1k,
riscv32, riscv64, s390x.
For a subset of these architectures, I have checked with the following
program the new structure is the correct one:
#include <stdio.h>
#define __USE_LARGEFILE64
#include <fcntl.h>
int main(void)
{
printf("struct flock64 %d\n", sizeof(struct flock64));
printf("l_type %d\n", &((struct flock64 *)0)->l_type);
printf("l_whence %d\n", &((struct flock64 *)0)->l_whence);
printf("l_start %d\n", &((struct flock64 *)0)->l_start);
printf("l_len %d\n", &((struct flock64 *)0)->l_len);
printf("l_pid %d\n", &((struct flock64 *)0)->l_pid);
}
[I have checked aarch64, alpha, arm, s390x]
I have also fixed the alignment value for sh4 (to align llong on 4 bytes)
(see c2e3dee6e0 "linux-user: Define target alignment size")
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
include/exec/user/abitypes.h | 2 +-
linux-user/syscall_defs.h | 23 ++++++++---------------
2 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/include/exec/user/abitypes.h b/include/exec/user/abitypes.h
index ba188608c2..743b8bb9ea 100644
--- a/include/exec/user/abitypes.h
+++ b/include/exec/user/abitypes.h
@@ -15,7 +15,7 @@
#define ABI_LLONG_ALIGNMENT 2
#endif
-#if defined(TARGET_I386) && !defined(TARGET_X86_64)
+#if (defined(TARGET_I386) && !defined(TARGET_X86_64)) || defined(TARGET_SH4)
#define ABI_LLONG_ALIGNMENT 4
#endif
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 23f5bccf0e..df3847bc16 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2649,28 +2649,21 @@ struct target_flock {
};
struct target_flock64 {
- short l_type;
- short l_whence;
-#if defined(TARGET_PPC) || defined(TARGET_X86_64) || defined(TARGET_MIPS) \
- || defined(TARGET_SPARC) || defined(TARGET_HPPA) \
- || defined(TARGET_MICROBLAZE) || defined(TARGET_TILEGX) \
- || defined(TARGET_XTENSA)
- int __pad;
-#endif
+ abi_short l_type;
+ abi_short l_whence;
abi_llong l_start;
abi_llong l_len;
- int l_pid;
-} QEMU_PACKED;
+ abi_int l_pid;
+};
#ifdef TARGET_ARM
struct target_eabi_flock64 {
- short l_type;
- short l_whence;
- int __pad;
+ abi_short l_type;
+ abi_short l_whence;
abi_llong l_start;
abi_llong l_len;
- int l_pid;
-} QEMU_PACKED;
+ abi_int l_pid;
+};
#endif
struct target_f_owner_ex {
--
2.14.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: remove useless padding in flock64 structure
2018-05-01 23:53 [Qemu-devel] [PATCH] linux-user: remove useless padding in flock64 structure Laurent Vivier
@ 2018-05-02 3:27 ` Richard Henderson
2018-05-02 14:52 ` Laurent Vivier
1 sibling, 0 replies; 3+ messages in thread
From: Richard Henderson @ 2018-05-02 3:27 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: Riku Voipio
On 05/01/2018 04:53 PM, Laurent Vivier wrote:
> Since commit 8efb2ed5ec ("linux-user: Correct signedness of
> target_flock l_start and l_len fields"), flock64 structure uses
> abi_llong for l_start and l_len in place of "unsigned long long"
> this should force them to be aligned accordingly to the target
> rules. So we can remove the padding field and the QEMU_PACKED
> attribute.
>
> I have compared the result of the following program before and
> after the change:
>
> cat -> flock64_dump <<EOF
> p/d sizeof(struct target_flock64)
> p/d &((struct target_flock64 *)0)->l_type
> p/d &((struct target_flock64 *)0)->l_whence
> p/d &((struct target_flock64 *)0)->l_start
> p/d &((struct target_flock64 *)0)->l_len
> p/d &((struct target_flock64 *)0)->l_pid
> quit
> EOF
>
> for file in build/all/*-linux-user/qemu-* ; do
> echo $file
> gdb -batch -nx -x flock64_dump $file 2> /dev/null
> done
>
> The sizeof() changes because we remove the QEMU_PACKED.
> The new size is 32 (except for i386 and m68k) and this is
> the real size of "struct flock64" on the target architecture.
>
> The following architectures differ:
> aarch64_be, aarch64, alpha, armeb, arm, cris, hppa, nios2, or1k,
> riscv32, riscv64, s390x.
>
> For a subset of these architectures, I have checked with the following
> program the new structure is the correct one:
>
> #include <stdio.h>
> #define __USE_LARGEFILE64
> #include <fcntl.h>
>
> int main(void)
> {
> printf("struct flock64 %d\n", sizeof(struct flock64));
> printf("l_type %d\n", &((struct flock64 *)0)->l_type);
> printf("l_whence %d\n", &((struct flock64 *)0)->l_whence);
> printf("l_start %d\n", &((struct flock64 *)0)->l_start);
> printf("l_len %d\n", &((struct flock64 *)0)->l_len);
> printf("l_pid %d\n", &((struct flock64 *)0)->l_pid);
> }
>
> [I have checked aarch64, alpha, arm, s390x]
>
> I have also fixed the alignment value for sh4 (to align llong on 4 bytes)
> (see c2e3dee6e0 "linux-user: Define target alignment size")
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: remove useless padding in flock64 structure
2018-05-01 23:53 [Qemu-devel] [PATCH] linux-user: remove useless padding in flock64 structure Laurent Vivier
2018-05-02 3:27 ` Richard Henderson
@ 2018-05-02 14:52 ` Laurent Vivier
1 sibling, 0 replies; 3+ messages in thread
From: Laurent Vivier @ 2018-05-02 14:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Riku Voipio, Richard Henderson
Le 02/05/2018 à 01:53, Laurent Vivier a écrit :
> Since commit 8efb2ed5ec ("linux-user: Correct signedness of
> target_flock l_start and l_len fields"), flock64 structure uses
> abi_llong for l_start and l_len in place of "unsigned long long"
> this should force them to be aligned accordingly to the target
> rules. So we can remove the padding field and the QEMU_PACKED
> attribute.
>
> I have compared the result of the following program before and
> after the change:
>
> cat -> flock64_dump <<EOF
> p/d sizeof(struct target_flock64)
> p/d &((struct target_flock64 *)0)->l_type
> p/d &((struct target_flock64 *)0)->l_whence
> p/d &((struct target_flock64 *)0)->l_start
> p/d &((struct target_flock64 *)0)->l_len
> p/d &((struct target_flock64 *)0)->l_pid
> quit
> EOF
>
> for file in build/all/*-linux-user/qemu-* ; do
> echo $file
> gdb -batch -nx -x flock64_dump $file 2> /dev/null
> done
>
> The sizeof() changes because we remove the QEMU_PACKED.
> The new size is 32 (except for i386 and m68k) and this is
> the real size of "struct flock64" on the target architecture.
>
> The following architectures differ:
> aarch64_be, aarch64, alpha, armeb, arm, cris, hppa, nios2, or1k,
> riscv32, riscv64, s390x.
>
> For a subset of these architectures, I have checked with the following
> program the new structure is the correct one:
>
> #include <stdio.h>
> #define __USE_LARGEFILE64
> #include <fcntl.h>
>
> int main(void)
> {
> printf("struct flock64 %d\n", sizeof(struct flock64));
> printf("l_type %d\n", &((struct flock64 *)0)->l_type);
> printf("l_whence %d\n", &((struct flock64 *)0)->l_whence);
> printf("l_start %d\n", &((struct flock64 *)0)->l_start);
> printf("l_len %d\n", &((struct flock64 *)0)->l_len);
> printf("l_pid %d\n", &((struct flock64 *)0)->l_pid);
> }
>
> [I have checked aarch64, alpha, arm, s390x]
>
> I have also fixed the alignment value for sh4 (to align llong on 4 bytes)
> (see c2e3dee6e0 "linux-user: Define target alignment size")
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
> include/exec/user/abitypes.h | 2 +-
> linux-user/syscall_defs.h | 23 ++++++++---------------
> 2 files changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/include/exec/user/abitypes.h b/include/exec/user/abitypes.h
> index ba188608c2..743b8bb9ea 100644
> --- a/include/exec/user/abitypes.h
> +++ b/include/exec/user/abitypes.h
> @@ -15,7 +15,7 @@
> #define ABI_LLONG_ALIGNMENT 2
> #endif
>
> -#if defined(TARGET_I386) && !defined(TARGET_X86_64)
> +#if (defined(TARGET_I386) && !defined(TARGET_X86_64)) || defined(TARGET_SH4)
> #define ABI_LLONG_ALIGNMENT 4
> #endif
>
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 23f5bccf0e..df3847bc16 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -2649,28 +2649,21 @@ struct target_flock {
> };
>
> struct target_flock64 {
> - short l_type;
> - short l_whence;
> -#if defined(TARGET_PPC) || defined(TARGET_X86_64) || defined(TARGET_MIPS) \
> - || defined(TARGET_SPARC) || defined(TARGET_HPPA) \
> - || defined(TARGET_MICROBLAZE) || defined(TARGET_TILEGX) \
> - || defined(TARGET_XTENSA)
> - int __pad;
> -#endif
> + abi_short l_type;
> + abi_short l_whence;
> abi_llong l_start;
> abi_llong l_len;
> - int l_pid;
> -} QEMU_PACKED;
> + abi_int l_pid;
> +};
>
> #ifdef TARGET_ARM
> struct target_eabi_flock64 {
> - short l_type;
> - short l_whence;
> - int __pad;
> + abi_short l_type;
> + abi_short l_whence;
> abi_llong l_start;
> abi_llong l_len;
> - int l_pid;
> -} QEMU_PACKED;
> + abi_int l_pid;
> +};
> #endif
>
> struct target_f_owner_ex {
>
I think my change breaks the ARM OABI: in QEMU, OABI uses target_flock64
and in kernel it uses oabi_flock64 which uses "__attribute__
((packed,aligned(4)));".
While in kernel EABI uses the standard flock64 structure and in QEMU
arm_eabi_flock64...
Thanks,
Laurent
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-05-02 14:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-01 23:53 [Qemu-devel] [PATCH] linux-user: remove useless padding in flock64 structure Laurent Vivier
2018-05-02 3:27 ` Richard Henderson
2018-05-02 14:52 ` Laurent Vivier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.