This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch 4/4] unwinder: s390 and s390x


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;



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]