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

Statically linking it with libstdc ++ and folly broken because of experimental/exception_tracer #1623

Open
neogenie opened this issue Jul 28, 2021 · 7 comments

Comments

@neogenie
Copy link

In commit: 8140959#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20a

Permits including folly/experimental/exception_tracer/ in the cmake build.

Closes: #1524.

experimental/exception_tracer library became a part of main CMakeList.txt file and now a part of main folly.a library. But...

This make broken logic described in library annotation:

Exception tracer library

This library allows you to inspect the exception stack at runtime. The library can be used in three ways:

Link in the exception_tracer_base library. You get access to the functions in ExceptionTracer.h, but no stack traces. This has no runtime overhead, and is compliant with the C++ ABI.

Link in the (full) exception_tracer library. You get access to the functions in ExceptionTracer.h, the std::terminate and std::unexpected handlers are installed by default, and you get full stack traces with all exceptions. This has some runtime overhead (the stack trace must be captured and stored whenever an exception is thrown) added to throw and catch, but no runtime overhead otherwise. This is less portable (depends on internal details of GNU's libstdc++).

LD_PRELOAD libexceptiontracer.so. This is equivalent to #2 above, but requires no link-time changes. On the other hand, you need to ensure that libexceptiontracer.so is compiled with the same compiler and flags as your binary, and the usual caveats about LD_PRELOAD apply (it propagates to child processes, etc)

In principle, it is incorrect because the file ExceptionTracerLib.cpp ignores FOLLY_HAVE_ELF & FOLLY_HAVE_DWARF definitions, although relevant libraries may not be available on the build platform.

And most dramatically, it made it impossible to build an application by statically linking it with libstdc ++ and folly.

[100%] Linking CXX executable 
/usr/lib/gcc/x86_64-linux-gnu/11/libstdc++.a(eh_catch.o): In function `__cxa_begin_catch':
(.text.__cxa_begin_catch+0x0): multiple definition of `__cxa_begin_catch'
/usr/local/lib/libfolly.a(ExceptionTracerLib.cpp.o):/tmp/build/folly/folly/experimental/exception_tracer/ExceptionTracerLib.cpp:123: first defined here

It would be nice to rollback these changes or still make it cleaner by providing the necessary compilation options.

@kvtb
Copy link

kvtb commented Oct 16, 2021

It won't work with static linking :(
because of dlsym to get original functions:

static auto orig_cxa_rethrow = reinterpret_cast<decltype(&__cxa_rethrow)>(
dlsym(RTLD_NEXT, "__cxa_rethrow"));

@kvtb
Copy link

kvtb commented Oct 16, 2021

Something like https://stackoverflow.com/a/3053921/15239223 should be used to handle static linking

kvtb added a commit to kvtb/folly that referenced this issue Oct 16, 2021
kvtb added a commit to kvtb/folly that referenced this issue Oct 17, 2021
@mhx
Copy link
Contributor

mhx commented Nov 21, 2021

I haven't seen a rationale/context in #1524 for why exactly exception_tracer should be part of folly. Why was it a problem to have it as a separate library?

I also just found out that static builds for one of my projects are broken because of this.

@yfeldblum, is it really intentional to break static builds linking against folly?

@mhx
Copy link
Contributor

mhx commented Mar 18, 2022

ping

1 similar comment
@draveness
Copy link

ping

facebook-github-bot pushed a commit that referenced this issue Apr 21, 2022
Summary:
When Exception Tracer was added to Folly, it broke statically linking with libstdc++ (see #1623 for more details). While this isn't a common requirement, it's needed in order to have OpenCV and Airstore link correctly for the Avator Research Supercluster project.

This change just allows one to disable building the exception tracer code. Other solutions (wrapper/real functions or alternative linker arguments) would possibly be better, but would increase the scope & risk of this change without offering us any benefit.

Reviewed By: yfeldblum

Differential Revision: D35791075

fbshipit-source-id: db845e035b453e55e61b9b969e7216e041ed4766
@fzhedu
Copy link

fzhedu commented Nov 18, 2022

change the way to overwrite __cxa_throw:

extern "C" {
void __real___cxa_throw(...);
void __wrap___cxa_throw(...) {
    // print track trace
    __real___cxa_throw(...);
}
}

linking: -WL, -wrap=__cxa_throw

folly/folly/experimental/exception_tracer/ExceptionTracerLib.cpp

@qc00
Copy link

qc00 commented Mar 3, 2023

FYI, folly's CMake now has an option to disable exception_tracer

misiugodfrey pushed a commit to heavyai/heavydb that referenced this issue Aug 15, 2023
…246)

* deps: bump tbb to 2021.8.0 and fix tsan config

* deps: bump ninja to 1.11.1

* deps: bump folly to 2023.02.20.00 and fmtlib to 9.1.0

Disable folly exception tracer to fix static linking.
See facebook/folly#1623

* deps: bump centos openssl to 1.1.1m

* deps: bump blosc to 1.21.2

Signed-off-by: Misiu Godfrey <[email protected]>
misiugodfrey pushed a commit to heavyai/heavydb that referenced this issue Aug 21, 2023
…246)

* deps: bump tbb to 2021.8.0 and fix tsan config

* deps: bump ninja to 1.11.1

* deps: bump folly to 2023.02.20.00 and fmtlib to 9.1.0

Disable folly exception tracer to fix static linking.
See facebook/folly#1623

* deps: bump centos openssl to 1.1.1m

* deps: bump blosc to 1.21.2

Signed-off-by: Misiu Godfrey <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants