Library ABI seriously broken!!

View: New views
10 Messages — Rating Filter:   Alert me  

Library ABI seriously broken!!

by Paolo Carlini-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Hi,

sorry if the issue is already well known, but it's so serious that I
decided to post an heads up: today (not yesterday), the ABI is broken,
the size of many symbols exported by libstdc++ changed, see eg:

    http://gcc.gnu.org/ml/gcc-testresults/2009-10/msg02895.html
    http://gcc.gnu.org/ml/gcc-testresults/2009-10/msg02903.html
    http://gcc.gnu.org/ml/gcc-testresults/2009-10/msg02904.html

I'm adding below some of the many error messages spilled by abi_check.

I *strongly* suspect:

2009-10-28  Jerry Quinn  <jlquinn@...>

        * mangle.c (mangle_type_string_for_rtti): Revert r149964.
        (needs_fake_anon): Likewise.
        (write_name): Likewise.
        (write_nested_name): Likewise.
        * cp-tree.h (mangle_type_string_for_rtti): Likewise.
        (get_anonymous_namespace): Likewise.
        * name-lookup.c (get_anonymous_namespace_name): Likewise.
        * rtti.c (tinfo_name): Insert '*' in front of private names.
        (tinfo_base_init): Use it.

2009-10-28  Jerry Quinn  <jlquinn@...>

        * libsupc++/tinfo.cc (operator=(const type_info&)): Compare by
        pointer if name begins with '*'.
        * libsupc++/typeinfo (type_info::name()): Likewise.
        * libsupc++/tinfo2.cc (before):  Likewise.

Please provide feedback ASAP.

Thanks,
Paolo.

////////////////////////

171 incompatible symbols
0
_ZTSSt18basic_stringstreamIcSt11char_traitsIcESaIcEE
typeinfo name for std::basic_stringstream<char, std::char_traits<char>,
std::allocator<char> >
version status: unversioned
type: object
type size: 48
status: <default>

    incompatible sizes
    49
    48

1
_ZTSSt7codecvtIcc11__mbstate_tE
typeinfo name for std::codecvt<char, char, __mbstate_t>
version status: unversioned
type: object
type size: 27
status: <default>

    incompatible sizes
    28
    27

2
_ZTSSt10moneypunctIwLb0EE
typeinfo name for std::moneypunct<wchar_t, false>
version status: unversioned
type: object
type size: 21
status: <default>

    incompatible sizes
    22
    21

3
_ZTSSt13basic_fstreamIcSt11char_traitsIcEE
typeinfo name for std::basic_fstream<char, std::char_traits<char> >
version status: unversioned
type: object
type size: 38
status: <default>

    incompatible sizes
    39
    38

4
_ZTSSt12ctype_bynameIwE
typeinfo name for std::ctype_byname<wchar_t>
version status: unversioned
type: object
type size: 19
status: <default>

    incompatible sizes
    20
    19

5
_ZTSPa
typeinfo name for signed char*
version status: unversioned
type: object
type size: 2
status: <default>

    incompatible sizes
    3
    2

6
_ZTSSt15underflow_error
typeinfo name for std::underflow_error
version status: unversioned
type: object
type size: 19
status: <default>

    incompatible sizes
    20
    19

7
_ZTSSt19basic_ostringstreamIcSt11char_traitsIcESaIcEE
typeinfo name for std::basic_ostringstream<char, std::char_traits<char>,
std::allocator<char> >
version status: unversioned
type: object
type size: 49
status: <default>

    incompatible sizes
    50
    49

8
_ZTSSt9money_putIcSt19ostreambuf_iteratorIcSt11char_traitsIcEEE
typeinfo name for std::money_put<char, std::ostreambuf_iterator<char,
std::char_traits<char> > >
version status: unversioned
type: object
type size: 59
status: <default>

    incompatible sizes
    60
    59

9
_ZTSSt7num_putIwSt19ostreambuf_iteratorIwSt11char_traitsIwEEE
typeinfo name for std::num_put<wchar_t,
std::ostreambuf_iterator<wchar_t, std::char_traits<wchar_t> > >
version status: unversioned
type: object
type size: 57
status: <default>

    incompatible sizes
    58
    57

