Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build SmartExceptionTracer with cmake #2397

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

huww98
Copy link

@huww98 huww98 commented Mar 9, 2025

When using FOLLY_NO_EXCEPTION_TRACER, we may still want to use GtestHelpers.h, which references SmartExceptionTracer.h. For this to work, we can manually link the folly_exception_tracer target like:

target_link_libraries(main PRIVATE $<LINK_LIBRARY:WHOLE_ARCHIVE,Folly::folly_exception_tracer>)

Without WHOLE_ARCHIVE, the global initialize will not be pulled in by linker.

Manually tested this with a simple project: folly is configured with -DGFLAGS_USE_TARGET_NAMESPACE=ON -DFOLLY_NO_EXCEPTION_TRACER=ON
main.cpp:

#include <exception>
#include <iostream>
#include <folly/debugging/exception_tracer/SmartExceptionTracer.h>

void f() {
    throw std::exception();
}

int main() {
    try {
        f();
    } catch(const std::exception& ex) {
        std::cerr << folly::exception_tracer::getTrace(ex);
    }
}

CMakeLists.txt

cmake_minimum_required(VERSION 3.24)
project(folly-try)

set(CMAKE_CXX_STANDARD 17)

set(GFLAGS_USE_TARGET_NAMESPACE ON)
find_package(gflags REQUIRED)
find_package(folly REQUIRED)

add_executable(main main.cpp)
target_link_libraries(main PRIVATE $<LINK_LIBRARY:WHOLE_ARCHIVE,Folly::folly_exception_tracer>)
target_link_libraries(main PRIVATE Folly::folly)

Ordering folly_exception_tracer before folly is important. (Maybe flaw of cmake?)

# ./main 
Exception type: std::exception (7 frames)
    @ 0000000000014745 _ZN5folly16exception_tracer12_GLOBAL__N_113throwCallbackEPvPSt9type_infoPPFvS2_E
                       /root/folly/folly/debugging/exception_tracer/SmartExceptionStackTraceHooks.cpp:64
    @ 000000000000ed51 __cxa_throw
                       /root/folly/folly/debugging/exception_tracer/ExceptionTracerLib.cpp:73
    @ 000000000000dc36 _Z1fv
                       /root/folly_try/main.cpp:6
    @ 000000000000dc57 main
                       /root/folly_try/main.cpp:11
    @ 0000000000029d8f (unknown)
    @ 0000000000029e3f __libc_start_main
    @ 000000000000db34 _start

Without linking to folly_exception_tracer: link error

[ 50%] Linking CXX executable main
/usr/bin/ld: CMakeFiles/main.dir/main.cpp.o: in function `main':
/root/folly_try/main.cpp:13: undefined reference to `folly::exception_tracer::getTrace(std::exception const&)'
/usr/bin/ld: /root/folly_try/main.cpp:13: undefined reference to `folly::exception_tracer::operator<<(std::ostream&, folly::exception_tracer::ExceptionInfo const&)'
collect2: error: ld returned 1 exit status
make[2]: *** [CMakeFiles/main.dir/build.make:122: main] Error 1
make[1]: *** [CMakeFiles/Makefile2:87: CMakeFiles/main.dir/all] Error 2
make: *** [Makefile:91: all] Error 2

link to folly_exception_tracer without WHOLE_ARCHIVE

# ./main 
WARNING: Logging before InitGoogleLogging() is written to STDERR
W0309 16:01:49.557803 113195 SmartExceptionTracer.cpp:48] Smart exception tracer library not linked, stack traces not available
Exception type: std::exception

So even without FOLLY_NO_EXCEPTION_TRACER, we will need this, because we definitely don't want to specify WHOLE_ARCHIVE for the main folly library.

BTW, ExceptionAbi.h is removed from install. It is copied from libstdc++, and folly should not expose this.

The build script of exception_tracer have changed many times.

  1. separated from main lib in da420c9
  2. merged back in include experimental/exception_tracer in library #1524
  3. an option FOLLY_NO_EXCEPTION_TRACER is included to separate it again in 41a25b7

IMO, we should merge the folly_exception_tracer_base to the main lib, so that GtestHelper works out-of-box. It does not have any side-effect if not used. And keep folly_exception_tracer, folly_exception_counter separate lib, because they rely on global initialization and needs specifying WHOLE_ARCHIVE when linking. What do you think?

When using FOLLY_NO_EXCEPTION_TRACER, we may still want to use GtestHelpers.h, which references SmartExceptionTracer.h. For this to work, we can manually link the `folly_exception_tracer` target like:

    target_link_libraries(main PRIVATE $<LINK_LIBRARY:WHOLE_ARCHIVE,Folly::folly_exception_tracer>)

Without WHOLE_ARCHIVE, the global `initialize` will not be pulled in by linker.
@facebook-github-bot
Copy link
Contributor

Hi @huww98!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants