[UBSan] Fix isDerivedFromAtOffset on iOS ARM64

[filcab]

Summary

iOS on ARM64 doesn't unique RTTI.
Ref: clang's iOS64CXXABI::shouldRTTIBeUnique()

Due to this, pointer-equality will not necessarily work in this
architecture, across dylib boundaries.

dynamic_cast<>() will still work, since Apple ships with one prepared
for this, but we can't rely on it by simply comparing the type names for
pointer-equality.

I've limited the expensive strcmp check to the specific architecture
which needs it.

Example which triggers this bug:

lib.h:

struct X {
  virtual ~X() {}
};
X *libCall();

lib.mm:

X *libCall() {
  return new X;
}

prog.mm:

int main() {
  X *px = libCall();
  delete px;
}

Expected output: Nothing
Actual output:

<unknown>: runtime error: member call on address 0x00017001ef50 which does not point to an object of type 'X'
0x00017001ef50: note: object is of type 'X'
 00 00 00 00  60 00 0f 00 01 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for ‘X’

Compilation is fairly straightforward (be sure to pass -isysroot,
-mios-min-version, etc). I ended up looking at what Xcode did to compile
and built a script.

No test since it doesn't seem we support anything related to running on
iOS devices. I couldn't even figure out a nice way to do it from the
command line.

[filcab]

I erred on the side of caution by being verbose in the commit message.
The problem is non-obvious and I want anyone who might think about removing that line to have no excuse (assuming they blame that line and see the commit ;-) ).

This might happen in other platforms too… :-)

[filcab]

Reword the dynamic_cast paragraph (wasn't making sense).

[filcab]

Ping.

[filcab]

Ping! (Adding Anna Zaks)

[samsonov]

LGTM. Sorry, for some reason I've lost track of this :-/

[zaks.anna]

No test since it doesn't seem we support anything related to running on
iOS devices. I couldn't even figure out a nice way to do it from the
command line.

The test case you provided should be passing on all architectures, correct? If so, why not just add it?
(We do have a way to run tests on iOS devices internally.)

[filcab]

I can try to add a generic "darwin" test (maybe even darwin+linux, but
adding Windows might be harder, since the platform is so different). This
test would pass both before and after my change, though (in every platform
I can test).
Do your internal test harnesses account for tests which need
executable+dylib (two files) to run?

Filipe

[zaks.anna]

I can try to add a generic "darwin" test (maybe even darwin+linux, ...

I think this would be overall goodness.

Do your internal test harnesses account for tests which need
executable+dylib (two files) to run?

Yes. (Though, we are only testing ASan at the moment.)

[zaks.anna]

I can try to add a generic "darwin" test (maybe even darwin+linux, ...

I think this would be overall goodness.

Do your internal test harnesses account for tests which need
executable+dylib (two files) to run?

Yes. (Though, we are only testing ASan at the moment.)

[filcab]

Sorry, I totally forgot about this revision. Refreshing it, with a test.

I'm not sure how well the test will run on all platforms (no idea about Windows), especially on the ones that really need it (iOS ARM64 is impossible to test outside of Apple, AFAICT).

compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform.h

 ///  * 1800: Microsoft Visual Studio 2013 / 12.0
 ///  * 1900: Microsoft Visual Studio 2015 / 14.0
 #ifdef _MSC_VER
 # define MSC_PREREQ(version) (_MSC_VER >= (version))
 #else
 # define MSC_PREREQ(version) 0
 #endif
 
+#if defined(__arm64__) && SANITIZER_IOS
+# define SANITIZER_NON_UNIQUE_TYPEINFO 1
+#else
+# define SANITIZER_NON_UNIQUE_TYPEINFO 0
+#endif
+
 #endif // SANITIZER_PLATFORM_H

compiler-rt/trunk/lib/ubsan/ubsan_type_hash_itanium.cc

static __ubsan::HashValue *getTypeCacheHashTableBucket(__ubsan::HashValue V) {
   return &__ubsan_vptr_hash_set[First];
 }
 
 /// \brief Determine whether \p Derived has a \p Base base class subobject at
 /// offset \p Offset.
 static bool isDerivedFromAtOffset(const abi::__class_type_info *Derived,
                                   const abi::__class_type_info *Base,
                                   sptr Offset) {
-  if (Derived->__type_name == Base->__type_name)
+  if (Derived->__type_name == Base->__type_name ||
+      (SANITIZER_NON_UNIQUE_TYPEINFO &&
+       !internal_strcmp(Derived->__type_name, Base->__type_name)))
     return Offset == 0;
 
   if (const abi::__si_class_type_info *SI =
         dynamic_cast<const abi::__si_class_type_info*>(Derived))
     return isDerivedFromAtOffset(SI->__base_type, Base, Offset);
 
   const abi::__vmi_class_type_info *VTI =
     dynamic_cast<const abi::__vmi_class_type_info*>(Derived);

compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/Helpers/lit.local.cfg

+# Sources in this directory are helper files for tests which test functionality
+# involving multiple translation units.
+config.suffixes = []

compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/Helpers/vptr-non-unique-typeinfo-lib.h

+struct X {
+  virtual ~X() {}
+};
+X *libCall();

compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/Helpers/vptr-non-unique-typeinfo-lib.cpp

+#include "vptr-non-unique-typeinfo-lib.h"
+
+X *libCall() {
+  return new X;
+}

compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/vptr-non-unique-typeinfo.cpp

+// RUN: %clangxx -frtti -fsanitize=vptr -fno-sanitize-recover=vptr -I%p/Helpers %p/Helpers/vptr-non-unique-typeinfo-lib.cpp -fPIC -shared -o %t-lib.so
+// RUN: %clangxx -frtti -fsanitize=vptr -fno-sanitize-recover=vptr -I%p/Helpers -g %s -O3 -o %t %t-lib.so
+// RUN: %run %t
+
+#include "vptr-non-unique-typeinfo-lib.h"
+
+int main() {
+  X *px = libCall();
+  delete px;
+}