Re: PowerPC - more warnings cleaned up

[Jonathan Larmour <jifl at eCosCentric dot com>]

Gary Thomas wrote:
> ------------------------------------------------------------------------
>
> Index: hal/powerpc/arch/current/ChangeLog
> ===================================================================
> RCS file: /misc/cvsfiles/ecos/packages/hal/powerpc/arch/current/ChangeLog,v
> retrieving revision 1.62
> diff -u -5 -p -r1.62 ChangeLog
> --- hal/powerpc/arch/current/ChangeLog	23 Apr 2004 20:55:32 -0000	1.62
> +++ hal/powerpc/arch/current/ChangeLog	23 Apr 2004 21:10:38 -0000
> @@ -1,7 +1,8 @@
>  2004-04-23  Gary Thomas  <gary@mlbassoc.com>
>  
> +	* include/hal_arch.h: 
>  	* src/redboot_linux_exec.c (do_exec): Fix compiler warning about bad
>  	cast (dereferencing type-punned pointer will break strict-aliasing rules)

This may silence the warning, but not necessarily comply with strict 
aliasing rules. The GCC folks have said that aliasing problems can't always 
be detected. I still have the scars on my back from dealing with libm to 
understand that. You have to go through the tedious motions if casting 
between pointers of potentially different types.

The redboot_linux_exec.c change is okay I believe as that just makes the 
function match the prototype. include/hal_arch.h should be different 
because HAL_SavedRegisters doesn't necessarily have the same layout as 
GDB_Registers.

This is therefore a more reliable change. I don't think we need to be shy 
of GNUisms in HAL code BTW, and arguably there's no point fighting a losing 
battle anyway.

Jifl

Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/powerpc/arch/current/ChangeLog,v
retrieving revision 1.63
diff -u -5 -p -r1.63 ChangeLog
--- ChangeLog	23 Apr 2004 21:11:59 -0000	1.63
+++ ChangeLog	29 Apr 2004 03:38:48 -0000
@@ -1,5 +1,11 @@
+2004-04-29  Jonathan Larmour  <jifl@eCosCentric.com>
+
+	* include/hal_arch.h (HAL_SET_GDB_FLOATING_POINT_REGISTERS): Make
+	absolutely safe with respect to strict-aliasing.
+	(HAL_SET_GDB_FLOATING_POINT_REGISTERS): Ditto.
+
 2004-04-23  Gary Thomas  <gary@mlbassoc.com>

 	* include/hal_arch.h:
 	* src/redboot_linux_exec.c (do_exec): Fix compiler warning about bad
 	cast (dereferencing type-punned pointer will break strict-aliasing rules)
Index: include/hal_arch.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/powerpc/arch/current/include/hal_arch.h,v
retrieving revision 1.14
diff -u -5 -p -r1.14 hal_arch.h
--- include/hal_arch.h	23 Apr 2004 21:11:59 -0000	1.14
+++ include/hal_arch.h	29 Apr 2004 03:38:48 -0000
@@ -11,10 +11,11 @@
 //####ECOSGPLCOPYRIGHTBEGIN####
 // -------------------------------------------
 // This file is part of eCos, the Embedded Configurable Operating System.
 // Copyright (C) 1998, 1999, 2000, 2001, 2002 Red Hat, Inc.
 // Copyright (C) 2004 Gary Thomas
+// Copyright (C) 2004 Jonathan Larmour <jifl@eCosCentric.com>
 //
 // eCos is free software; you can redistribute it and/or modify it under
 // the terms of the GNU General Public License as published by the Free
 // Software Foundation; either version 2 or (at your option) any later 
version.
 //
@@ -268,11 +269,16 @@ typedef struct {
 	
 // Copy a set of registers from a HAL_SavedRegisters structure into a
 // GDB ordered array.
 #define HAL_GET_GDB_REGISTERS( _aregval_, _regs_ )              \
     CYG_MACRO_START                                             \
-    GDB_Registers *_gdb_ = (GDB_Registers *)((void *)(_aregval_));      \
+    union __gdbreguniontype {                                   \
+      __typeof__(_aregval_) _aregval2_;                         \
+      GDB_Registers *_gdbr;                                     \
+    } __gdbregunion;                                            \
+    __gdbregunion._aregval2_ = (_aregval_);                     \
+    GDB_Registers *_gdb_ = __gdbregunion._gdbr;                 \
     int _i_;                                                    \
                                                                 \
     for( _i_ = 0; _i_ < 32; _i_++ )                             \
         _gdb_->gpr[_i_] = (_regs_)->d[_i_];                     \
                                                                 \
@@ -286,11 +292,16 @@ typedef struct {
     CYG_MACRO_END

 // Copy a GDB ordered array into a HAL_SavedRegisters structure.
 #define HAL_SET_GDB_REGISTERS( _regs_ , _aregval_ )             \
     CYG_MACRO_START                                             \
-    GDB_Registers *_gdb_ = (GDB_Registers *)((void *)(_aregval_));      \
+    union __gdbreguniontype {                                   \
+      __typeof__(_aregval_) _aregval2_;                         \
+      GDB_Registers *_gdbr;                                     \
+    } __gdbregunion;                                            \
+    __gdbregunion._aregval2_ = (_aregval_);                     \
+    GDB_Registers *_gdb_ = __gdbregunion._gdbr;                 \
     int _i_;                                                    \
                                                                 \
     for( _i_ = 0; _i_ < 32; _i_++ )                             \
         (_regs_)->d[_i_] = _gdb_->gpr[_i_];                     \
                                                                 \


--
eCosCentric    http://www.eCosCentric.com/    The eCos and RedBoot experts
--["No sense being pessimistic, it wouldn't work anyway"]-- Opinions==mine