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

No abi::__forced_unwind in libc++abi #549

Closed
coffee-lord opened this issue Nov 8, 2017 · 5 comments
Closed

No abi::__forced_unwind in libc++abi #549

coffee-lord opened this issue Nov 8, 2017 · 5 comments

Comments

@coffee-lord
Copy link

coffee-lord commented Nov 8, 2017

Commit 039b34e added a hard dependency on abi::__forced_unwind in common.h on __linux__ platform. This causes a compilation error when compiled with libc++abi:

.../spdlog/details/logger_impl.h:80:5: error: no type named '__forced_unwind' in namespace '__cxxabiv1'
    SPDLOG_CATCH_ALL
    ^~~~~~~~~~~~~~~~
.../spdlog/common.h:47:38: note: expanded from macro 'SPDLOG_CATCH_ALL'
#define SPDLOG_CATCH_ALL catch (abi::__forced_unwind&) { _err_handler("Unknown exception"); throw; } catch (...)

Relevant: cxxabi.h from libc++abi

Tested OSs: Archlinux, Gentoo (~amd64), Debian (sid)
Compiler: LLVM (trunk 317567)

  • libc++
  • libc++abi
  • Clang 6.0.0
@gabime
Copy link
Owner

gabime commented Nov 8, 2017

Please provide the exact linux and gcc versions.

@coffee-lord
Copy link
Author

GCC's cxxabi.h includes cxxabi_forced.h which defines _CXXABI_FORCED_H. A weak but valid workaround to make it compile with LLVM's libc++abi would be to check for it.

#if defined(__linux__) && !defined(__ANDROID__)
#include <cxxabi.h>
#ifdef _CXXABI_FORCED_H
#define SPDLOG_CATCH_ALL catch (abi::__forced_unwind&) { _err_handler("Unknown exception"); throw; } catch (...)
#else // _CXXABI_FORCED_H
#define SPDLOG_CATCH_ALL catch (...)
#endif // _CXXABI_FORCED_H
#else // __linux__
#define SPDLOG_CATCH_ALL catch (...)
#endif // __linux__

Is abi::__forced_unwind a portable API or is it a GCC extension? LLVM might have something similar.

@manuel-schiller

@gabime
Copy link
Owner

gabime commented Nov 10, 2017

The more I think about it the more I believe this catch(..) is bad idea. If an exception other than ‘std::exception’ happened, than it is really an unexpected one that cannot be treated correctly by spdlog. anyway, and certainly should not be masked.
So I think that catching std::exception is enough.

@coffee-lord
Copy link
Author

It seems that abi::__forced_unwind is GCC-specific. Commit author, @manuel-schiller wanted to handle pthread_cancel() and AFAIK this kind of thread cancellation is not portable. GCC throws this exception when a thread is being cancelled but I couldn't find anything similar in LLVM. This article explains the problem.

I agree that this is probably out of scope of a logging library. Just catching std::exception should be enough unless we're missing something.

@gabime
Copy link
Owner

gabime commented Nov 12, 2017

Fixed (kind of) in 8ca1d84

Now spdlog won't try to catch(...) - only catch(std::exception&)

@gabime gabime closed this as completed Nov 12, 2017
kdesysadmin pushed a commit to KDE/akonadi that referenced this issue May 9, 2019
Summary:
abi::__forced_unwind is gcc specific, apparently.
gabime/spdlog#549

Test Plan: Builds

Reviewers: dvratil

Reviewed By: dvratil

Subscribers: kde-pim

Tags: #kde_pim

Differential Revision: https://phabricator.kde.org/D21077
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

2 participants