[patch 4/4] unwinder: s390 and s390x
Mark Wielaard
mjw at redhat.com
Wed Dec 18 16:06:29 UTC 2013
On Tue, 2013-12-17 at 19:05 +0100, Jan Kratochvil wrote:
> > > + /* Avoid conversion double -> integer. */
> > > + eu_static_assert (sizeof user_regs.regs.fp_regs.fprs[0]
> > > + == sizeof state->regs[0]);
> > > + for (unsigned u = 0; u < 16; u++)
> > > + dwarf_regs[u] = *((const __typeof (*state->regs) *)
> > > + &user_regs.regs.fp_regs.fprs[u]));
> > > + if (! setfunc (0, 16, dwarf_regs, arg))
> > > + return false;
>
> > > + for (unsigned u = 0; u < 16; u++)
> > > + dwarf_frame_state_reg_set (state, 16 + u,
> > > + *((const __typeof (*state->regs) *)
> > > + &user_regs.regs.fp_regs.fprs[u]));
> >
> > I don't understand this. What is the intention?
>
> This second block is a leftover, deleted. Sorry I apparently forgot to
> re-test the patch on native s390* after the last changes.
>
> Sorry if obvious but if you ask about the cast it is stated above:
> /* Avoid conversion double -> integer. */
> because:
> /usr/include/sys/user.h
> struct _user_fpregs_struct
> {
> unsigned int fpc;
> double fprs[16];
> };
The question was indeed about the second block. I thought I understood
the casting, but apparently the compiler (gcc 4.8.2) doesn't...
gcc -D_GNU_SOURCE -DHAVE_CONFIG_H -DLOCALEDIR='"/usr/local/share/locale"' -I. -I/root/elfutils/backends -I.. -I. -I/root/elfutils/backends -I/root/elfutils/lib -I.. -I/root/elfutils/libebl -I/root/elfutils/libasm -I/root/elfutils/libelf -I/root/elfutils/libdw -std=gnu99 -Wall -Wshadow -Werror -Wunused -Wextra -Wformat=2 -g -O2 -MT s390_initreg.o -MD -MP -MF .deps/s390_initreg.Tpo -c -o s390_initreg.o /root/elfutils/backends/s390_initreg.c
/root/elfutils/backends/s390_initreg.c: In function ‘s390_set_initial_registers_tid’:
/root/elfutils/backends/s390_initreg.c:72:9: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
&user_regs.regs.fp_regs.fprs[u]);
^
cc1: all warnings being treated as errors
make[2]: *** [s390_initreg.o] Error 1
Should we use something a bit more explicit to help the compiler?
For example:
diff --git a/backends/s390_initreg.c b/backends/s390_initreg.c
index 62a1531..d4a9051 100644
--- a/backends/s390_initreg.c
+++ b/backends/s390_initreg.c
@@ -68,8 +68,15 @@ s390_set_initial_registers_tid (pid_t tid __attribute__ ((unused)),
eu_static_assert (sizeof user_regs.regs.fp_regs.fprs[0]
== sizeof dwarf_regs[0]);
for (unsigned u = 0; u < 16; u++)
- dwarf_regs[u] = *((const __typeof (dwarf_regs[0]) *)
- &user_regs.regs.fp_regs.fprs[u]);
+ {
+ // Store the double bits as is in the Dwarf_Word without conversion.
+ union
+ {
+ double d;
+ Dwarf_Word w;
+ } fpr = { .d = user_regs.regs.fp_regs.fprs[u] };
+ dwarf_regs[u] = fpr.w;
+ }
if (! setfunc (16, 16, dwarf_regs, arg))
return false;
dwarf_regs[0] = user_regs.regs.psw.addr;
More information about the elfutils-devel
mailing list