RE: [PATCH] Re: STDCXX-1072 SPARC V8 mutex alignment requirements

2012-10-13 Thread Travis Vitek 

Liviu,

I built the library and tests before and after your change with the 64-bit 
flag, and I saw no differences in the number of failed tests between the 
builds. I've attached the output of 'gmake -k runall' before and after your 
change to STDCXX-1072 in case you want to look over them.

I wrote up the following test case to prove that I could reproduce the issue 
using the compile/link options used to build the stdcxx tests. The code is 
below..

  [vitek@andromeda] 138 % cat t.cpp
  #include thread.h
  #include synch.h
  #include malloc.h

  int main ()
  {
int *ip;
mutex_t *mp;

ip = (int*)malloc(sizeof (int) + sizeof (mutex_t));
mp = (mutex_t*)(ip + 1);

mutex_init(mp, USYNC_THREAD | LOCK_ROBUST, 0);
mutex_lock(mp);
mutex_unlock(mp);
mutex_destroy(mp);

free(ip);
  }
  [vitek@andromeda] 139 % gmake t
  CC -c -D_RWSTDDEBUG -mt -I/amd/homes/vitek/tmp/stdcxx-4.2.x/include \
-I/build/vitek/stdcxx-4.2.x_sunpro-5.8_sunos-5.10-patched/include \
-I/amd/homes/vitek/tmp/stdcxx-4.2.x/tests/include \
-library=%none -g  -m64 +w -errtags -erroff=hidef   t.cpp
  CC t.o -o t -L/build/vitek/stdcxx-4.2.x_sunpro-5.8_sunos-5.10-patched/rwtest \
-lrwtest15S -library=%none -mt -m64 \
-L/build/vitek/stdcxx-4.2.x_sunpro-5.8_sunos-5.10-patched/lib \
-lstd15S  -lm
  Bus Error (core dumped)
  [vitek@andromeda] 140 %

I also tested with POSIX mutexes and saw the same behavior.

Travis

 From: Liviu Nicoara 
 Sent: Thursday, October 11, 2012 5:28 AM
 To: dev@stdcxx.apache.org
 Subject: Re: [PATCH] Re: STDCXX-1072 SPARC V8 mutex alignment requirements
 
 I applied the patch on 4.2.x. If someone with access to a SPARC machine 
 could give it a runall and post the results here it would be awesome. I 
 will postpone closing the incident until then.
 
 Thanks!
 
 Liviu

Re: [PATCH] Re: STDCXX-1072 SPARC V8 mutex alignment requirements

2012-10-13 Thread Liviu Nicoara 

Thanks, Travis!

On 10/13/12 15:36, Travis Vitek wrote:


Liviu,

I built the library and tests before and after your change with the 64-bit 
flag, and I saw no differences in the number of failed tests between the 
builds. I've attached the output of 'gmake -k runall' before and after your 
change to STDCXX-1072 in case you want to look over them.

I wrote up the following test case to prove that I could reproduce the issue 
using the compile/link options used to build the stdcxx tests. The code is 
below..

   [vitek@andromeda] 138 % cat t.cpp
   #include thread.h
   #include synch.h
   #include malloc.h

   int main ()
   {
 int *ip;
 mutex_t *mp;

 ip = (int*)malloc(sizeof (int) + sizeof (mutex_t));
 mp = (mutex_t*)(ip + 1);

 mutex_init(mp, USYNC_THREAD | LOCK_ROBUST, 0);
 mutex_lock(mp);
 mutex_unlock(mp);
 mutex_destroy(mp);

 free(ip);
   }
   [vitek@andromeda] 139 % gmake t
   CC -c -D_RWSTDDEBUG -mt -I/amd/homes/vitek/tmp/stdcxx-4.2.x/include \
 -I/build/vitek/stdcxx-4.2.x_sunpro-5.8_sunos-5.10-patched/include \
 -I/amd/homes/vitek/tmp/stdcxx-4.2.x/tests/include \
 -library=%none -g  -m64 +w -errtags -erroff=hidef   t.cpp
   CC t.o -o t -L/build/vitek/stdcxx-4.2.x_sunpro-5.8_sunos-5.10-patched/rwtest 
\
 -lrwtest15S -library=%none -mt -m64 \
 -L/build/vitek/stdcxx-4.2.x_sunpro-5.8_sunos-5.10-patched/lib \
 -lstd15S  -lm
   Bus Error (core dumped)
   [vitek@andromeda] 140 %

I also tested with POSIX mutexes and saw the same behavior.

Travis


From: Liviu Nicoara
Sent: Thursday, October 11, 2012 5:28 AM
To: dev@stdcxx.apache.org
Subject: Re: [PATCH] Re: STDCXX-1072 SPARC V8 mutex alignment requirements

I applied the patch on 4.2.x. If someone with access to a SPARC machine
could give it a runall and post the results here it would be awesome. I
will postpone closing the incident until then.

Thanks!

Liviu

Re: [PATCH] Re: STDCXX-1072 SPARC V8 mutex alignment requirements

2012-10-11 Thread Liviu Nicoara 
I applied the patch on 4.2.x. If someone with access to a SPARC machine 
could give it a runall and post the results here it would be awesome. I 
will postpone closing the incident until then.


Thanks!

Liviu

On 10/06/12 16:56, Liviu Nicoara wrote:

On 09/29/12 15:33, Liviu Nicoara wrote:

On 9/28/12 11:32 AM, Travis Vitek wrote:


