segfault with clang

[malytomas]

repro case: https://github.com/malytomas/optick-test-clang

when compiling Optick and/or the application that uses Optick with clang++ on linux with debug symbols and enabled optimizations, the application crashes

[topin89]

Try to compile with -fsanitize=undefined compiler and linker flags, not sure how do it with CMake.

I got same crash with standard ConsoleApp example, and when I compiled it with -fsanitize=undefined I got this:

Starting profiler test.
../../src/optick_memory.h:67:42: runtime error: constructor call on misaligned address 0x7f6277480018 for type 'Optick::MemoryChunk<Optick::EventDescription, 4096>', which requires 64 byte alignment
0x7f6277480018: note: pointer points here
 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^
../../src/optick_memory.h:147:3: runtime error: constructor call on misaligned address 0x7f6277480018 for type 'Optick::MemoryChunk<Optick::EventDescription, 4096> *', which requires 64 byte alignment
0x7f6277480018: note: pointer points here
 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^
UndefinedBehaviorSanitizer:DEADLYSIGNAL
==31644==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f6276f382d4 bp 0x7f62771f8d50 sp 0x7fffddde82f0 T31644)
==31644==The signal is caused by a READ memory access.
==31644==Hint: address points to the zero page.
    #0 0x7f6276f382d3 in Optick::MemoryChunk<Optick::EventDescription, 4096u>::MemoryChunk() /mnt/d/dumpster_rep/optick-1.2.3.0/build/gmake/../../src/optick_memory.h:147:28
    #1 0x7f6276f382d3 in Optick::MemoryChunk<Optick::EventDescription, 4096u>* Optick::Memory::New<Optick::MemoryChunk<Optick::EventDescription, 4096u> >() /mnt/d/dumpster_rep/optick-1.2.3.0/build/gmake/../../src/optick_memory.h:67
    #2 0x7f6276f03b86 in Optick::MemoryPool<Optick::EventDescription, 4096u>::AddChunk() /mnt/d/dumpster_rep/optick-1.2.3.0/build/gmake/../../src/optick_memory.h:183:23
    #3 0x7f6276f03b86 in Optick::MemoryPool<Optick::EventDescription, 4096u>::Add() /mnt/d/dumpster_rep/optick-1.2.3.0/build/gmake/../../src/optick_memory.h:206
    #4 0x7f6276f03b86 in Optick::EventDescriptionBoard::CreateDescription(char const*, char const*, unsigned int, unsigned int, unsigned int) /mnt/d/dumpster_rep/optick-1.2.3.0/build/gmake/../../src/optick_core.cpp:435
    #5 0x7f6276f12846 in Optick::EventDescription::Create(char const*, char const*, unsigned long, unsigned long, unsigned long) /mnt/d/dumpster_rep/optick-1.2.3.0/build/gmake/../../src/optick_core.cpp:202:38
    #6 0x7f6276f12846 in Optick::Core::Core() /mnt/d/dumpster_rep/optick-1.2.3.0/build/gmake/../../src/optick_core.cpp:1086
    #7 0x7f6276f178e0 in Optick::Core::Get() /mnt/d/dumpster_rep/optick-1.2.3.0/build/gmake/../../src/optick_core.cpp:726:14
    #8 0x7f6276f178e0 in Optick::SetStateChangedCallback(bool (*)(Optick::State::Type)) /mnt/d/dumpster_rep/optick-1.2.3.0/build/gmake/../../src/optick_core.cpp:1520
    #9 0x42d49d in Test::Engine::Engine() /mnt/d/dumpster_rep/optick-1.2.3.0/build/gmake/../../samples/Common/TestEngine/TestEngine.cpp:502:2
    #10 0x42bb49 in main /mnt/d/dumpster_rep/optick-1.2.3.0/build/gmake/../../samples/ConsoleApp/main.cpp:25:15
    #11 0x7f6275ad1b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #12 0x404699 in _start (/mnt/d/dumpster_rep/optick-1.2.3.0/bin/gmake/x64/Release/ConsoleApp+0x404699)

UndefinedBehaviorSanitizer can not provide additional info.
==31644==ABORTING

Key words are: constructor call on misaligned address <...> which requires 64 byte alignment

optick_memory.h:67:42 is a placement new call: return new (Memory::Alloc(sizeof(T))) T();

My guess is there is some SIMD optimization somewhere, probably in Optick::EventDescription on -O3 and that requires whole class to be 64 byte aligned and Memory::Alloc knows nothing about alignment so we got what we got.

Update:

It was indeed an alignment problem. Speed hack that solves it is here: https://github.com/topin89/optick/commit/dac204ce6e5d30ef50ef0efdcf628b11e7b4e8a9

Not sure how to solve it properly, best I can think of is using std::align and alignof(T) and some overallocation.

[bombomby]

@topin89 thanks a lot for the deep analysis of the issue. I've just removed unnecessary alignment which was causing the problem. Please let me know if you still bump into this problem after grabbing my fix https://github.com/bombomby/optick/commit/c81a0235d03125982d10a779830403d730954333.

[topin89]

@bombomby You're welcome! Just checked with clang sanitizers, there was no problem.

There was small problem with gcc.

$ UBSAN_OPTIONS=print_stacktrace=1 ./ConsoleApp
Starting profiler test.
Engine successfully created.
Starting main loop update.
../../src/optick_core.cpp:58:21: runtime error: load of misaligned address 0x7fb1eda5619b for type 'const uint64_t', which requires 8 byte alignment
0x7fb1eda5619b: note: pointer points here
 47  50 55 00 47 50 55 20 46  72 61 6d 65 00 2f 75 73  72 2f 69 6e 63 6c 75 64  65 2f 63 2b 2b 2f 38
              ^
    #0 0x7fb1ed8b4e5d in Optick::MurmurHash64A(void const*, int, unsigned long) ../../src/optick_core.cpp:58
    #1 0x7fb1ed8fda73 in Optick::StringHash::StringHash(char const*) ../../src/optick_core.h:31
    #2 0x7fb1ed8fda73 in Optick::EventDescriptionBoard::CreateSharedDescription(char const*, char const*, unsigned int, unsigned int, unsigned int) ../../src/optick_core.cpp:447
    #3 0x7fb1eda53af4 in Test::Engine::Draw() ../../samples/Common/TestEngine/TestEngine.cpp:250
    #4 0x7fb1eda53af4 in Test::Engine::Update() ../../samples/Common/TestEngine/TestEngine.cpp:180
    #5 0x7fb1eda4eba3 in main ../../samples/ConsoleApp/main.cpp:41
    #6 0x7fb1ec78409a in __libc_start_main ../csu/libc-start.c:308
    #7 0x7fb1eda4ef69 in _start (/mnt/c/Users/topin/Downloads/optick-master/bin/gmake/x64/Release/ConsoleApp+0x7f69)

$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/8/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 8.3.0-6' --with-bugurl=file:///usr/share/doc/gcc-8/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr --with-gcc-major-version-only --program-suffix=-8 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib --enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 8.3.0 (Debian 8.3.0-6)

Looks like at least gcc 8.3.0 do not align C stings literals by 8 (like "GPU Frame" here). On x86 platform, that's a minor slowdown. On ARM cores, this will result in bus fault, but it's not supported anyway. So gcc is also OK.