10
_ZTSPh
typeinfo name for unsigned char*
version status: unversioned
type: object
type size: 2
status: <default>

    incompatible sizes
    3
    2

11
_ZTSSt13basic_filebufIcSt11char_traitsIcEE
typeinfo name for std::basic_filebuf<char, std::char_traits<char> >
version status: unversioned
type: object
type size: 38
status: <default>

    incompatible sizes
    39
    38

12
_ZTSs
typeinfo name for short
version status: unversioned
type: object
type size: 1
status: <default>

    incompatible sizes
    2
    1

13
_ZTSSt8bad_cast
typeinfo name for std::bad_cast
version status: unversioned
type: object
type size: 11
status: <default>

    incompatible sizes
    12
    11

14
_ZTSPKj
typeinfo name for unsigned int const*
version status: unversioned
type: object
type size: 3
status: <default>

    incompatible sizes
    4
    3

15
_ZTSPKc
typeinfo name for char const*
version status: unversioned
type: object
type size: 3
status: <default>

    incompatible sizes
    4
    3

16
_ZTSe
typeinfo name for long double
version status: unversioned
type: object
type size: 1
status: <default>

    incompatible sizes
    2
    1

17
_ZTSSt14basic_ifstreamIcSt11char_traitsIcEE
typeinfo name for std::basic_ifstream<char, std::char_traits<char> >
version status: unversioned
type: object
type size: 39
status: <default>

    incompatible sizes
    40
    39

18
_ZTSSt9money_putIwSt19ostreambuf_iteratorIwSt11char_traitsIwEEE
typeinfo name for std::money_put<wchar_t,
std::ostreambuf_iterator<wchar_t, std::char_traits<wchar_t> > >
version status: unversioned
type: object
type size: 59
status: <default>

    incompatible sizes
    60
    59

19
_ZTSl
typeinfo name for long
version status: unversioned
type: object
type size: 1
status: <default>

    incompatible sizes
    2
    1

20
_ZTSSt21__ctype_abstract_baseIcE
typeinfo name for std::__ctype_abstract_base<char>
version status: unversioned
type: object
type size: 28
status: <default>

    incompatible sizes
    29
    28

21
_ZTSPx
typeinfo name for long long*
version status: unversioned
type: object
type size: 2
status: <default>

    incompatible sizes
    3
    2

22
_ZTSSt9time_base
typeinfo name for std::time_base
version status: unversioned
type: object
type size: 12
status: <default>

    incompatible sizes
    13
    12

23
_ZTSSt8messagesIcE
typeinfo name for std::messages<char>
version status: unversioned
type: object
type size: 14
status: <default>

    incompatible sizes
    15
    14

24
_ZTSSt9bad_alloc
typeinfo name for std::bad_alloc
version status: unversioned
type: object
type size: 12
status: <default>

    incompatible sizes
    13
    12

25
_ZTSN9__gnu_cxx18stdio_sync_filebufIwSt11char_traitsIwEEE
typeinfo name for __gnu_cxx::stdio_sync_filebuf<wchar_t,
std::char_traits<wchar_t> >
version status: unversioned
type: object
type size: 53
status: <default>

    incompatible sizes
    54
    53

26
_ZTSSt15basic_streambufIwSt11char_traitsIwEE
typeinfo name for std::basic_streambuf<wchar_t, std::char_traits<wchar_t> >
version status: unversioned
type: object
type size: 40
status: <default>

    incompatible sizes
    41
    40

27
_ZTSSt12system_error
typeinfo name for std::system_error
version status: unversioned
type: object
type size: 16
status: <default>

    incompatible sizes
    17
    16

28
_ZTSSt8numpunctIcE
typeinfo name for std::numpunct<char>
version status: unversioned
type: object
type size: 14
status: <default>

    incompatible sizes
    15
    14

29
_ZTSSt15time_put_bynameIwSt19ostreambuf_iteratorIwSt11char_traitsIwEEE
typeinfo name for std::time_put_byname<wchar_t,
std::ostreambuf_iterator<wchar_t, std::char_traits<wchar_t> > >
version status: unversioned
type: object
type size: 66
status: <default>

    incompatible sizes
    67
    66

30
_ZTSPKl
typeinfo name for long const*
version status: unversioned
type: object
type size: 3
status: <default>

    incompatible sizes
    4
    3

