This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos 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]

[APPROVE?] Fix MIPS and PowerPC with GCC 3.x


It appears that a problem we thought was fixed with GCC 3.x actually wasn't. Essentially the C++ ABI changed in GCC 3.x and GCC now reuses extraneous padding of base classes in following members of a derived class. This affects MIPS and PowerPC particularly for some reason.

e.g. class a {
  long long x;
  int y;
};

class b :public a
  int z;
};

is no longer the same size as the following in plain C:

struct a {
  long long x;
  int y;
};

struct b {
  struct a a1;
  int z;
};

As indeed the official C++ ABI says - there's nothing that permits a base class to be mapped to a single struct member in C. Unfortunately we were relying on this in the mapping between the kernel definitions for C in kapidata.h and the real C++ definitions. This caused an assertion failure.

We knew about this before and this is what caused the horrid munging of kapidata.h.... the fix was to make every struct be "flat", e.g. in the above:

struct b {
  long long x;
  int y;
  int z;
};

With this, GCC behaves identically in C and C++. However this broke GCC 2.x targets. So the attached patch adds a simple check to DTRT. And add some documentation to explain the obfuscation.

Checked in to trunk and definitely wants approval for branch.

If anyone has any suggestions on how to resolve this compatibility without the munging of kapidata.h I'd be all ears!

Jifl
--
eCosCentric    http://www.eCosCentric.com/    The eCos and RedBoot experts
--[ "You can complain because roses have thorns, or you ]--
--[  can rejoice because thorns have roses." -Lincoln   ]-- Opinions==mine
Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/ChangeLog,v
retrieving revision 1.97
diff -u -5 -p -r1.97 ChangeLog
--- ChangeLog	25 Feb 2003 18:42:00 -0000	1.97
+++ ChangeLog	27 Feb 2003 06:04:20 -0000
@@ -1,5 +1,11 @@
+2003-02-27  Jonathan Larmour  <jifl at eCosCentric dot com>
+
+	* include/kapidata.h: Revert change of 2001-08-23 and instead make
+	it conditional on the GCC version. Also add comments explaining why
+	this file has been apparently obfuscated.
+
 2003-02-25  Nick Garnett  <nickg at calivar dot com>
 
 	* tests/fptest.c (alarm_fn): Added CYG_TEST_PASS() call to keep
 	configtool happy.
 
Index: include/kapidata.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/include/kapidata.h,v
retrieving revision 1.12
diff -u -5 -p -r1.12 kapidata.h
--- include/kapidata.h	5 Aug 2002 21:53:26 -0000	1.12
+++ include/kapidata.h	27 Feb 2003 06:04:21 -0000
@@ -53,10 +53,28 @@
 //              configuration and must be kept in step with their real
 //              counterparts in the C++ headers.
 //              IMPORTANT: It is NOT guaranteed that the fields of these
 //              structures correspond to the equivalent fields in the
 //              C++ classes they shadow.
+//
+//              One oddity with this file is that the way many of the "mirror"
+//              classes are defined with macros. The resulting structures
+//              then have a "flat" layout, rather than just declaring a
+//              member structure directly in the structure. The reason for
+//              this is that as of GCC 3.x, the C++ compiler will optimise
+//              classes by removing padding and reusing it for subsequent
+//              members defined in a derived class. This affects some targets
+//              (including PowerPC and MIPS at least) when a C++ base class
+//              includes a long long. By instead arranging for the C structure
+//              to just list all the members directly, the compiler will then
+//              behave the same for the C structures as the C++ classes.
+//
+//              This means that care has to be taken to follow the same
+//              methodology if new stuff is added to this file. Even if
+//              it doesn't contain long longs for your target, it may for
+//              others, depending on HAL definitions.
+//
 // Usage:       included by kapi.h
 //
 //####DESCRIPTIONEND####
 //
 //==========================================================================*/
@@ -337,13 +355,23 @@ typedef struct
 typedef struct 
 {
     CYG_SCHEDTHREAD_MEMBERS
 } cyg_schedthread;
 
+/* This compiler version test is required because the C++ ABI changed in
+   GCC v3.x and GCC could now reuse "spare" space from base classes in derived
+   classes, and in C++ land, cyg_alarm is a base class of cyg_threadtimer.
+*/
+#if defined(__GNUC__) && (__GNUC__ < 3)
 #define CYG_THREADTIMER_MEMBERS \
     cyg_alarm           alarm;  \
     cyg_thread          *thread;
+#else
+#define CYG_THREADTIMER_MEMBERS \
+    CYG_ALARM_MEMBERS           \
+    cyg_thread          *thread;
+#endif
 
 /*---------------------------------------------------------------------------*/
 /* Thread structure                                                          */
 
 typedef struct 

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