This is an archive of the discontinued LLVM Phabricator instance.

[sanitizer] Add CMake flag to build with internal symbolizer
ClosedPublic

Authored by vitalybuka on Aug 14 2023, 7:06 PM.

Details

Summary

This intermediate result in moving internal symbolizer build
from sh script to CMake rules.

The flag is supposed to be used with:
-DLLVM_ENABLE_PROJECTS="clang;lld;compiler-rt" -DLLVM_ENABLE_RUNTIMES="libunwind;libcxx;libcxxabi" -Sllvm-project/llvm

After converting sh script into cmake, we may add support for other build modes.

For https://github.com/llvm/llvm-project/issues/30098

Diff Detail

Event Timeline

vitalybuka created this revision.Aug 14 2023, 7:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 14 2023, 7:06 PM
Herald added a subscriber: Enna1. · View Herald Transcript
vitalybuka requested review of this revision.Aug 14 2023, 7:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 14 2023, 7:06 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
Enna1 added a comment.EditedAug 14 2023, 11:22 PM

Thanks for working on this, this is a real useful feature!

IIUR, for the long term, we will replace current intermediate result (execute build_symbolizer.sh with CMake add_custom_command) with pure CMake rules?

We have add_custom_libcxx in compiler-rt/cmake/Modules/AddCompilerRT.cmake, so I think we can add add_custom_llvm in compiler-rt/cmake/Modules/AddCompilerRT.cmake.
Then we can use add_custom_libcxx and add_custom_llvm in compiler-rt/lib/sanitizer_common/symbolizer/CMakeLists.txt to build LLVMSymbolize like we do in build_symbolizer.sh.

MTC added a subscriber: MTC.Aug 14 2023, 11:24 PM
vitalybuka added a comment.EditedAug 14 2023, 11:52 PM

Thanks for working on this, this is a real useful feature!

IIUR, for the long term, we will replace current intermediate result (execute build_symbolizer.sh with CMake add_custom_command) with pure CMake rules?

We have add_custom_libcxx in compiler-rt/cmake/Modules/AddCompilerRT.cmake, so I think we can add add_custom_llvm in compiler-rt/cmake/Modules/AddCompilerRT.cmake.
Then we can use add_custom_libcxx and add_custom_llvm in compiler-rt/lib/sanitizer_common/symbolizer/CMakeLists.txt to build LLVMSymbolize like we do in build_symbolizer.sh.

Yes. Actually I tried that in one patch and it's getting ugly. So I decide to lock parts which work and get back to the rest later.

kstoimenov accepted this revision.Aug 15 2023, 5:55 PM
This revision is now accepted and ready to land.Aug 15 2023, 5:55 PM

Can you describe how to specify COMPILER_RT_INTERNAL_SYMBOLIZER_ZLIB_SRC= in the summary/commit message?

This does not work with -Scompiler-rt:

