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

Fix runtime when build with -fsanitize=cfi #1972

Merged
merged 1 commit into from
Nov 25, 2021
Merged

Fix runtime when build with -fsanitize=cfi #1972

merged 1 commit into from
Nov 25, 2021

Conversation

bansan85
Copy link
Contributor

-flto=thin -fvisibility=hidden -fno-sanitize-trap=cfi

See https://gcc.gnu.org/wiki/Visibility

@bansan85
Copy link
Contributor Author

bansan85 commented Jun 16, 2021

I tried to build and run a program with -fsanitize=cfi (and -flto=thin -fvisibility=hidden -fno-sanitize-trap=cfi) according to the clang documentation.

So with this option, all class are by default hidden.

According to GCC documentation, the C++ visibility support should be __attribute__ ((visibility ("default"))) for SPDLOG_API with Linux.

@bansan85
Copy link
Contributor Author

bansan85 commented Jun 20, 2021

I now have a doubt. Maybe the __attribute__ ((visibility ("default"))) should not be applied in header_only mode / static lib.

@gabime
Copy link
Owner

gabime commented Jun 20, 2021

I now have a doubt. Maybe the attribute ((visibility ("default"))) should not be applied in header_only mode / static lib.

Yes, need to be careful here not to break backward compatibility.

Here is the closest I could find (I didn't test it. Please make sure it indeed works):

#ifdef SPDLOG_COMPILED_LIB
#   if defined(SPDLOG_SHARED_LIB) && defined(_WIN32)
#       ifdef spdlog_EXPORTS
#           define SPDLOG_API __declspec(dllexport)
#       else
#           define SPDLOG_API __declspec(dllimport)
#       endif
#   elif defined(SPDLOG_SHARED_LIB) && (defined(__GNUC__) || defined(__clang__))
#       define SPDLOG_API __attribute__((visibility("default")))
#   else // if not SPDLOG_SHARED_LIB or other compiler
#       define SPDLOG_API
#   endif
#   define SPDLOG_INLINE
#   undef SPDLOG_HEADER_ONLY
#else // !defined(SPDLOG_COMPILED_LIB)
#   define SPDLOG_API
#   define SPDLOG_HEADER_ONLY
#   define SPDLOG_INLINE inline
#endif // #ifdef SPDLOG_COMPILED_LIB

@gabime
Copy link
Owner

gabime commented Jul 4, 2021

@bansan85 any news on this pr?

@bansan85
Copy link
Contributor Author

bansan85 commented Jul 4, 2021

Sorry. Thanks for reminding me.

I have to take some time to do it. Give me 14 days so I can make so good tests.

@gabime
Copy link
Owner

gabime commented Aug 22, 2021

@bansan85 Any news?

@bansan85
Copy link
Contributor Author

I'm sorry 😕 I think the code should work but at the current time, I don't have the motivation to write some tests 😩

@bansan85
Copy link
Contributor Author

I finally took some time to test.

Run the command:

/mnt/c/temp/spdlog-1.9.2 # rm -Rf build; cmake -S . -B build -DCMAKE_CXX_COMPILER=clang++ -DSPDLOG_BUILD_SHARED=yes -DCMAKE_CXX_FLAGS="-O3 -flto=thin -fvisibility=hidden -fno-sanitize-trap=cfi"; VERBOSE=1 cmake --build build/ --target all --config Release --parallel

@gabime gabime merged commit c6d144d into gabime:v1.x Nov 25, 2021
@gabime
Copy link
Owner

gabime commented Nov 25, 2021

Thanks @bansan85

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

Successfully merging this pull request may close these issues.

2 participants