31
_ZTSSt9exception
typeinfo name for std::exception
version status: unversioned
type: object
type size: 12
status: <default>

    incompatible sizes
    13
    12

32
_ZTSSt13runtime_error
typeinfo name for std::runtime_error
version status: unversioned
type: object
type size: 17
status: <default>

    incompatible sizes
    18
    17

33
_ZTSSt15basic_streambufIcSt11char_traitsIcEE
typeinfo name for std::basic_streambuf<char, std::char_traits<char> >
version status: unversioned
type: object
type size: 40
status: <default>

    incompatible sizes
    41
    40

34
_ZTSPKe
typeinfo name for long double const*
version status: unversioned
type: object
type size: 3
status: <default>

    incompatible sizes
    4
    3

35
_ZTSSt7collateIwE
typeinfo name for std::collate<wchar_t>
version status: unversioned
type: object
type size: 13
status: <default>

    incompatible sizes
    14
    13

36
_ZTSPw
typeinfo name for wchar_t*
version status: unversioned
type: object
type size: 2
status: <default>

    incompatible sizes
    3
    2

37
_ZTSSt8time_putIwSt19ostreambuf_iteratorIwSt11char_traitsIwEEE
typeinfo name for std::time_put<wchar_t,
std::ostreambuf_iterator<wchar_t, std::char_traits<wchar_t> > >
version status: unversioned
type: object
type size: 58
status: <default>

    incompatible sizes
    59
    58

38
_ZTSPKw
typeinfo name for wchar_t const*
version status: unversioned
type: object
type size: 3
status: <default>

    incompatible sizes
    4
    3

39
_ZTSSi
typeinfo name for std::istream
version status: unversioned
type: object
type size: 2
status: <default>

    incompatible sizes
    3
    2

40
_ZTSSt12length_error
typeinfo name for std::length_error
version status: unversioned
type: object
type size: 16
status: <default>

    incompatible sizes
    17
    16


Re: Library ABI seriously broken!!

by Jerry Quinn :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Fri, 2009-10-30 at 12:48 +0100, Paolo Carlini wrote:

> Hi,
>
> sorry if the issue is already well known, but it's so serious that I
> decided to post an heads up: today (not yesterday), the ABI is broken,
> the size of many symbols exported by libstdc++ changed, see eg:
>
>     http://gcc.gnu.org/ml/gcc-testresults/2009-10/msg02895.html
>     http://gcc.gnu.org/ml/gcc-testresults/2009-10/msg02903.html
>     http://gcc.gnu.org/ml/gcc-testresults/2009-10/msg02904.html
>
> I'm adding below some of the many error messages spilled by abi_check.
>
> I *strongly* suspect:
>
> 2009-10-28  Jerry Quinn  <jlquinn@...>
>
> * mangle.c (mangle_type_string_for_rtti): Revert r149964.
> (needs_fake_anon): Likewise.
> (write_name): Likewise.
> (write_nested_name): Likewise.
> * cp-tree.h (mangle_type_string_for_rtti): Likewise.
> (get_anonymous_namespace): Likewise.
> * name-lookup.c (get_anonymous_namespace_name): Likewise.
> * rtti.c (tinfo_name): Insert '*' in front of private names.
> (tinfo_base_init): Use it.


Hi, Paolo,

I've reverted the patch.

This was intended to fix the broken bootstrap under
--enable-build-with-cxx.  Currently anonymous namespaces get random
names so successive builds of the compiler have different binaries when
built with C++.  The patch marked anonymous namespaces so they could be
compared by pointer instead of string allowing removal of the random
naming.  Obviously a different strategy is needed.

I missed the ABI breakage when testing.  Sorry for the aggravation.

Jerry




Re: Library ABI seriously broken!!

by Paolo Carlini-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Jerry Quinn wrote:
> I've reverted the patch.
>  
Thanks Jerry for your quick feedback.

Paolo.

Re: Library ABI seriously broken!!

by Richard Guenther-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Fri, Oct 30, 2009 at 1:54 PM, Paolo Carlini <paolo.carlini@...> wrote:
> Jerry Quinn wrote:
>> I've reverted the patch.
>>
> Thanks Jerry for your quick feedback.

I think it was just

 static tree
