Bug 52539 - : TypeInfoHelper::getClassName() may crash if cppunit is compiled with older compiler
Summary: : TypeInfoHelper::getClassName() may crash if cppunit is compiled with older ...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: Other All
: medium normal
Assignee: Markus Mohrhard
URL:
Whiteboard: BSA
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-26 13:08 UTC by Andriy Gapon
Modified: 2012-08-14 15:32 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
proposed patch (550 bytes, patch)
2012-07-26 13:08 UTC, Andriy Gapon
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andriy Gapon 2012-07-26 13:08:36 UTC
Created attachment 64731 [details]
proposed patch

It seems that GCC ABI compatibility is not strict enough and has a hidden "gem".  TypeInfoHelper::getClassName() method may produce a crash of cppunit library is compiled with older GCC (e.g. GCC 4.2) and the method is used to inquire information about code produced with newer GCC (e.g. GCC 4.6).
Note that there would not be any ABI incompatibilities detected at link time or run-time, but the code would crash.

Let's try to unravel this issue step by step.
The crash happens because of std::logic_error exception with what being "basic_string::_S_construct NULL".

This exception happens when an std::string is constructed from a NULL C-string.

According to a backtrace that I obtained the exception is triggered in src/cppunit/TypeInfoHelper.cpp, TypeInfoHelper::getClassName() method, at the following lines:

  c_name = abi::__cxa_demangle( info.name(), 0, 0, &status );

  std::string name( c_name );

The immediate cause of the crash is that the cppunit code doesn't verify neither status produced by abi::__cxa_demangle nor its return value.  In other words, the code doesn't expect that abi::__cxa_demangle may fail and doesn't properly handle its failure.
This could be considered a bug on its own.
In general std::type_info::name() is not supposed to a return a name that abi::__cxa_demangle would not understand.  But it's still better to be on the safe side and produce useful diagnostics if something goes wrong.

In my environment I determined that depending on a compiler used to compile the cppunit library std::type_info::name() returns either "*N12_GLOBAL__N_14TestE" for gcc 4.2 or
"N12_GLOBAL__N_14TestE" for 4.6 (without the leading asterisk) in the test where the crash happens.

The mangled name with the leading asterisk is not valid according to C++ ABI, but it is produced by GCC 4.6.

It seems that GCC 4.6 uses that leading asterisk for some internal purposed and it is supposed to be hidden/ignored.

Unfortunately, it seems that std::type_info::name() is an inline method and thus it can not be really used to guarantee ABI compatibility across GCC versions.
To show what I mean here is how std::type_info::name() is implemented in GCC 4.2:
    const char* name() const
    { return __name; }
Now the same from GCC 4.6:
    const char* name() const
    { return __name[0] == '*' ? __name + 1 : __name; }

So, as we can see, the later GCC knows about the asterisk and hides it from callers.  GCC 4.2 doesn't know about it and so it returns __name as is.  Due to inlining the code compiled by GCC 4.2 may incorrectly return typeinfo::name for code compiled with GCC 4.6.
If std::type_info::name() was not inlined, then a version from libstdc++ would be used and it would be impossible to call the 4.2 version while dealing with 4.6 code.

So this is the trigger of the problem.  The attached patch could be used to work around the problem by applying the same logic as type_info::name() in GCC 4.6 uses.

The patch doesn't add error handling that I mentioned above.
Comment 1 Markus Mohrhard 2012-08-07 15:09:51 UTC
Will look into that one tonight or tomorrow.
Comment 2 Markus Mohrhard 2012-08-07 21:51:13 UTC
Ok, I implemented a simple fix for it that should prevent the crash in case of failed demangling. I need to think more about your patch and about some implication in case of a pure gcc 4.2 environment.
Comment 3 Markus Mohrhard 2012-08-07 21:54:00 UTC
http://cgit.freedesktop.org/libreoffice/cppunit/commit/?id=71fddd30ba03374fde7d740085f10a17be17f106

The next step would be a better check against the status value of abi::__cxa_demangle .
Comment 4 Markus Mohrhard 2012-08-14 15:04:47 UTC
Ok, I applied your patch with some slight modifications by me.