From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28687 invoked by alias); 21 Jan 2005 17:46:55 -0000 Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Archive: List-Post: List-Help: Sender: gcc-bugs-owner@gcc.gnu.org Received: (qmail 27859 invoked by uid 48); 21 Jan 2005 17:45:48 -0000 Date: Fri, 21 Jan 2005 17:46:00 -0000 Message-ID: <20050121174548.27858.qmail@sourceware.org> From: "ncm-nospam at cantrip dot org" To: gcc-bugs@gcc.gnu.org In-Reply-To: <20021121063601.8670.jkanze@caicheuvreux.com> References: <20021121063601.8670.jkanze@caicheuvreux.com> Reply-To: gcc-bugzilla@gcc.gnu.org Subject: [Bug libstdc++/8670] Alignment problem in std::basic_string X-Bugzilla-Reason: CC X-SW-Source: 2005-01/txt/msg03084.txt.bz2 List-Id: ------- Additional Comments From ncm-nospam at cantrip dot org 2005-01-21 17:45 ------- Do I understand correctly that the FE fix has been applied? I don't see any corresponding __attribute__ in HEAD for basic_string.h. Probably _Rep should be aligned not to a constant size, but rather to the most stringent needs of _Rep_base's members and and the actual_ charT itself. Can that be expressed? If __align__ can't do it, an ugly union trick should work. The tricky bit is to avoid wasting space if _charT is bigger than a member of _Rep_base, so we probasbly don't want to use Martin's trick directly. (Anyway as written it doesn't work properly because you have to align the first member, not the last.) The right fix involves aligning the whole struct, rather than the first member, almost as described in the proposed fix for 19495, but looking like: union { _Atomic_word _M_w; size_type _M_s; _charT _M_c; } _Alloc_unit; The only problem I see is that if sizeof _charT happens not to be a power of two, the "/ sizeof _Alloc_unit" doesn't optimize nicely to a shift. That seems tolerable. The remaining problem is that the calculations "this+1" and "this-1" don't account for alignment. It seems to need trickier casting, along the lines of _Rep* _M_rep() const - { return &((reinterpret_cast<_Rep*> (_M_data()))[-1]); } + { + return reinterpret_cast<_Rep*>( + &((reinterpret_cast<_Alloc_unit*>(_M_data()))[-1])); + } and _CharT* _M_refdata() throw() - { return reinterpret_cast<_CharT*>(this + 1); } + { + return reinterpret_cast<_CharT*>( + reinterpret_cast<_Alloc_unit*>(this) + 1); + } I wonder if these should use unions, instead of reinterpret_cast<>, so as to be compatible with -fstrict-aliasing. Then I guess they look more like _Rep* _M_rep() const - { return &((reinterpret_cast<_Rep*> (_M_data()))[-1]); } + { + union { _charT* _M_cp, _Alloc_unit* _M_ap; _Rep* _M_rp; } _Aligner; + _Aligner __p = { _M_data() }; + --__p._M_ap; + return __p._M_rp; + } and _CharT* _M_refdata() throw() - { return reinterpret_cast<_CharT*>(this + 1); } + { + union { _Rep* _M_rp; _Alloc_unit* _M_ap; _charT* _M_cp,} _Aligner; + _Aligner __p = { this }; + ++__p._M_ap; + return __p._M_cp; + } Some people might like them better that way anyhow. -- What |Removed |Added ---------------------------------------------------------------------------- CC| |ncm-nospam at cantrip dot | |org http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8670