From 5ffc05f7a72e9e0eb0906ba60c15a425e11214b0 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Sun, 28 Mar 2021 18:42:35 +0200 Subject: [PATCH 1/5] Fix HIFI_MEMORY_DEBUGGING on Linux/gcc It seems that libasan was running into some sort of trouble due to static linking. Additionally, -fstack-protector-strong has been enabled by disabling the usage of the FIR_1x4_AVX512 function under memory debugging. --- cmake/macros/MemoryDebugger.cmake | 11 ++++++++--- libraries/audio/src/AudioHRTF.cpp | 9 +++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/cmake/macros/MemoryDebugger.cmake b/cmake/macros/MemoryDebugger.cmake index 09716715f0..ebcd55a7dc 100644 --- a/cmake/macros/MemoryDebugger.cmake +++ b/cmake/macros/MemoryDebugger.cmake @@ -21,9 +21,14 @@ if (HIFI_MEMORY_DEBUGGING) SET(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -shared-libasan -fsanitize=undefined -fsanitize=address -fsanitize-recover=address") else () # for gcc on Linux - SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fsanitize=address -U_FORTIFY_SOURCE -fno-stack-protector -fno-omit-frame-pointer") - SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -static-libasan -static-libstdc++ -fsanitize=undefined -fsanitize=address") - SET(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -static-libasan -static-libstdc++ -fsanitize=undefined -fsanitize=address") + # For some reason, using -fstack-protector results in this error: + # usr/bin/ld: ../../libraries/audio/libaudio.so: undefined reference to `FIR_1x4_AVX512(float*, float*, float*, float*, float*, float (*) [64], int)' + # The '-DSTACK_PROTECTOR' argument below disables the usage of this function in the code. This should be fine as it only works on the latest Intel hardware, + # and is an optimization that should make no functional difference. + + SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fsanitize=address -U_FORTIFY_SOURCE -DSTACK_PROTECTOR -fstack-protector-strong -fno-omit-frame-pointer") + SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined -fsanitize=address") + SET(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined -fsanitize=address") endif() endif (UNIX) endif () diff --git a/libraries/audio/src/AudioHRTF.cpp b/libraries/audio/src/AudioHRTF.cpp index be7fecb450..fa8665d1a9 100644 --- a/libraries/audio/src/AudioHRTF.cpp +++ b/libraries/audio/src/AudioHRTF.cpp @@ -91,8 +91,8 @@ static const float azimuthTable[NAZIMUTH][3] = { // A first-order shelving filter is used to minimize the disturbance in ITD. // // Loosely based on data from S. Spagnol, "Distance rendering and perception of nearby virtual sound sources -// with a near-field filter model,” Applied Acoustics (2017) -// +// with a near-field filter model," Applied Acoustics (2017) +// static const int NNEARFIELD = 9; static const float nearFieldTable[NNEARFIELD][3] = { // { b0, b1, a1 } { 0.008410604f, -0.000262748f, -0.991852144f }, // gain = 1/256 @@ -388,7 +388,12 @@ void crossfade_4x2_AVX2(float* src, float* dst, const float* win, int numFrames) void interpolate_AVX2(const float* src0, const float* src1, float* dst, float frac, float gain); static void FIR_1x4(float* src, float* dst0, float* dst1, float* dst2, float* dst3, float coef[4][HRTF_TAPS], int numFrames) { +#ifndef STACK_PROTECTOR + // Enabling -fstack-protector on gcc causes an undefined reference to FIR_1x4_AVX512 here static auto f = cpuSupportsAVX512() ? FIR_1x4_AVX512 : (cpuSupportsAVX2() ? FIR_1x4_AVX2 : FIR_1x4_SSE); +#else + static auto f = cpuSupportsAVX2() ? FIR_1x4_AVX2 : FIR_1x4_SSE; +#endif (*f)(src, dst0, dst1, dst2, dst3, coef, numFrames); // dispatch } From 65c74986ee041964e4b85c98956bdff9533af32d Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Tue, 30 Mar 2021 00:35:54 +0200 Subject: [PATCH 2/5] Rename variable to VIRCADIA_MEMORY_DEBUGGING This is to make it consistent with an upcoming addition of VIRCADIA_THREAD_DEBUGGING. Also, check if the variable contains anything and isn't just defined to avoid hard to figure out bugs and confusion. --- cmake/macros/MemoryDebugger.cmake | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmake/macros/MemoryDebugger.cmake b/cmake/macros/MemoryDebugger.cmake index ebcd55a7dc..8afaab686e 100644 --- a/cmake/macros/MemoryDebugger.cmake +++ b/cmake/macros/MemoryDebugger.cmake @@ -8,11 +8,11 @@ # macro(SETUP_MEMORY_DEBUGGER) -if (DEFINED ENV{HIFI_MEMORY_DEBUGGING}) - SET( HIFI_MEMORY_DEBUGGING true ) +if (NOT "$ENV{VIRCADIA_MEMORY_DEBUGGING}" STREQUAL "") + SET( VIRCADIA_MEMORY_DEBUGGING true ) endif () -if (HIFI_MEMORY_DEBUGGING) +if (VIRCADIA_MEMORY_DEBUGGING) if (UNIX) if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") # for clang on Linux From 029e602075ed60e7f847dcf5d1757148d89b76ca Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Tue, 30 Mar 2021 00:37:33 +0200 Subject: [PATCH 3/5] Remove incorrect argument from clang memory debugger args, add leak detection. --- cmake/macros/MemoryDebugger.cmake | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cmake/macros/MemoryDebugger.cmake b/cmake/macros/MemoryDebugger.cmake index 8afaab686e..df62e40417 100644 --- a/cmake/macros/MemoryDebugger.cmake +++ b/cmake/macros/MemoryDebugger.cmake @@ -16,9 +16,9 @@ if (VIRCADIA_MEMORY_DEBUGGING) if (UNIX) if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") # for clang on Linux - SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer -shared-libasan -fsanitize=undefined -fsanitize=address -fsanitize-recover=address") - SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -shared-libasan -fsanitize=undefined -fsanitize=address -fsanitize-recover=address") - SET(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -shared-libasan -fsanitize=undefined -fsanitize=address -fsanitize-recover=address") + SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer -fsanitize=undefined -fsanitize=address -fsanitize=leak -fsanitize-recover=address") + SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined -fsanitize=address -fsanitize=leak -fsanitize-recover=address") + SET(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined -fsanitize=address -fsanitize=leak -fsanitize-recover=address") else () # for gcc on Linux # For some reason, using -fstack-protector results in this error: @@ -26,9 +26,9 @@ if (VIRCADIA_MEMORY_DEBUGGING) # The '-DSTACK_PROTECTOR' argument below disables the usage of this function in the code. This should be fine as it only works on the latest Intel hardware, # and is an optimization that should make no functional difference. - SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fsanitize=address -U_FORTIFY_SOURCE -DSTACK_PROTECTOR -fstack-protector-strong -fno-omit-frame-pointer") - SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined -fsanitize=address") - SET(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined -fsanitize=address") + SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fsanitize=address -fsanitize=leak -U_FORTIFY_SOURCE -DSTACK_PROTECTOR -fstack-protector-strong -fno-omit-frame-pointer") + SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined -fsanitize=address -fsanitize=leak ") + SET(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined -fsanitize=address -fsanitize=leak") endif() endif (UNIX) endif () From c02c38bf4a14fa0352ea267e3be96b6b7f12eb03 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Tue, 30 Mar 2021 00:38:23 +0200 Subject: [PATCH 4/5] Emit error if memory debugging isn't implemented on the platform --- cmake/macros/MemoryDebugger.cmake | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmake/macros/MemoryDebugger.cmake b/cmake/macros/MemoryDebugger.cmake index df62e40417..522c2b3d0b 100644 --- a/cmake/macros/MemoryDebugger.cmake +++ b/cmake/macros/MemoryDebugger.cmake @@ -30,6 +30,8 @@ if (VIRCADIA_MEMORY_DEBUGGING) SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined -fsanitize=address -fsanitize=leak ") SET(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined -fsanitize=address -fsanitize=leak") endif() - endif (UNIX) + else() + message(FATAL_ERROR "Memory debugging is not supported on this platform." ) + endif() endif () endmacro(SETUP_MEMORY_DEBUGGER) From 7e914796149fdf4579d40d2be0a836bb995e310f Mon Sep 17 00:00:00 2001 From: daleglass <51060919+daleglass@users.noreply.github.com> Date: Thu, 1 Apr 2021 23:16:49 +0200 Subject: [PATCH 5/5] Update cmake/macros/MemoryDebugger.cmake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Julian Groß --- cmake/macros/MemoryDebugger.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/macros/MemoryDebugger.cmake b/cmake/macros/MemoryDebugger.cmake index 522c2b3d0b..5aa99787bf 100644 --- a/cmake/macros/MemoryDebugger.cmake +++ b/cmake/macros/MemoryDebugger.cmake @@ -8,7 +8,7 @@ # macro(SETUP_MEMORY_DEBUGGER) -if (NOT "$ENV{VIRCADIA_MEMORY_DEBUGGING}" STREQUAL "") +if ("$ENV{VIRCADIA_MEMORY_DEBUGGING}") SET( VIRCADIA_MEMORY_DEBUGGING true ) endif ()