From e02c1adf795c496a38946488c2c983908fe18bd2 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 29 Oct 2018 11:05:27 +0100 Subject: [PATCH 1/8] increment version number to 0.20.23 --- NEWS | 2 ++ configure.ac | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index b331b18fb..77d4b48af 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,5 @@ +ver 0.20.23 (not yet released) + ver 0.20.22 (2018/10/23) * protocol - add tag fallbacks for AlbumArtistSort, ArtistSort diff --git a/configure.ac b/configure.ac index 687823dc4..2c75c3239 100644 --- a/configure.ac +++ b/configure.ac @@ -1,10 +1,10 @@ AC_PREREQ(2.60) -AC_INIT(mpd, 0.20.22, musicpd-dev-team@lists.sourceforge.net) +AC_INIT(mpd, 0.20.23, musicpd-dev-team@lists.sourceforge.net) VERSION_MAJOR=0 VERSION_MINOR=20 -VERSION_REVISION=22 +VERSION_REVISION=23 VERSION_EXTRA=0 AC_CONFIG_SRCDIR([src/Main.cxx]) From 1e6c445320a42acd0169e8546411c4b58b212b8e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 29 Oct 2018 11:00:20 +0100 Subject: [PATCH 2/8] configure.ac: add `-funwind-tables` to work around clang bug Replaces the workaround from commit 751fff07fb28720156d0d1dc833a2b6534959a0d which fixed only one of many crash locations. See: https://github.com/MusicPlayerDaemon/MPD/issues/373 https://github.com/android-ndk/ndk/issues/831 https://bugs.llvm.org/show_bug.cgi?id=32611 --- configure.ac | 5 +++++ src/input/Error.hxx | 8 -------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/configure.ac b/configure.ac index 2c75c3239..6ea941d7e 100644 --- a/configure.ac +++ b/configure.ac @@ -1354,6 +1354,11 @@ AX_APPEND_COMPILE_FLAGS([-fno-threadsafe-statics]) AX_APPEND_COMPILE_FLAGS([-fmerge-all-constants]) AX_APPEND_COMPILE_FLAGS([-ffast-math]) AX_APPEND_COMPILE_FLAGS([-ftree-vectorize]) + +dnl Workaround for clang bug +dnl https://bugs.llvm.org/show_bug.cgi?id=32611 +AX_APPEND_COMPILE_FLAGS([-funwind-tables]) + AC_LANG_POP dnl ---------------------------------- debug ---------------------------------- diff --git a/src/input/Error.hxx b/src/input/Error.hxx index 0811abc68..b52b9d06b 100644 --- a/src/input/Error.hxx +++ b/src/input/Error.hxx @@ -30,15 +30,7 @@ * exist? This function attempts to recognize exceptions thrown by * various input plugins. */ -#ifndef __clang__ -/* the "pure" attribute must be disabled because it triggers a clang - bug, wrongfully leading to std::terminate() even though the - function catches all exceptions thrown by std::rethrow_exception(); - this can be reproduced with clang 7 from Android NDK r18b and on - clang 6 on FreeBSD - (https://github.com/MusicPlayerDaemon/MPD/issues/373) */ gcc_pure -#endif bool IsFileNotFound(std::exception_ptr e); From f76544be4c0e7539e326d1f598c183970404bea7 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 29 Oct 2018 11:01:15 +0100 Subject: [PATCH 3/8] db/update: catch all exceptions --- src/db/update/Archive.cxx | 4 ++-- src/db/update/Container.cxx | 4 ++-- src/db/update/ExcludeList.cxx | 2 +- src/db/update/InotifyUpdate.cxx | 16 ++++++++-------- src/db/update/UpdateIO.cxx | 12 ++++++------ src/db/update/Walk.cxx | 4 ++-- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/db/update/Archive.cxx b/src/db/update/Archive.cxx index 6004ed835..88950e3c3 100644 --- a/src/db/update/Archive.cxx +++ b/src/db/update/Archive.cxx @@ -153,8 +153,8 @@ UpdateWalk::UpdateArchiveFile(Directory &parent, const char *name, ArchiveFile *file; try { file = archive_file_open(&plugin, path_fs); - } catch (const std::runtime_error &e) { - LogError(e); + } catch (...) { + LogError(std::current_exception()); if (directory != nullptr) editor.LockDeleteDirectory(directory); return; diff --git a/src/db/update/Container.cxx b/src/db/update/Container.cxx index fc4ec416c..6067145e4 100644 --- a/src/db/update/Container.cxx +++ b/src/db/update/Container.cxx @@ -119,9 +119,9 @@ UpdateWalk::UpdateContainerFile(Directory &directory, modified = true; } - } catch (const std::runtime_error &e) { + } catch (...) { editor.LockDeleteDirectory(contdir); - LogError(e); + LogError(std::current_exception()); return false; } diff --git a/src/db/update/ExcludeList.cxx b/src/db/update/ExcludeList.cxx index 332e4bbc7..fadc8817a 100644 --- a/src/db/update/ExcludeList.cxx +++ b/src/db/update/ExcludeList.cxx @@ -78,7 +78,7 @@ ExcludeList::Check(Path name_fs) const noexcept try { if (i.Check(NarrowPath(name_fs).c_str())) return true; - } catch (const std::runtime_error &) { + } catch (...) { } } #else diff --git a/src/db/update/InotifyUpdate.cxx b/src/db/update/InotifyUpdate.cxx index 562315fa3..bd82f3e7a 100644 --- a/src/db/update/InotifyUpdate.cxx +++ b/src/db/update/InotifyUpdate.cxx @@ -187,8 +187,8 @@ recursive_watch_subdirectories(WatchDirectory *directory, FileInfo fi; try { fi = FileInfo(child_path_fs); - } catch (const std::runtime_error &e) { - LogError(e); + } catch (...) { + LogError(std::current_exception()); continue; } @@ -198,8 +198,8 @@ recursive_watch_subdirectories(WatchDirectory *directory, try { ret = inotify_source->Add(child_path_fs.c_str(), IN_MASK); - } catch (const std::runtime_error &e) { - FormatError(e, + } catch (...) { + FormatError(std::current_exception(), "Failed to register %s", child_path_fs.c_str()); continue; @@ -302,8 +302,8 @@ mpd_inotify_init(EventLoop &loop, Storage &storage, UpdateService &update, inotify_source = new InotifySource(loop, mpd_inotify_callback, nullptr); - } catch (const std::runtime_error &e) { - LogError(e); + } catch (...) { + LogError(std::current_exception()); return; } @@ -312,8 +312,8 @@ mpd_inotify_init(EventLoop &loop, Storage &storage, UpdateService &update, int descriptor; try { descriptor = inotify_source->Add(path.c_str(), IN_MASK); - } catch (const std::runtime_error &e) { - LogError(e); + } catch (...) { + LogError(std::current_exception()); delete inotify_source; inotify_source = nullptr; return; diff --git a/src/db/update/UpdateIO.cxx b/src/db/update/UpdateIO.cxx index 2623b0dc4..6977204de 100644 --- a/src/db/update/UpdateIO.cxx +++ b/src/db/update/UpdateIO.cxx @@ -36,8 +36,8 @@ GetInfo(Storage &storage, const char *uri_utf8, StorageFileInfo &info) noexcept try { info = storage.GetInfo(uri_utf8, true); return true; -} catch (const std::runtime_error &e) { - LogError(e); +} catch (...) { + LogError(std::current_exception()); return false; } @@ -46,8 +46,8 @@ GetInfo(StorageDirectoryReader &reader, StorageFileInfo &info) noexcept try { info = reader.GetInfo(true); return true; -} catch (const std::runtime_error &e) { - LogError(e); +} catch (...) { + LogError(std::current_exception()); return false; } @@ -58,7 +58,7 @@ DirectoryExists(Storage &storage, const Directory &directory) noexcept try { info = storage.GetInfo(directory.GetPath(), true); - } catch (const std::runtime_error &) { + } catch (...) { return false; } @@ -83,7 +83,7 @@ directory_child_is_regular(Storage &storage, const Directory &directory, try { return GetDirectoryChildInfo(storage, directory, name_utf8) .IsRegular(); -} catch (const std::runtime_error &) { +} catch (...) { return false; } diff --git a/src/db/update/Walk.cxx b/src/db/update/Walk.cxx index c71b3d73c..da13f0d5d 100644 --- a/src/db/update/Walk.cxx +++ b/src/db/update/Walk.cxx @@ -244,8 +244,8 @@ try { FormatDebug(update_domain, "%s is not a directory, archive or music", name); } -} catch (const std::exception &e) { - LogError(e); +} catch (...) { + LogError(std::current_exception()); } /* we don't look at "." / ".." nor files with newlines in their name */ From a75d2fdd5abec3eccb8c8900596e254bf38c26a2 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 29 Oct 2018 12:01:28 +0100 Subject: [PATCH 4/8] NEWS: mention the new clang crash bug workaround --- NEWS | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS b/NEWS index 77d4b48af..db8cdb497 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,5 @@ ver 0.20.23 (not yet released) +* new clang crash bug workaround ver 0.20.22 (2018/10/23) * protocol From 7aa1dceef6032b6772bbebf9e8d766acaadf569d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 29 Oct 2018 12:01:17 +0100 Subject: [PATCH 5/8] player/Control: move IDLE_PLAYER to Player::SeekDecoder() This emits the event even if PlayerControl::Play() is used to replay the current song, which emits the missing "player" idle event. Closes #381 --- NEWS | 2 ++ src/player/Control.cxx | 8 ++------ src/player/Thread.cxx | 2 ++ 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index db8cdb497..ffb1a2ef2 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.20.23 (not yet released) +* protocol + - emit "player" idle event when restarting the current song * new clang crash bug workaround ver 0.20.22 (2018/10/23) diff --git a/src/player/Control.cxx b/src/player/Control.cxx index 013b57bf5..0c18a09fa 100644 --- a/src/player/Control.cxx +++ b/src/player/Control.cxx @@ -246,12 +246,8 @@ PlayerControl::LockSeek(DetachedSong *song, SongTime t) { assert(song != nullptr); - { - const std::lock_guard protect(mutex); - SeekLocked(song, t); - } - - idle_add(IDLE_PLAYER); + const std::lock_guard protect(mutex); + SeekLocked(song, t); } void diff --git a/src/player/Thread.cxx b/src/player/Thread.cxx index fcb7b39da..001c96fa1 100644 --- a/src/player/Thread.cxx +++ b/src/player/Thread.cxx @@ -580,6 +580,8 @@ Player::SeekDecoder() { assert(pc.next_song != nullptr); + idle_add(IDLE_PLAYER); + pc.outputs.Cancel(); const SongTime start_time = pc.next_song->GetStartTime(); From b0a6a569dff5aee5cadee57aea52e5ec9a03aa11 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 29 Oct 2018 22:38:32 +0100 Subject: [PATCH 6/8] pcm/FloatConvert: add `static_assert` on the factor This assertion currently fails for S32 due to integer overflow (#380). --- src/pcm/FloatConvert.hxx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/pcm/FloatConvert.hxx b/src/pcm/FloatConvert.hxx index 8022fd378..5a1756b36 100644 --- a/src/pcm/FloatConvert.hxx +++ b/src/pcm/FloatConvert.hxx @@ -35,6 +35,7 @@ struct FloatToIntegerSampleConvert { typedef typename DstTraits::value_type DV; static constexpr SV factor = 1 << (DstTraits::BITS - 1); + static_assert(factor > 0, "Wrong factor"); gcc_const static DV Convert(SV src) noexcept { @@ -54,6 +55,7 @@ struct IntegerToFloatSampleConvert { typedef typename DstTraits::value_type DV; static constexpr DV factor = 0.5 / (1 << (SrcTraits::BITS - 2)); + static_assert(factor > 0, "Wrong factor"); gcc_const static DV Convert(SV src) noexcept { From a3f7127e7250196ce3cb7224739dffa2f10bf72e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 29 Oct 2018 22:50:06 +0100 Subject: [PATCH 7/8] pcm/FloatConvert: use FloatToIntegerSampleConvert::factor for IntegerToFloatSampleConvert::factor --- src/pcm/FloatConvert.hxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pcm/FloatConvert.hxx b/src/pcm/FloatConvert.hxx index 5a1756b36..2270fbb90 100644 --- a/src/pcm/FloatConvert.hxx +++ b/src/pcm/FloatConvert.hxx @@ -54,7 +54,7 @@ struct IntegerToFloatSampleConvert { typedef typename SrcTraits::value_type SV; typedef typename DstTraits::value_type DV; - static constexpr DV factor = 0.5 / (1 << (SrcTraits::BITS - 2)); + static constexpr DV factor = 1.0 / FloatToIntegerSampleConvert::factor; static_assert(factor > 0, "Wrong factor"); gcc_const From cc5fab28af0a37aa48a6f445e1ac5cc5ef80f933 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 29 Oct 2018 22:47:53 +0100 Subject: [PATCH 8/8] pcm/FloatConvert: fix compile-time integer overflow for S32 The compile-time calculation for `factor` overflows because `1<<31` cannot be represented by `int`. By casting to `uintmax_t` first, we can avoid this overflow. Closes #380 --- NEWS | 1 + src/pcm/FloatConvert.hxx | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index ffb1a2ef2..083855fd6 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ ver 0.20.23 (not yet released) * protocol - emit "player" idle event when restarting the current song +* fix broken float to s32 conversion * new clang crash bug workaround ver 0.20.22 (2018/10/23) diff --git a/src/pcm/FloatConvert.hxx b/src/pcm/FloatConvert.hxx index 2270fbb90..70b00bcba 100644 --- a/src/pcm/FloatConvert.hxx +++ b/src/pcm/FloatConvert.hxx @@ -34,7 +34,7 @@ struct FloatToIntegerSampleConvert { typedef typename SrcTraits::long_type SL; typedef typename DstTraits::value_type DV; - static constexpr SV factor = 1 << (DstTraits::BITS - 1); + static constexpr SV factor = uintmax_t(1) << (DstTraits::BITS - 1); static_assert(factor > 0, "Wrong factor"); gcc_const