From 808cc65e2ef030a3b62eb0b7ec3657363f667b45 Mon Sep 17 00:00:00 2001 From: Nathaniel Cesario Date: Mon, 25 Sep 2023 11:30:52 -0600 Subject: [PATCH 1/2] Add ASAN integration testing Add jobs for testing with -fsanitize=address and -fsanitize=thread. --- .github/workflows/continuous_integration.yml | 53 ++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/.github/workflows/continuous_integration.yml b/.github/workflows/continuous_integration.yml index 73446847b2..090fbaa4e4 100644 --- a/.github/workflows/continuous_integration.yml +++ b/.github/workflows/continuous_integration.yml @@ -64,6 +64,59 @@ jobs: ctest --output-on-failure && cd ../Test && ./runtests + linux-asan: + runs-on: ubuntu-22.04 + strategy: + fail-fast: false + matrix: + compiler: [{cc: gcc, cxx: g++}] + cmake_build_type: [Debug] + flags: ['-fsanitize=address', '-fsanitize=thread'] + steps: + - uses: actions/checkout@3df4ab11eba7bda6032a0b82a6bb43b11571feac # v4.0.0 + - uses: lukka/get-cmake@4dcd3eb73017c61159eb130746fbca35d78a3d5f # v3.27.4 + - uses: actions/setup-python@61a6322f88396a6271a6ee3565807d608ecaddd1 # v4.7.0 + with: + python-version: '3.7' + - name: Setup ccache + uses: hendrikmuhs/ccache-action@6d1841ec156c39a52b1b23a810da917ab98da1f4 # v1.2.10 + with: + key: ubuntu-22-${{ matrix.cmake_build_type }}-${{ matrix.compiler.cc }}-${{matrix.compiler.cxx}}-${{matrix.flags}} + - name: Install GoogleTest + run: | + # check out pre-breakage version of googletest; can be deleted when + # issue 3128 is fixed + # git clone --depth=1 https://github.com/google/googletest.git External/googletest + mkdir -p External/googletest + cd External/googletest + git init + git remote add origin https://github.com/google/googletest.git + git fetch --depth 1 origin 0c400f67fcf305869c5fb113dd296eca266c9725 + git reset --hard FETCH_HEAD + cd ../.. + - name: Update Glslang Sources + run: ./update_glslang_sources.py + - name: Configure + run: cmake -S . -B build -D CMAKE_BUILD_TYPE=${{ matrix.cmake_build_type }} + env: + CC: ${{matrix.compiler.cc}} + CXX: ${{matrix.compiler.cxx}} + CMAKE_GENERATOR: Ninja + CMAKE_C_COMPILER_LAUNCHER: ccache + CMAKE_CXX_COMPILER_LAUNCHER: ccache + CFLAGS: ${{matrix.flags}} + CXXFLAGS: ${{matrix.flags}} + LDFLAGS: ${{matrix.flags}} + - name: Build + run: cmake --build build + - name: Install + run: cmake --install build --prefix build/install + - name: Test + run: | + cd build + ctest --output-on-failure && + cd ../Test && ./runtests + # Ensure we can compile/run on an older distro linux_min: name: Linux Backcompat From 1a79acee99613ee9cde6159f820ae3dd49ab0580 Mon Sep 17 00:00:00 2001 From: Nathaniel Cesario Date: Tue, 26 Sep 2023 11:50:10 -0600 Subject: [PATCH 2/2] Fix race condition identified by TSAN Guard global variables 'CompileFailed' and 'LinkFailed' in the StandAlone application with atomics. --- StandAlone/StandAlone.cpp | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/StandAlone/StandAlone.cpp b/StandAlone/StandAlone.cpp index 8833e38eb2..b31a644941 100644 --- a/StandAlone/StandAlone.cpp +++ b/StandAlone/StandAlone.cpp @@ -51,15 +51,16 @@ #include "../SPIRV/doc.h" #include "../SPIRV/disassemble.h" -#include -#include +#include +#include #include #include -#include +#include +#include #include #include -#include #include +#include #include "../glslang/OSDependent/osinclude.h" @@ -144,8 +145,9 @@ void FreeFileData(char* data); void InfoLogMsg(const char* msg, const char* name, const int num); // Globally track if any compile or link failure. -bool CompileFailed = false; -bool LinkFailed = false; +std::atomic CompileFailed{0}; +std::atomic LinkFailed{0}; +std::atomic CompileOrLinkFailed{0}; // array of unique places to leave the shader names and infologs for the asynchronous compiles std::vector> WorkItems; @@ -1166,6 +1168,7 @@ void CompileShaders(glslang::TWorklist& worklist) if (Options & EOptionDebug) Error("cannot generate debug information unless linking to generate code"); + // NOTE: TWorkList::remove is thread-safe glslang::TWorkItem* workItem; if (Options & EOptionStdin) { if (worklist.remove(workItem)) { @@ -1442,7 +1445,7 @@ void CompileAndLinkShaderUnits(std::vector compUnits) if (shader->preprocess(GetResources(), defaultVersion, ENoProfile, false, false, messages, &str, includer)) { PutsIfNonEmpty(str.c_str()); } else { - CompileFailed = true; + CompileFailed = 1; } StderrIfNonEmpty(shader->getInfoLog()); StderrIfNonEmpty(shader->getInfoDebugLog()); @@ -1450,7 +1453,7 @@ void CompileAndLinkShaderUnits(std::vector compUnits) } if (! shader->parse(GetResources(), defaultVersion, false, messages, includer)) - CompileFailed = true; + CompileFailed = 1; if (!compileOnly) program.addShader(shader); @@ -1496,7 +1499,9 @@ void CompileAndLinkShaderUnits(std::vector compUnits) // Dump SPIR-V if (Options & EOptionSpv) { - if (CompileFailed || LinkFailed) + CompileOrLinkFailed.fetch_or(CompileFailed); + CompileOrLinkFailed.fetch_or(LinkFailed); + if (static_cast(CompileOrLinkFailed.load())) printf("SPIR-V is not generated for failed compile or link\n"); else { std::vector intermediates; @@ -1555,7 +1560,9 @@ void CompileAndLinkShaderUnits(std::vector compUnits) } } - if (depencyFileName && !(CompileFailed || LinkFailed)) { + CompileOrLinkFailed.fetch_or(CompileFailed); + CompileOrLinkFailed.fetch_or(LinkFailed); + if (depencyFileName && !static_cast(CompileOrLinkFailed.load())) { std::set includedFiles = includer.getIncludedFiles(); sources.insert(sources.end(), includedFiles.begin(), includedFiles.end()); @@ -1731,9 +1738,9 @@ int singleMain() ShFinalize(); } - if (CompileFailed) + if (CompileFailed.load()) return EFailCompile; - if (LinkFailed) + if (LinkFailed.load()) return EFailLink; return 0;