[compiler-rt] Make DemangleSwiftAndCXX robust against FreeBSD __cxa_demangle

[zaks.anna]

Summary

The sanitizer unit tests fail on FreeBSD because __cxa_demangle returns invalid demanglings when dealing with non-C++ mangled names. It is not guaranteed to return the same string back. If the input string is not a C++ mangling, don't try to demangle it.

Reid Kleckner <[e-mail]>:

"
This test is failing on FreeBSD, it appears because __cxa_demangle consumes the leading 'f' of foo and returns "float":
http://lab.llvm.org:8011/builders/sanitizer_x86_64-freebsd/builds/12709/steps/test%20sanitizer/logs/stdio
FAIL: SanitizerCommon-Unit :: Sanitizer-x86_64-Test/Symbolizer.DemangleSwiftAndCXX (292 of 309)

• TEST 'SanitizerCommon-Unit :: Sanitizer-x86_64-Test/Symbolizer.DemangleSwiftAndCXX' FAILED ****

Note: Google Test filter = Symbolizer.DemangleSwiftAndCXX
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from Symbolizer
[ RUN ] Symbolizer.DemangleSwiftAndCXX
/usr/home/buildslave/slave_as-bldslv5/sanitizer_x86_64-freebsd/llvm.src/projects/compiler-rt/lib/sanitizer_common/tests/sanitizer_symbolizer_test.cc:65: Failure
Value of: DemangleSwiftAndCXX("foo")

Actual: "float"

Expected: "foo"
[ FAILED ] Symbolizer.DemangleSwiftAndCXX (3 ms)
[----------] 1 test from Symbolizer (3 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (7 ms total)
[ PASSED ] 0 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] Symbolizer.DemangleSwiftAndCXX

1 FAILED TEST
"

[dim]

@zaks.anna, I had already created D22939, sorry. But there is a fly in the ointment, alas: some other tests start failing because of this fix. :(

[zaks.anna]

No Problem! If you do make it work, I slightly prefer this implementation since it's symmetrical with DemangleSwift.

[dim]

In D22953#500654, @zaks.anna wrote:

No Problem! If you do make it work, I slightly prefer this implementation since it's symmetrical with DemangleSwift.

I like the symmetry too, but on the other hand, neither DemangleCXXABI nor DemangleSwift are used anywhere outside this source file, so maybe they should be static? And since there is already a nullptr check for the name in DemangleSwiftAndCXX, the nullptr checks in DemangleCXXABI and DemangleSwift can be removed.

[zaks.anna]

It does make sense to make them static. If you decide to remove the check, please, replace it with an assertion.

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

namespace __cxxabiv1 {
   char *__cxa_demangle(const char *mangled, char *buffer,
                                   size_t *length, int *status);
 }
 
 namespace __sanitizer {
 
 // Attempts to demangle the name via __cxa_demangle from __cxxabiv1.
 const char *DemangleCXXABI(const char *name) {
+  if (!name) return nullptr;
+
+  // Check if we are dealing with a C++ mangled name first.
+  if (name[0] != '_' || name[1] != 'Z') {
+    return nullptr;
+  }
+
   // FIXME: __cxa_demangle aggressively insists on allocating memory.
   // There's not much we can do about that, short of providing our
   // own demangler (libc++abi's implementation could be adapted so that
   // it does not allocate). For now, we just call it anyway, and we leak
   // the returned value.
   if (__cxxabiv1::__cxa_demangle)
     if (const char *demangled_name =
           __cxxabiv1::__cxa_demangle(name, 0, 0, 0))
       return demangled_name;
 
-  return name;
+  return nullptr;
 }
 
 // As of now, there are no headers for the Swift runtime. Once they are
 // present, we will weakly link since we do not require Swift runtime to be
 // linked.
 typedef char *(*swift_demangle_ft)(const char *mangledName,
                                    size_t mangledNameLength, char *outputBuffer,
                                    size_t *outputBufferSize, uint32_t flags);
const char *DemangleSwift(const char *name) {
 
   return nullptr;
 }
 
 const char *DemangleSwiftAndCXX(const char *name) {
   if (!name) return nullptr;
   if (const char *swift_demangled_name = DemangleSwift(name))
     return swift_demangled_name;
-  return DemangleCXXABI(name);
+  if (const char *cxx_demangled_name = DemangleCXXABI(name))
+    return cxx_demangled_name;
+  return name;
 }
 
 bool SymbolizerProcess::StartSymbolizerSubprocess() {
   if (!FileExists(path_)) {
     if (!reported_invalid_path_) {
       Report("WARNING: invalid path to external symbolizer!\n");
       reported_invalid_path_ = true;
     }