This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ 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]

Re: [v3] Fix Werror breakage (maintainer-mode)


On Sun, Jul 6, 2008 at 7:51 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Sun, Jul 6, 2008 at 12:08 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>>> Can somebody enlighten me as to a proper locale_init.cc fix?
>>
>> Yes, the warning is new and I'd like to understand whether the warning is worrisome or not, or even bogus. That's not clear to me, at the moment. As a LAST resort, the warning could be suppressed via the "usual" trick, something like:
>>
>>    const char* __p = reinterpret_cast<const char*>(c_locale);
>>    return *reinterpret_cast<const locale*>(__p);
>>
>> which I would consider a tad better than disabling -Werror selectively.
>
> The code does
>
>  typedef char fake_locale[sizeof(locale)]
>  __attribute__ ((aligned(__alignof__(locale))));
>  fake_locale c_locale;
>
>  const locale&
>  locale::classic()
>  {
>    _S_initialize();
>    return reinterpret_cast<const locale&>(c_locale);
>  }

This idiom -- which I opposed when it was first mentioned, but now I think
is better than alternatives -- is to designed to work around the unordered
initialization of globals accross translation units.

The idea behind it is that, we first allocate (static) storare for the object
(fake_locale type), which we also name c_locale.  Then later on, we actually
give that storage a definite type (the dynamic type) by using the new-placement
operator.  From there on, the object acquires the definite dynamic type till
destruction.   So, the reinterpret_cast really is not mucking around
-- it does not
change the dynamic type of the object.  So, everything is fine.

Notice that this idiom may become common place, now that C++0x has alignas
(the moral equivalent of GNU C++'s attribute above), along with the library
aligned_storage<> template.

>
> which violates C aliasing rules.  But - I understand that possibly
>
>  void
>  locale::_S_initialize_once()
>  {
>    // 2 references.
>    // One reference for _S_classic, one for _S_global
>    _S_classic = new (&c_locale_impl) _Impl(2);
>    _S_global = _S_classic;
>    new (&c_locale) locale(_S_classic);
>  }
>
> is always called before the above, so the dynamic type of c_locale
> will be locale, so this _doesn't_ violate the C++ aliasing rules.

That is correct.

>
> (of course some of us don't agree on the interpretation of the C++
> standard wording with regards to whether it is allowed to change
> the dynamic type of storage with a declared type, c_locale in this case)
>
> A miscompilation is very unlikely (even though GCC still may have bugs
> with honoring the C++ memory model), as reads/writes of type c_locale
> always conflict with reads/writes of type char, which is the declared type
> of c_clocale.

We need to make sure that the compiler guarantees the semantics -- e.g.
its getting `smarter' does not inadvertently produce miscompilation.

>
> For "correctness" and code documentation purposes in this case it would
> be nice to have a way to declare un-typed memory which cannot be
> accessed through the declaration identifier or a pointer of its "type", like
>
>  typedef void fake_locale
> __attribute__((aligned(__alignof__(locale)),size(__sizeof__(locale)));
>  fake_locale c_locale;

C++0x aligned_storage<> is supposed to give a nicer syntax.
C++0x also as the keyword alignas.

>
> or some similar way of associating the identifier c_locale with some memory
> (basically a "static" malloc).

Yep.

>
> Richard.
>


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