% (cd /tmp/p; git clone https://github.com/madler/zlib.git)
% cmake -GNinja -Scompiler-rt -B/tmp/out/rt-x86_64 -DCMAKE_C_COMPILER=/tmp/Rel/bin/clang -DCMAKE_CXX_COMPILER=/tmp/Rel/bin/clang++ -DLLVM_CMAKE_DIR=/tmp/Rel -DLLVM_APPEND_VC_REV=OFF -DCMAKE_C{,XX}_COMPILER_TARGET=x86_64-unknown-linux-gnu -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=on -DCOMPILER_RT_DEFAULT_TARGET_ONLY=on -DCOMPILER_RT_HAS_LLD=on -DCOMPILER_RT_TEST_USE_LLD=on -DCOMPILER_RT_INCLUDE_TESTS=on -DCOMPILER_RT_ENABLE_INTERNAL_SYMBOLIZER=on -DCOMPILER_RT_INTERNAL_SYMBOLIZER_ZLIB_SRC=/tmp/p/zlib
% ninja -C /tmp/out/rt-x86_64
ninja: Entering directory `/tmp/out/rt-x86_64'
ninja: error: 'lib/sanitizer_common/symbolizer/clang', needed by 'lib/sanitizer_common/symbolizer/RTSanitizerCommonSymbolizerInternal.x86_64.o', missing and no known rule to make it

Perhaps you can mention that zstd is not used at the present.

The following works. Is RTSanitizerCommonSymbolizerInternalObj.x86_64 the intended final target name?

% cmake -GNinja -Sllvm -B/tmp/out/custom -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS='clang;lld;compiler-rt' -DCOMPILER_RT_ENABLE_INTERNAL_SYMBOLIZER=on -DCOMPILER_RT_INTERNAL_SYMBOLIZER_ZLIB_SRC=/tmp/p/zlib
% ninja -C /tmp/out/custom RTSanitizerCommonSymbolizerInternalObj.x86_64

This does not work with -Scompiler-rt:

% (cd /tmp/p; git clone https://github.com/madler/zlib.git)
% cmake -GNinja -Scompiler-rt -B/tmp/out/rt-x86_64 -DCMAKE_C_COMPILER=/tmp/Rel/bin/clang -DCMAKE_CXX_COMPILER=/tmp/Rel/bin/clang++ -DLLVM_CMAKE_DIR=/tmp/Rel -DLLVM_APPEND_VC_REV=OFF -DCMAKE_C{,XX}_COMPILER_TARGET=x86_64-unknown-linux-gnu -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=on -DCOMPILER_RT_DEFAULT_TARGET_ONLY=on -DCOMPILER_RT_HAS_LLD=on -DCOMPILER_RT_TEST_USE_LLD=on -DCOMPILER_RT_INCLUDE_TESTS=on -DCOMPILER_RT_ENABLE_INTERNAL_SYMBOLIZER=on -DCOMPILER_RT_INTERNAL_SYMBOLIZER_ZLIB_SRC=/tmp/p/zlib
% ninja -C /tmp/out/rt-x86_64
ninja: Entering directory `/tmp/out/rt-x86_64'
ninja: error: 'lib/sanitizer_common/symbolizer/clang', needed by 'lib/sanitizer_common/symbolizer/RTSanitizerCommonSymbolizerInternal.x86_64.o', missing and no known rule to make it

It does not suppose to work as we expect full llvm tree.

Perhaps you can mention that zstd is not used at the present.

The following works. Is RTSanitizerCommonSymbolizerInternalObj.x86_64 the intended final target name?

% cmake -GNinja -Sllvm -B/tmp/out/custom -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS='clang;lld;compiler-rt' -DCOMPILER_RT_ENABLE_INTERNAL_SYMBOLIZER=on -DCOMPILER_RT_INTERNAL_SYMBOLIZER_ZLIB_SRC=/tmp/p/zlib
% ninja -C /tmp/out/custom RTSanitizerCommonSymbolizerInternalObj.x86_64

remove zlib option

vitalybuka edited the summary of this revision. (Show Details)Aug 15 2023, 6:51 PM
MaskRay accepted this revision.Aug 15 2023, 7:09 PM

If -DCLANG_DEFAULT_RTLIB=compiler-rt -DCLANG_DEFAULT_UNWINDLIB=libunwind -DCLANG_DEFAULT_CXX_STDLIB=libc++ is used, unwind builtins should also be added as a depdency, otherwise we may have a linker error if unwind builtins haven't been built. I suspect that cxx cxxabi are similar.

compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
169

remove trailing space

vitalybuka edited the summary of this revision. (Show Details)Aug 15 2023, 7:21 PM
vitalybuka marked an inline comment as done.Aug 15 2023, 7:26 PM

If -DCLANG_DEFAULT_RTLIB=compiler-rt -DCLANG_DEFAULT_UNWINDLIB=libunwind -DCLANG_DEFAULT_CXX_STDLIB=libc++ is used, unwind builtins should also be added as a depdency, otherwise we may have a linker error if unwind builtins haven't been built. I suspect that cxx cxxabi are similar.

If -DCLANG_DEFAULT_RTLIB=compiler-rt -DCLANG_DEFAULT_UNWINDLIB=libunwind -DCLANG_DEFAULT_CXX_STDLIB=libc++ is used, unwind builtins should also be added as a dependency, otherwise we may have a linker error if unwind builtins haven't been built. I suspect that cxx cxxabi are similar.

Not sure I understand, RTSanitizerCommonSymbolizerInternal.${arch}.o should not depend on unwind.
Or your commend about top level cmake invocation in the description?

MaskRay accepted this revision.EditedAug 15 2023, 7:49 PM

If -DCLANG_DEFAULT_RTLIB=compiler-rt -DCLANG_DEFAULT_UNWINDLIB=libunwind -DCLANG_DEFAULT_CXX_STDLIB=libc++ is used, unwind builtins should also be added as a depdency, otherwise we may have a linker error if unwind builtins haven't been built. I suspect that cxx cxxabi are similar.

If -DCLANG_DEFAULT_RTLIB=compiler-rt -DCLANG_DEFAULT_UNWINDLIB=libunwind -DCLANG_DEFAULT_CXX_STDLIB=libc++ is used, unwind builtins should also be added as a dependency, otherwise we may have a linker error if unwind builtins haven't been built. I suspect that cxx cxxabi are similar.

Not sure I understand, RTSanitizerCommonSymbolizerInternal.${arch}.o should not depend on unwind.
Or your commend about top level cmake invocation in the description?

% cmake -GNinja -Sllvm -B/tmp/out/custom -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS='clang;lld;compiler-rt' -DLLVM_ENABLE_RUNTIMES='libcxx;libcxxabi;libunwind' -DCOMPILER_RT_ENABLE_INTERNAL_SYMBOLIZER=on -DCOMPILER_RT_INTERNAL_SYMBOLIZER_ZLIB_SRC=/tmp/p/zlib -DCLANG_DEFAULT_RTLIB=compiler-rt -DCLANG_DEFAULT_UNWINDLIB=libunwind -DCLANG_DEFAULT_CXX_STDLIB=libc++
% ninja -C /tmp/out/custom RTSanitizerCommonSymbolizerInternal.x86_64.o
...
-- Check for working C compiler: /tmp/out/custom/bin/clang - broken
CMake Error at /usr/share/cmake-3.25/Modules/CMakeTestCCompiler.cmake:70 (message):
  The C compiler

    "/tmp/out/custom/bin/clang"

  is not able to compile a simple test program.

  It fails with the following output:

    Change Dir: /tmp/out/custom/projects/compiler-rt/lib/sanitizer_common/symbolizer/RTSanitizerCommonSymbolizerInternal.x86_64/symbolizer/libcxx/CMakeFiles/CMakeScratch/TryCompile-CTptMA

    Run Build Command(s):/usr/bin/ninja cmTC_769ed && [1/2] Building C object CMakeFiles/cmTC_769ed.dir/testCCompiler.c.o
    [2/2] Linking C executable cmTC_769ed
    FAILED: cmTC_769ed
    : && /tmp/out/custom/bin/clang   CMakeFiles/cmTC_769ed.dir/testCCompiler.c.o -o cmTC_769ed   && :
    ld.lld: error: cannot open /tmp/out/custom/lib/clang/18/lib/linux/libclang_rt.builtins-x86_64.a: No such file or directory
    ld.lld: error: unable to find library -lunwind
    ld.lld: error: cannot open /tmp/out/custom/lib/clang/18/lib/linux/libclang_rt.builtins-x86_64.a: No such file or directory
    ld.lld: error: unable to find library -lunwind
    clang: error: linker command failed with exit code 1 (use -v to see invocation)
    ninja: build stopped: subcommand failed.

I think we get ld.lld: error: cannot open /tmp/out/custom/lib/clang/18/lib/linux/libclang_rt.builtins-x86_64.a: No such file or directory etc because builtins and unwind are not built yet.

compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
190

Q: How is the new RTSanitizerCommonSymbolizerInternal.${arch}.o injected into libclang_rt.*san*.a?

% cmake -GNinja -Sllvm -B/tmp/out/custom -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS='clang;lld;compiler-rt' -DLLVM_ENABLE_RUNTIMES='libcxx;libcxxabi;libunwind' -DCOMPILER_RT_ENABLE_INTERNAL_SYMBOLIZER=on -DCOMPILER_RT_INTERNAL_SYMBOLIZER_ZLIB_SRC=/tmp/p/zlib -DCLANG_DEFAULT_RTLIB=compiler-rt -DCLANG_DEFAULT_UNWINDLIB=libunwind -DCLANG_DEFAULT_CXX_STDLIB=libc++
% ninja -C /tmp/out/custom RTSanitizerCommonSymbolizerInternal.x86_64.o
...
-- Check for working C compiler: /tmp/out/custom/bin/clang - broken
CMake Error at /usr/share/cmake-3.25/Modules/CMakeTestCCompiler.cmake:70 (message):
  The C compiler

    "/tmp/out/custom/bin/clang"

  is not able to compile a simple test program.

  It fails with the following output:

    Change Dir: /tmp/out/custom/projects/compiler-rt/lib/sanitizer_common/symbolizer/RTSanitizerCommonSymbolizerInternal.x86_64/symbolizer/libcxx/CMakeFiles/CMakeScratch/TryCompile-CTptMA

    Run Build Command(s):/usr/bin/ninja cmTC_769ed && [1/2] Building C object CMakeFiles/cmTC_769ed.dir/testCCompiler.c.o
    [2/2] Linking C executable cmTC_769ed
    FAILED: cmTC_769ed
    : && /tmp/out/custom/bin/clang   CMakeFiles/cmTC_769ed.dir/testCCompiler.c.o -o cmTC_769ed   && :
    ld.lld: error: cannot open /tmp/out/custom/lib/clang/18/lib/linux/libclang_rt.builtins-x86_64.a: No such file or directory
    ld.lld: error: unable to find library -lunwind
    ld.lld: error: cannot open /tmp/out/custom/lib/clang/18/lib/linux/libclang_rt.builtins-x86_64.a: No such file or directory
    ld.lld: error: unable to find library -lunwind
    clang: error: linker command failed with exit code 1 (use -v to see invocation)
    ninja: build stopped: subcommand failed.
I see, I probably have it installed, so it does not fail on our bots and my workstation.
compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
190
vitalybuka added inline comments.Aug 15 2023, 11:27 PM
compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
190

Q: How is the new RTSanitizerCommonSymbolizerInternal.${arch}.o injected into libclang_rt.*san*.a?

D157921

If it's widely used, we could consider driver linking it.
So far it's quite niche, so probably not worth it.

This change broke our macOS builders (edb211cb78317ad73aa4bd2d3df75194b7f23a72 didn't help):

CMake Error at /opt/s/w/ir/x/w/llvm-llvm-project/compiler-rt/cmake/Modules/AddCompilerRT.cmake:357 (add_library):
  Error evaluating generator expression:

    $<TARGET_OBJECTS:RTSanitizerCommonSymbolizerInternal.osx>

  Objects of target "RTSanitizerCommonSymbolizerInternal.osx" referenced but
  no such target exists.
Call Stack (most recent call first):
  /opt/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/stats/CMakeLists.txt:23 (add_compiler_rt_runtime)


CMake Error at /opt/s/w/ir/x/w/llvm-llvm-project/compiler-rt/cmake/Modules/AddCompilerRT.cmake:357 (add_library):
  Error evaluating generator expression:

    $<TARGET_OBJECTS:RTSanitizerCommonSymbolizerInternal.osx>

  Objects of target "RTSanitizerCommonSymbolizerInternal.osx" referenced but
  no such target exists.
Call Stack (most recent call first):
  /opt/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/stats/CMakeLists.txt:23 (add_compiler_rt_runtime)


CMake Error at /opt/s/w/ir/x/w/llvm-llvm-project/compiler-rt/cmake/Modules/AddCompilerRT.cmake:357 (add_library):
  Error evaluating generator expression:

    $<TARGET_OBJECTS:RTSanitizerCommonSymbolizerInternal.osx>

  Objects of target "RTSanitizerCommonSymbolizerInternal.osx" referenced but
  no such target exists.
Call Stack (most recent call first):
  /opt/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/stats/CMakeLists.txt:23 (add_compiler_rt_runtime)


CMake Error at /opt/s/w/ir/x/w/llvm-llvm-project/compiler-rt/cmake/Modules/AddCompilerRT.cmake:357 (add_library):
  No SOURCES given to target: clang_rt.stats_osx_dynamic
Call Stack (most recent call first):
  /opt/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/stats/CMakeLists.txt:23 (add_compiler_rt_runtime)


CMake Generate step failed.  Build files cannot be regenerated correctly.

Would it be possible to take a look and revert if needed?

This broke the LLDB bot as well. I left a comment in https://github.com/llvm/llvm-project/commit/edb211cb78317ad73aa4bd2d3df75194b7f23a72. Given that this has been broken for a while I went ahead and reverted this: https://github.com/llvm/llvm-project/commit/0f50d0108c7ee8b081574a76816a428e30c6701a

This broke the LLDB bot as well. I left a comment in https://github.com/llvm/llvm-project/commit/edb211cb78317ad73aa4bd2d3df75194b7f23a72. Given that this has been broken for a while I went ahead and reverted this: https://github.com/llvm/llvm-project/commit/0f50d0108c7ee8b081574a76816a428e30c6701a

Thanks for revert.

Do we have build bot so I can watch on the next attempt?

vitalybuka reopened this revision.Sep 8 2023, 10:55 AM
This revision is now accepted and ready to land.Sep 8 2023, 10:55 AM

This broke the LLDB bot as well. I left a comment in https://github.com/llvm/llvm-project/commit/edb211cb78317ad73aa4bd2d3df75194b7f23a72. Given that this has been broken for a while I went ahead and reverted this: https://github.com/llvm/llvm-project/commit/0f50d0108c7ee8b081574a76816a428e30c6701a

Thanks for revert.

Do we have build bot so I can watch on the next attempt?

Thanks, I see a couple of URLs in https://github.com/llvm/llvm-project/commit/edb211cb78317ad73aa4bd2d3df75194b7f23a72

vitalybuka added a comment.EditedSep 8 2023, 4:50 PM

I guess it was the followup patch edb211cb78317ad73aa4bd2d3df75194b7f23a72. It's re-landed (with fix) and passed both bots.

I will try to reland this one. Don't hesitate to revert if it break stuff.