fix: Remove incorrect compile flags #455
Conversation
Taepper
force-pushed
the
fix-compile-flags
branch
from
982deed to
da38617
Compare
Taepper
force-pushed
the
fix-compile-flags
branch
from
da38617 to
f016b63
Compare
|
I think this warrants a description on how to reproduce the error (best in the body of the commit message). It's probably worth including the minimal test case that you made, too, or alternatively give the diff of the change that you applied to silo to make the issue show up reliably (I'd also include that in the commit message, indented by 4 spaces). |
Taepper
force-pushed
the
fix-compile-flags
branch
from
f016b63 to
66f3cb0
Compare
pflanze
force-pushed
the
fix-compile-flags
branch
from
66f3cb0 to
d462d64
Compare
|
I have added some more information to the commit message, from my notes. Merge if you're happy with that. |
|
Ah, yes that looks very nice and is certainly a better location than just having it in the PR body. I will change
to |
Those led to spurious aborts on insert, when exceptions were thrown in
a callback run in a worker thread via tbb. To reproduce easily, apply
the following change:
diff --git a/src/silo/preprocessing/preprocessor.cpp b/src/silo/preprocessing/preprocessor.cpp
index 92642ca9..7fe6996d 100644
--- a/src/silo/preprocessing/preprocessor.cpp
+++ b/src/silo/preprocessing/preprocessor.cpp
@@ -62,6 +62,9 @@ Preprocessor::Preprocessor(
}
Database Preprocessor::preprocess() {
+ tbb::parallel_for(tbb::blocked_range(0, 1), [](const auto i){
+ throw std::runtime_error("");
+ });
SPDLOG_INFO(
"preprocessing - creating intermediate results directory '{}'",
preprocessing_config.getIntermediateResultsDirectory().string()
then build for linux with --release and run the preprocessing.
This will be the relevant information of the `gcc` manpage:
There are several situations in which an application should
use the shared libgcc instead of the static version. The
most common of these is when the application wishes to throw
and catch exceptions across different shared libraries. In
that case, each of the libraries as well as the application
itself should use the shared libgcc.
We violated that, due to tbb being linked dynamically, which led to
libgcc also being linked dynamically, while we already linked libgcc
statically to the other code.
Taepper
force-pushed
the
fix-compile-flags
branch
from
d462d64 to
8a045d4
Compare
|
Ah, I had forgotten that you had some info in the PR already when I edited the commit message. But except for the
Actually it doesn't apparently, since the size of the .o files stayed the same when I looked. But that was a side concern only anyway. (I forgot to add a |
resolves #448
Summary
Remove incorrect flags:
set(CMAKE_CXX_FLAGS_RELEASE "[...] -static-libstdc++ -static-libgcc")These should instead be declared as
target_link_options(siloApi PUBLIC -static-libstdc++ -static-libgcc)because
CMAKE_CXX_FLAGS..are used for the compilation of every unit (.cpp) file. This links standard library code into the output multiple times.But having that compilation flag is also not desirable, as the tbb's shard object (.so) file is itself linked against shared libgcc and libstdc++ as verified by
And using different standard libraries (statically linked in our code, shared in tbb dependency) leads to problems as pointed out by
man gcc.. and as observed in #448
PR Checklist
- [ ] All necessary documentation has been adapted or there is an issue to do so.- [ ] The implemented feature is covered by an appropriate test.