[...] I think an even cleaner solution is to switch
to using __rw_aligned_buffer instead. It gives us a single point of
failure for alignment issues like this, and it makes the code
self-documenting and easier to read.


I am attaching another patch here, which makes use of the
__rw_aligned_buffer, slightly more verbose but the code is slightly
cleaner.


If there are no objections, I would check it in sometime after this
week-end.

Liviu

[PATCH] Re: STDCXX-1072 SPARC V8 mutex alignment requirements

2012-09-29 Thread Liviu Nicoara 

On 9/28/12 11:32 AM, Travis Vitek wrote:



-Original Message-
From: Liviu Nicoara
Sent: Friday, September 28, 2012 5:29 AM
[...]

The patch assumes the type `long double' exists on every platform. While I do 
believe that it is available everywhere, we have lots of conditional code 
guarding its use in the library now. If we are going to use `long double' in 
this context, we should guard it with _RWSTD_NO_LONG_DOUBLE. I think an even 
cleaner solution is to switch to using __rw_aligned_buffer instead. It gives us 
a single point of failure for alignment issues like this, and it makes the code 
self-documenting and easier to read.


I am attaching another patch here, which makes use of the __rw_aligned_buffer, 
slightly more verbose but the code is slightly cleaner.


Thanks!

Liviu


Index: src/messages.cpp
===
--- src/messages.cpp(revision 1390953)
+++ src/messages.cpp(working copy)
@@ -38,6 +38,7 @@
 
 #include rw/_mutex.h
 
+#include podarray.h
 
 #if !defined (_RWSTD_NO_NL_TYPES_H)  !defined (_RWSTD_NO_CATOPEN_CATGETS)
 #  include nl_types.h
@@ -64,10 +65,7 @@ _RWSTD_NAMESPACE (__rw) {
 struct __rw_open_cat_data
 {
 nl_catd catd;
-union {
-void *_C_align;
-char _C_data [sizeof (_STD::locale)];
-} loc;
+__rw_aligned_buffer_STD::locale loc;
 };
 
 struct __rw_open_cat_page
@@ -159,7 +157,8 @@ __rw_manage_cat_data (int cat,  __rw_op
 cat = int (n_catalogs);
 
 catalogs [cat]-catd = pcat_data-catd;
-memcpy (catalogs [cat]-loc, pcat_data-loc,
+memcpy (catalogs [cat]-loc._C_store (), 
+pcat_data-loc._C_store (),
 sizeof (_STD::locale));
 
 if (size_t (cat)  largest_cat)
@@ -175,7 +174,8 @@ __rw_manage_cat_data (int cat,  __rw_op
 }
 
 catalogs [cat]-catd = pcat_data-catd;
-memcpy (catalogs [cat]-loc, pcat_data-loc,
+memcpy (catalogs [cat]-loc._C_store (),
+pcat_data-loc._C_store (),
 sizeof (_STD::locale));
 
 if (size_t (cat)  largest_cat)
@@ -258,8 +258,9 @@ int __rw_cat_open (const _STD::string c
 return -1;
 
 __rw_open_cat_data cat_data;
+
 cat_data.catd = catd;
-new (cat_data.loc) _STD::locale (loc);
+new (cat_data.loc._C_store ()) _STD::locale (loc);
 
 int cat = -1;
 __rw_manage_cat_data (cat, cat_data);
@@ -307,7 +308,7 @@ const _STD::locale __rw_get_locale (int
 
 _RWSTD_ASSERT (0 != pcat_data);
 
-return *(_RWSTD_REINTERPRET_CAST (_STD::locale*, (pcat_data-loc)));
+return *pcat_data-loc._C_data ();
 }
 
 
@@ -329,8 +330,7 @@ void __rw_cat_close (int cat)
 
 catclose (pcat_data-catd);
 
-_STD::locale* const ploc =
-_RWSTD_REINTERPRET_CAST (_STD::locale*, pcat_data-loc);
+_STD::locale* const ploc = pcat_data-loc._C_data ();
 
 ploc-~locale ();
 
Index: src/locale_body.cpp
===
--- src/locale_body.cpp (revision 1390953)
+++ src/locale_body.cpp (working copy)
@@ -1024,14 +1024,12 @@ _C_manage (__rw_locale *plocale, const c
 
 // the classic C locale is statically allocated
 // and not destroyed during the lifetime of the process
-static union {
-void* _C_align;
-char  _C_buf [sizeof (__rw_locale)];
-} classic_body;
+static __rw_aligned_buffer__rw_locale classic_body;
 
 // construct a locale body in place
 // with the initial reference count of 1
-classic = new (classic_body) __rw_locale (locname);
+classic = new (classic_body._C_store ()) 
+__rw_locale (locname);
 
 _RWSTD_ASSERT (1 == classic-_C_ref);
 }
Index: src/use_facet.h
===
--- src/use_facet.h (revision 1390953)
+++ src/use_facet.h (working copy)
@@ -37,6 +37,7 @@
 #include rw/_defs.h
 
 #include access.h
+#include podarray.h
 
 
 // helper macro _RWSTD_DEFINE_FACET_FACTORY() defines a facet factory
@@ -63,12 +64,9 @@
}   \
else {  \
/* construct an ordinary facet in static memory */  \
-   static union {  \
-   void *align_;   \
-   char  data_ [sizeof (__rw_ ## fid ## _facet)];  \
-   } f;\
+