[Bug libstdc++/8670] Alignment problem in std::basic_string

[ncm-nospam at cantrip dot org]

------- 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