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

Throwing exceptions in methods declared __attribute__((pure)) leads to abort on (at least) FreeBSD #41

Closed
jonsiddle opened this issue May 7, 2017 · 6 comments

Comments

@jonsiddle
Copy link

jonsiddle commented May 7, 2017

MPD was recently changed to use C++ exceptions and I see aborts on FreeBSD. The simplest way to reproduce is to point at a music library directory with a single directory "foo" under which there's a single file "foo.mp3". Once MPD is running, move "foo" to "bar" and trigger an update. It will crash with an abort.

I tracked this down to the combination of __attribute__((pure)) and the clang compiler which FreeBSD uses. I guess it's optimising away the exception handling. gcc seems to not make the same optimisation.

There may be other cases. I've just worked around by redefining gcc_pure (which MPD defines to apply the attribute) to no-op.

@ephemeralriggs
Copy link
Contributor

ephemeralriggs commented May 7, 2017

Wow, this is interesting! You mean like setting
#define gcc_pure
in Compiler.h?
Does this happen to solve the library update problem mentioned in https://forum.musicpd.org/viewtopic.php?f=7&t=4318#p6880 and https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219048 ?
Will test this ...

@ephemeralriggs
Copy link
Contributor

Seems indeed to work!
Thanks for tracking down this one!

@jonsiddle
Copy link
Author

Yes, that's exactly what I did. I didn't bother with a patch since it was such a simple change and I figured someone who knew the code might want to do it properly and keep the optimisation where exceptions weren't used or make it based on compiler. I'd be happy to have a go at a better fix if that would be helpful.

@MaxKellermann
Copy link
Member

Huh, interesting find. The GCC documentation of pure leaves this part undefined - it does not mention whether "pure" functions are allowed to throw exceptions (which I interpreted as just another kind of "return value"), while clang appears to assume that a pure function is also noexcept.

MaxKellermann added a commit that referenced this issue May 8, 2017
The "pure" and "const" attributes are not so well-defined, and a
recent clang version implements an optimization which pushes the
definition's boundary beyond what I believed it was.  clang now
assumes that functions declared "pure" cannot throw exceptions, even
if they lack the "noexcept" specification.

When compiled with this new clang version, MPD will crash randomly if
an exception happens to get thrown by such as "pure" function
(#41).

This commit removes all such misplaced "pure" and "const" attributes,
closing #41.
@MaxKellermann
Copy link
Member

I quickly wrote a clang plugin which shows me code locations that need to be fixed: https://gist.github.com/MaxKellermann/758ed94d66cc744fe163bee0f854352d

@jonsiddle
Copy link
Author

Nice way to find them. I've applied the commit and can confirm it fixes the issue I was seeing.

bob-beck pushed a commit to openbsd/ports that referenced this issue May 30, 2017
Fixes a database update issue reported by ajacoutot@, fix found via
FreeBSD by Andre Smagin (cf MusicPlayerDaemon/MPD#41)

ok dcoppa@ (maintainer) ajacoutot@
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

3 participants