On 08/16/2013 08:26 PM, Richard Smith wrote:
On Fri, Aug 16, 2013 at 2:38 AM, Stephan Bergmann <[email protected] <mailto:[email protected]>> wrote:I was trying out -fsanitizer=undefined on the LibreOffice code base (with a recent Clang trunk build), and ran into false positives claiming member calls were made on base class subobjects of wrong type. Turns out LibreOffice is somewhat over-eager with -fvisibility=hidden, leading to some classes that are used across multiple dynamic libraries nevertheless having their RTTI name symbols bound locally per library. While that is in violation of the Itanium ABI, it at least works with recent libstdc++ (which resorts to strcmp rather than pointer comparison these days, see [1])---and wouldn't normally get noticed anyway as there should not be uses of dynamic_cast involving those classes in the LibreOffice code base. I would much prefer for this to be first resolved in the Itanium ABI. The fix you suggest below will hide real problems in code that uses libc++ (where type_info comparison directly compares the __type_name fields). Also, it seems to me that you've found a real LibreOffice bug, if they're hiding the type information for exported types, and I don't want to weaken a sanitizer because it found a real bug.
I'm not sure it is that clear cut. For one, it is arguably beneficial for LibreOffice to reduce the number of runtime (data) relocations by "illegally" reducing classes to hidden visibility that are used across multiple DSOs but for which it is known that their RTTI is not used (the classes in question are special in that they are analogous to Java interfaces, have only pure virtual member functions and no data members, so no harm results from reducing their visibility in practice). (But I could of course solve my original problem on the LibreOffice side instead of the ubsan side, by making those classes non-hidden in builds that use -fsanitizer=undefined.)
For another, the real problems that would be hidden by my suggested fix would necessarily involve ODR violations, right? (In the sense that two DSOs would contain non-exported classes with equal identifiers that are, or should be considered, different.)
From a more abstract POV, I think libstdc++ may have been misguided in trying to make the RTLD_LOCAL case "work". For instance, if I load two versions of the same DSO into my binary using RTLD_LOCAL, and use their type_info*s as keys in some container type (say, in a serialization library), there is no way to avoid a broken program. Perhaps it would have been better for them to admit that types with the same name from two different RTLD_LOCAL DSOs are different, just like every other entity.
I'm not sure I understand in how far you consider your example program broken. If the container uses type_info* keys and pointer equality on them, what will happen is that there can be different elements for which key1->operator==(*key2) is true. But that is just in line with the C++ Standard.
Stephan _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
