RE: [PATCH] STDCXX-853
Liviu, Should the volatile be to the left of the intT typename here? I know it is equivalent, but it is weird to look at the line of code below and see that we're following two different conventions. Travis ___ From: Liviu Nicoara Sent: Sunday, September 23, 2012 8:34 AM To: dev@stdcxx.apache.org Subject: [PATCH] STDCXX-853 Umm, I didn't think to search for a corresponding incident and I considered the defect to be so minor as to not warrant an issue. The following patch has been applied already on 4.2.x: Index: tests/support/atomic_xchg.cpp === --- tests/support/atomic_xchg.cpp (revision 1388732) +++ tests/support/atomic_xchg.cpp (revision 1388733) @@ -297,7 +297,7 @@ void run_test (intT, thr_args_base::tag_ // compute the expected result, "skipping" zeros by incrementing // expect twice when it overflows and wraps around to 0 (zero is // used as the lock variable in thread_routine() above) -intT expect = intT (1); +intT volatile expect = intT (1); const unsigned long nincr = (Args::nthreads_ * Args::nincr_) / 2U;
Re: STDCXX-1066 [was: Re: STDCXX forks]
On Sun, Sep 23, 2012 at 7:25 PM, Liviu Nicoara wrote: > I am not asking for any other implementation and I am not looking to change > anything. I wish you could explain it to us, but in the absence of trade > secret details I will take an explanation for the questions above. Sorry, no. This will not be another replay of the stdcxx-1056 email discussion, which was a three week waste of my time. The patch is provided "AS IS". No further testing and no further comments. I do have more important things to do. -- Stefan Teleman KDE e.V. stefan.tele...@gmail.com
Re: STDCXX-1056 : numpunct fix
On Fri, Sep 21, 2012 at 9:10 AM, Liviu Nicoara wrote: > On 09/21/12 05:13, Stefan Teleman wrote: >> >> On Fri, Sep 21, 2012 at 2:28 AM, Travis Vitek >> wrote: >> >> I have provided this list with test results showing that my patch >> *does* fix the race condition problems identified by all the tools at >> my disposal. I'm willing to bet you never looked at it. You dismissed >> it outright, just because you didn't like the *idea* that increasing >> the size of the cache, and eliminating that useless cache resizing >> would play an important role in eliminating the race conditions. > > > > I looked at it in great detail and I am sure Travis read it too. The > facet/locale buffer management is a red herring. Apparently, we cannot > convince you, regardless of the argument. That's fine by me. This bug [STDCXX-1056] was updated over the weekend with new comments. Here's the link to the comments, for the record: https://issues.apache.org/jira/browse/STDCXX-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13461452#comment-13461452 -- Stefan Teleman KDE e.V. stefan.tele...@gmail.com
Re: STDCXX-1066 [was: Re: STDCXX forks]
On 9/23/12 5:50 PM, Stefan Teleman wrote: On Sun, Sep 23, 2012 at 5:23 PM, Stefan Teleman wrote: The second URL says this: Due to a change in the implementation of the userland mutexes introduced by CR 6296770 in KU 137111-01, objects of type mutex_t and pthread_mutex_t must start at 8-byte aligned addresses. If this requirement is not satisfied, all non-compliant applications on Solaris/SPARC may fail with the signal SEGV with a callstack similar to the following one or with similar callstacks containing the function mutex_trylock_process. \*_atomic_cas_64(0x141f2c, 0x0, 0xff00, 0x1651, 0xff00, 0x466d90) set_lock_byte64(0x0, 0x1651, 0xff00, 0x0, 0xfec82a00, 0x0) fast_process_lock(0x141f24, 0x0, 0x1, 0x1, 0x0, 0xfeae5780) Here's a link to an official datatype alignment table for SPARCV8: http://docs.oracle.com/cd/E19205-01/819-5267/bkbkl/index.html The interesting table is: Table B–2 Storage Sizes and Default Alignments in Bytes I see nothing really outstanding here. What is it that I should pay attention to? Thanks, Liviu
Re: STDCXX-1066 [was: Re: STDCXX forks]
On Sun, Sep 23, 2012 at 5:23 PM, Stefan Teleman wrote: > The second URL says this: > > > Due to a change in the implementation of the userland mutexes > introduced by CR 6296770 in KU 137111-01, objects of type mutex_t and > pthread_mutex_t must start at 8-byte aligned addresses. If this > requirement is not satisfied, all non-compliant applications on > Solaris/SPARC may fail with the signal SEGV with a callstack similar > to the following one or with similar callstacks containing the > function mutex_trylock_process. > > \*_atomic_cas_64(0x141f2c, 0x0, 0xff00, 0x1651, 0xff00, 0x466d90) > set_lock_byte64(0x0, 0x1651, 0xff00, 0x0, 0xfec82a00, 0x0) > fast_process_lock(0x141f24, 0x0, 0x1, 0x1, 0x0, 0xfeae5780) > > Here's a link to an official datatype alignment table for SPARCV8: http://docs.oracle.com/cd/E19205-01/819-5267/bkbkl/index.html The interesting table is: Table B–2 Storage Sizes and Default Alignments in Bytes --Stefan -- Stefan Teleman KDE e.V. stefan.tele...@gmail.com
Re: STDCXX-1066 [was: Re: STDCXX forks]
On 9/23/12 3:48 PM, Stefan Teleman wrote: On Sun, Sep 23, 2012 at 3:26 PM, Liviu Nicoara wrote: To be honest it's quite bizarre that you cannot share that with us. Is it really a trade secret? How can that be the case if Oracle customers are also required to perform the same alignment, perhaps using the same techniques like you showed in the patch? That's the problem. I don't know what is and what is not a trade secret, or copyrighted information, or "unauthorized intellectual property disclosure" anymore. I think it's in the eye of the beholder. At Sun it was very clear. Believe it or not, I had to get written approval from the Legal Counsel's Office in order to be able to share these patches. And that in spite of the fact that these patches are published, and have already been published for *years*. That clarifies things. Thanks. Liviu
Re: STDCXX-1066 [was: Re: STDCXX forks]
On Sun, Sep 23, 2012 at 3:26 PM, Liviu Nicoara wrote: > To be honest it's quite bizarre that you cannot share that with us. Is it > really a trade secret? How can that be the case if Oracle customers are also > required to perform the same alignment, perhaps using the same techniques > like you showed in the patch? That's the problem. I don't know what is and what is not a trade secret, or copyrighted information, or "unauthorized intellectual property disclosure" anymore. I think it's in the eye of the beholder. At Sun it was very clear. Believe it or not, I had to get written approval from the Legal Counsel's Office in order to be able to share these patches. And that in spite of the fact that these patches are published, and have already been published for *years*. IANAL and I don't want to play one on TV. ;-) --Stefan -- Stefan Teleman KDE e.V. stefan.tele...@gmail.com
Re: STDCXX-1066 [was: Re: STDCXX forks]
On 9/23/12 2:02 PM, Stefan Teleman wrote:
On Sun, Sep 23, 2012 at 10:35 AM, Liviu Nicoara wrote:
2. The issue only exists in MT builds, should there be a guard in configs?
Yes, good point. The reason they aren't there is because we don't
actually provide a non-MT stdcxx at all in Solaris. I'll fix this.
I remember you mentioned it.
4. I see rw/_mutex.h has alignment pragmas for both __rw_mutex_base class
and its mutex member; same for __rw_static_mutex and its static member, etc.
How does that work?
It works. ;-) And it actually acts as a space saver. Wink-wink.
Nudge-nudge.
To be honest it's quite bizarre that you cannot share that with us. Is it really
a trade secret? How can that be the case if Oracle customers are also required
to perform the same alignment, perhaps using the same techniques like you showed
in the patch?
But I don't think the _C_mutex member is static. In rw/_mutex.h,
_RWSTD_MUTEX_T is #defined as:
#include
// [ ... snip ... ]
# define _RWSTD_MUTEX_Tpthread_mutex_t
(for the definition Solaris cares about, which is POSIX).
So, in
class _RWSTD_EXPORT __rw_mutex_base
{
public:
// [ ... snip ... ]
_RWSTD_MUTEX_T _C_mutex;
};
it looks like it's not declared static.
I meant the static member of __rw_static_mutex.
5. Why is __rw_guard aligned explicitly? I see it only contains a pointer to
a mutex object.
So that the __rw_mutex_base pointer ends up 8-byte aligned. There's a
lot of juju going on here.
6. The docs mention that the pragma must use the mangled variables names but I
don't see that in the patch.
Yes, a few things are a bit different. ;-) I wish I didn't have to be
as vague and secretive about these things as I have to be.
Well, that's a pity. Thanks anyway.
Liviu
Re: STDCXX-1066 [was: Re: STDCXX forks]
On Sun, Sep 23, 2012 at 10:35 AM, Liviu Nicoara wrote:
> I can't think of a valid scenario, either. I guess the point is moot.
I really don't think we should worry about this particular scenario.
For example if someone really wants to experiment with saying
#pragma align(2)
before #including any system header files, and then testing if their
program still works, more power to them. My bet is that this will not,
ever, work, under any circumstances.
> A few more questions, if you will, as I am going through the changes:
>
> 1. I see similarities with 1040, should/would you close that one?
Oh I completely forgot about that one. Yes I will close it as
duplicate of this one, because these patches attached here are in
production and over-tested. I don't even remember if the old patches
are identical or not.
> 2. The issue only exists in MT builds, should there be a guard in configs?
Yes, good point. The reason they aren't there is because we don't
actually provide a non-MT stdcxx at all in Solaris. I'll fix this.
> 3. The align reference docs talk only about aligning variables, not types.
> Is that different on SPARC?
It can be. ;-)
> 4. I see rw/_mutex.h has alignment pragmas for both __rw_mutex_base class
> and its mutex member; same for __rw_static_mutex and its static member, etc.
> How does that work?
It works. ;-) And it actually acts as a space saver. Wink-wink.
But I don't think the _C_mutex member is static. In rw/_mutex.h,
_RWSTD_MUTEX_T is #defined as:
#include
// [ ... snip ... ]
# define _RWSTD_MUTEX_Tpthread_mutex_t
(for the definition Solaris cares about, which is POSIX).
So, in
class _RWSTD_EXPORT __rw_mutex_base
{
public:
// [ ... snip ... ]
_RWSTD_MUTEX_T _C_mutex;
};
it looks like it's not declared static.
> 5. Why is __rw_guard aligned explicitly? I see it only contains a pointer to
> a mutex object.
So that the __rw_mutex_base pointer ends up 8-byte aligned. There's a
lot of juju going on here.
> 6. The docs mention that the pragma must use the mangled variables names but
> I don't see that in the patch.
Yes, a few things are a bit different. ;-) I wish I didn't have to be
as vague and secretive about these things as I have to be.
--Stefan
--
Stefan Teleman
KDE e.V.
stefan.tele...@gmail.com
[PATCH] STDCXX-1069 [was: Re: SUNPro 5.12 compilation failure in optimized Linux builds]
On 09/22/12 00:51, Liviu Nicoara wrote: Optimized (but not debug) builds fail to compile setlocale.cpp with the error: A patch and a comment have been attached to the issue. Thanks, Liviu
[PATCH] STDCXX-853
Umm, I didn't think to search for a corresponding incident and I considered the defect to be so minor as to not warrant an issue. The following patch has been applied already on 4.2.x: Index: tests/support/atomic_xchg.cpp === --- tests/support/atomic_xchg.cpp (revision 1388732) +++ tests/support/atomic_xchg.cpp (revision 1388733) @@ -297,7 +297,7 @@ void run_test (intT, thr_args_base::tag_ // compute the expected result, "skipping" zeros by incrementing // expect twice when it overflows and wraps around to 0 (zero is // used as the lock variable in thread_routine() above) -intT expect = intT (1); +intT volatile expect = intT (1); const unsigned long nincr = (Args::nthreads_ * Args::nincr_) / 2U;
Re: STDCXX-1066 [was: Re: STDCXX forks]
On 09/16/12 12:03, Stefan Teleman wrote: On Sat, Sep 15, 2012 at 5:47 PM, Liviu Nicoara wrote: I merely wanted to point out that restoring the default packing is not the same as restoring the packing to the previous value in effect. Given this, I thought about an alternative way of forcing this alignment, e.g., via a union, aligned on an appropriate type. I see an advantage here in that most of the changes would occur where we define the 'primitive' mutex and condition wrappers, saving a few pre-processor conditionals and pragmas along the way. I understood what you were saying. I just don't understand under what _sane_ circumstances a program would #include a system library header file with a previously set packing to something other than default. Or would expect a non-default packing to work during a function call to a sytem library. That's an insane configuration on any operating system that I know of, not just on SPARCV8. I can't think of a valid scenario, either. I guess the point is moot. A few more questions, if you will, as I am going through the changes: 1. I see similarities with 1040, should/would you close that one? 2. The issue only exists in MT builds, should there be a guard in configs? 3. The align reference docs talk only about aligning variables, not types. Is that different on SPARC? 4. I see rw/_mutex.h has alignment pragmas for both __rw_mutex_base class and its mutex member; same for __rw_static_mutex and its static member, etc. How does that work? 5. Why is __rw_guard aligned explicitly? I see it only contains a pointer to a mutex object. 6. The docs mention that the pragma must use the mangled variables names but I don't see that in the patch. Thanks! Liviu
