This is an archive of the discontinued LLVM Phabricator instance.

Differential D123736

[Darwin][ASan][Sanitizer] Fixes Sanitizer NonUnique Identifier to Account for Mac arm64 architectures.
ClosedPublic

Authored by thetruestblue on Apr 13 2022, 2:31 PM.

Details

Reviewers
yln
rsundahl
delcypher
aralisza
Commits
rGe0c44544655d: [Darwin][ASan][Sanitizer] Fixes Sanitizer NonUnique Identifier to Account for…
Summary

Current check assumes iOS as the only Apple devices running arm64.

#if SANITIZER_MAC && !(defined(__arm64__) && SANITIZER_IOS)

Stops Apple Silicon from being flagged as requiring unique RTTI.
This introduced unexpected behavior within the sanitizer.

This patch fixes existing test on arm64 Mac:
compiler-rt/test/ubsan/TestCases/TypeCheck/vptr-non-unique-typeinfo.cpp

rdar://91446703

Diff Detail

Unit TestsFailed

TimeTest
1,510 msx64 debian > Clang.utils/update_cc_test_checks::check-globals.test
630 msx64 debian > Clang.utils/update_cc_test_checks::global-hex-value-regex.test
630 msx64 debian > Clang.utils/update_cc_test_checks::global-value-regex.test
820 msx64 debian > SanitizerCommon-asan-x86_64-Linux.Linux::decorate_proc_maps.cpp
780 msx64 debian > SanitizerCommon-asan-x86_64-Linux.Posix::sem_open.cpp
View Full Test Results (51 Failed)

Event Timeline

thetruestblue created this revision.Apr 13 2022, 2:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2022, 2:31 PM
Herald added a subscriber: kristof.beyls. · View Herald Transcript
thetruestblue requested review of this revision.Apr 13 2022, 2:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2022, 2:31 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
thetruestblue edited the summary of this revision. (Show Details)Apr 13 2022, 2:32 PM
Harbormaster completed remote builds in B159553: Diff 422657.Apr 13 2022, 2:48 PM
thetruestblue edited the summary of this revision. (Show Details)Apr 13 2022, 2:56 PM
yln accepted this revision.Apr 13 2022, 3:24 PM

LGTM, thanks!

This revision is now accepted and ready to land.Apr 13 2022, 3:24 PM
This revision was landed with ongoing or failed builds.Apr 13 2022, 9:16 PM
Closed by commit rGe0c44544655d: [Darwin][ASan][Sanitizer] Fixes Sanitizer NonUnique Identifier to Account for… (authored by Blue Gaston <bgaston2@apple.com>). · Explain Why
This revision was automatically updated to reflect the committed changes.
Blue Gaston <bgaston2@apple.com> added a commit: rGe0c44544655d: [Darwin][ASan][Sanitizer] Fixes Sanitizer NonUnique Identifier to Account for….
rsundahl added a comment.Apr 20 2022, 8:39 AM

It may be more canonical to use:

#if SANITIZER_MAC && SANITIZER_X64

Reasoning: While _M_X64 is included in SANITIZER_X64 (along with x86_64), it's Windows-only and is clearly intended as an alias or alternate for x86_64 w.r.t. sanitizer.

Revision Contents

PathSize
compiler-rt/
lib/
sanitizer_common/
2 lines

Diff 422657

compiler-rt/lib/sanitizer_common/sanitizer_platform.h

Show First 20 Lines • Show All 334 Lines • ▼ Show 20 Lines
/// * 1800: Microsoft Visual Studio 2013 / 12.0 /// * 1800: Microsoft Visual Studio 2013 / 12.0
/// * 1900: Microsoft Visual Studio 2015 / 14.0 /// * 1900: Microsoft Visual Studio 2015 / 14.0
#ifdef _MSC_VER #ifdef _MSC_VER
# define MSC_PREREQ(version) (_MSC_VER >= (version)) # define MSC_PREREQ(version) (_MSC_VER >= (version))
#else #else
# define MSC_PREREQ(version) 0 # define MSC_PREREQ(version) 0
#endif #endif
#if SANITIZER_MAC && !(defined(__arm64__) && SANITIZER_IOS) #if SANITIZER_MAC && defined (__x86_64__)
# define SANITIZER_NON_UNIQUE_TYPEINFO 0 # define SANITIZER_NON_UNIQUE_TYPEINFO 0
#else #else
# define SANITIZER_NON_UNIQUE_TYPEINFO 1 # define SANITIZER_NON_UNIQUE_TYPEINFO 1
#endif #endif
// On linux, some architectures had an ABI transition from 64-bit long double // On linux, some architectures had an ABI transition from 64-bit long double
// (ie. same as double) to 128-bit long double. On those, glibc symbols // (ie. same as double) to 128-bit long double. On those, glibc symbols
// involving long doubles come in two versions, and we need to pass the // involving long doubles come in two versions, and we need to pass the
▲ Show 20 Lines • Show All 75 Lines • Show Last 20 Lines