-tinfo_name (tree type)
+tinfo_name (tree type, bool mark_private)
 {
   const char *name;
+  int length;
   tree name_string;

-  name = mangle_type_string_for_rtti (type);
-  name_string = fix_string_type (build_string (strlen (name) + 1, name));
-  return name_string;
+  name = mangle_type_string (type);
+  length = strlen (name);
+
+  if (mark_private)
+    {
+      /* Inject '*' at beginning of name to force pointer comparison.  */
+      char* buf = (char*) XALLOCAVEC (char, length + 1);
+      buf[0] = '*';
+      memcpy (buf + 1, name, length);
+      name_string = build_string (length + 1, buf);
+    }
+  else
+    name_string = build_string (length, name);
+
+  return fix_string_type (name_string);

where you replaced build_string (strlen (name) + 1, name) with
build_string (strlen (name), name).  I don't know if this renders the
ABIs incompatible, but I doubt it - it would be nice to verify that indeed
just extra '\0's are now missing at the end.

Richard.


> Paolo.
>

Re: Library ABI seriously broken!!

by Paolo Carlini-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Richard Guenther wrote:
> where you replaced build_string (strlen (name) + 1, name) with
> build_string (strlen (name), name).  I don't know if this renders the
> ABIs incompatible, but I doubt it - it would be nice to verify that indeed
> just extra '\0's are now missing at the end.
>  
To be clear: if knowledgeable people can confirm this kind of reasoning,
of course I have no objections, but then abi_check must be also fixed at
the same time, nothing should committed leading to regressions, either
substantive or only apparent.

Paolo.

Re: Library ABI seriously broken!!

by Richard Guenther-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Fri, Oct 30, 2009 at 2:15 PM, Paolo Carlini <paolo.carlini@...> wrote:

> Richard Guenther wrote:
>> where you replaced build_string (strlen (name) + 1, name) with
>> build_string (strlen (name), name).  I don't know if this renders the
>> ABIs incompatible, but I doubt it - it would be nice to verify that indeed
>> just extra '\0's are now missing at the end.
>>
> To be clear: if knowledgeable people can confirm this kind of reasoning,
> of course I have no objections, but then abi_check must be also fixed at
> the same time, nothing should committed leading to regressions, either
> substantive or only apparent.

I fully agree with you here.  A simple solution is to re-instantiate the +1
(with a comment before it).

Richard.

Re: Library ABI seriously broken!!

by Jason Merrill :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On 10/30/2009 09:20 AM, Richard Guenther wrote:

> On Fri, Oct 30, 2009 at 2:15 PM, Paolo Carlini<paolo.carlini@...>  wrote:
>> Richard Guenther wrote:
>>> where you replaced build_string (strlen (name) + 1, name) with
>>> build_string (strlen (name), name).  I don't know if this renders the
>>> ABIs incompatible, but I doubt it - it would be nice to verify that indeed
>>> just extra '\0's are now missing at the end.
>>>
>> To be clear: if knowledgeable people can confirm this kind of reasoning,
>> of course I have no objections, but then abi_check must be also fixed at
>> the same time, nothing should committed leading to regressions, either
>> substantive or only apparent.
>
> I fully agree with you here.  A simple solution is to re-instantiate the +1
> (with a comment before it).

Yes, the +1 does seem to be needed, my mistake.

Jason

Re: Library ABI seriously broken!!

by Paolo Carlini-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Jason Merrill wrote:
> Yes, the +1 does seem to be needed, my mistake.
Thanks. So I went ahead and reapplied the patch with the +1 fixed, after
having double checked that the testsuites are now fine.

Paolo.

Re: Library ABI seriously broken!!

by Jason Merrill :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On 10/30/2009 06:27 PM, Paolo Carlini wrote:
> Thanks. So I went ahead and reapplied the patch with the +1 fixed, after
> having double checked that the testsuites are now fine.

The lengths do still need to be different on the two code paths, though.
  You added back the NULL for the normal case, but the '*' path still
lacks it.  I'll fix this.

Jason

Re: Library ABI seriously broken!!

by Paolo Carlini-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Jason Merrill wrote:
> The lengths do still need to be different on the two code paths,
> though.  You added back the NULL for the normal case, but the '*' path
> still lacks it.  I'll fix this.
Ah, ok, thanks.

Paolo.