incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject [PATCH] Re: stdcxx 4.2.0/4.1.3 binary incompatibility on Linux
Date Thu, 18 Oct 2007 00:39:22 GMT
How does the attached patch look?

The patch adds two macros, _RWSTD_NO_STRING_ATOMIC_OPS and
_RWSTD_USE_STRING_ATOMIC_OPS. The first one is #defined for
all compilers on Linux/x86_64 (i.e., in wide mode), *unless*
the second one is defined, either on the command line on in
the generated config header by the user. This whole hackery
is guarded by _RWSTD_VER and automatically disabled (i.e.,
the library switches over to using atomic operations by
default) at version 5.

Martin

Martin Sebor wrote:
> Travis Vitek wrote:
>> So the problem is that the size of the __string_ref has changed, right?
> 
> Yes. More precisely, that is one part of the problem (the other
> part is the pthread functions that are inlined in user code and
> that expect to get a mutex and not random garbage).
> 
>> Specifically, this block of code causes the removal of the per-string
>> mutex from __string_ref.
>>
>> +#ifndef _RWSTD_NO_ATOMIC_OPS
>> +   // disable string mutex when atomic operations are available
>> +#  ifndef _RWSTD_NO_STRING_MUTEX
>> +#    define _RWSTD_NO_STRING_MUTEX
>> +#  endif   // _RWSTD_NO_STRING_MUTEX
>> +#endif   // _RWSTD_NO_ATOMIC_OPS
>>
>> An obvious option would be to just remove that block and let the user
>> decide if they want to define _RWSTD_NO_STRING_MUTEX or not. If they
>> define it then they must know that they are breaking binary
>> compatibility with a library previously compiled without it. This
>> doesn't help users compiling new source using the default configuration,
>> but it does maintain compatibility.
> 
> I'm also leaning in this general direction. Keep the compatible
> behavior and and let users opt into the breaking fix. I'll have
> to think some more about how to control the behavior. I was
> hoping to use an existing config macro without changing any
> library code (except _config-gcc.h).
> 
>>
>> We could also ask that they define _RWSTD_NO_ATOMIC_OPS, but that may
>> cause other binary incompatibilities elsewhere or it will at the very
>> least cause performance problems in other places.
> 
> It is very tempting to go with the improved implementation by
> default, but from a compatibility standpoint it would be the
> wrong thing to do.
> 
>>
>> It would be nice if we could just insert an appropriately sized pad
>> buffer in place of the _C_mutex member. If the methods of __string_ref
>> were not inlined I'm pretty sure that this would work. Unfortunately
>> _C_inc_ref() and _C_dec_ref() are inlined, so their code may be compiled
>> in the user executable, so I'm not convinced that this is a viable
>> option.
> 
> All the __string_ref members are trivial inline wrappers that
> are fully expected to be inlined. It's possible that some of
> them will not be inlined under some conditions but I expect
> those to be exceeding rare.
> 
> Martin
> 
>>
>> Travis
>>
>>
>> Martin Sebor wrote:
>>> Okay, I've got it:
>>>
>>>    http://issues.apache.org/jira/browse/STDCXX-162
>>>
>>> Damn that was hard!
>>>
>>> So, what do we do? Going back to using a mutex for strings would
>>> be *huge* performance hit on one of the most popular platforms
>>> (if not the most popular one), but then again, keeping the status
>>> quo will break binary compatibility on the (now) most popular
>>> platform.
>>>
>>> Opinions?
>>>
>>> Martin
>>>
>>> Martin Sebor wrote:
>>>> Martin Sebor wrote:
>>>>> In a 12D build with the default gcc 4.1.0 on SuSE Linux Enterprise
>>>>> Server 10 (x86_64), the following simple program abends with the
>>>>> error below after upgrading the 4.1.3 library to 4.2.0:
>>>> I've enhanced the program to replace operators new and delete
>>>> and to print the value of the pointer. The enhanced test case
>>>> and the output obtained from a 12D build with gcc 3.4.6 on Red
>>>> Hat Enterprise Linux AS release 4 (Nahant Update 4) is below.
>>>> Interestingly, the 12d (32-bit) output with Sun C++ on Solaris
>>>> is fine.
>>>>
>>>> $ cat t.cpp && LD_LIBRARY_PATH=../lib ./t
>>>> #include <cstdio>
>>>> #include <cstdlib>
>>>> #include <new>
>>>> #include <string>
>>>>
>>>> int main ()
>>>> {
>>>>     std::string s = "a";
>>>> }
>>>>
>>>> void* operator new (std::size_t n) throw (std::bad_alloc)
>>>> {
>>>>     void* const p = std::malloc (n);
>>>>     std::fprintf (stdout, "operator new (%zu) ==> %#p\n", n, p);
>>>>     return p;
>>>> }
>>>>
>>>> void operator delete (void *p) throw ()
>>>> {
>>>>     std::fprintf (stdout, "operator delete (%#p)\n", p);
>>>>     std::free (p);
>>>> }
>>>>
>>>> void* operator new[] (std::size_t n) throw (std::bad_alloc)
>>>> {
>>>>     void* const p = std::malloc (n);
>>>>     std::fprintf (stdout, "operator new[] (%zu) ==> %#p\n", n, p);
>>>>     return p;
>>>> }
>>>>
>>>> void operator delete[] (void *p) throw ()
>>>> {
>>>>     std::fprintf (stdout, "operator delete[] (%#p)\n", p);
>>>>     std::free (p);
>>>> }
>>>>
>>>> operator new (58) ==> 0x502010
>>>> operator delete (0x501fe8)
>>>> *** glibc detected *** free(): invalid pointer: 
>>> 0x0000000000501fe8 ***
>>>> Aborted
>>>>
>>>>
> 


Mime
View raw message