avmedia/source/viewer/mediawindow.cxx | 8 ++++---- external/firebird/ExternalProject_firebird.mk | 2 -- external/firebird/asan.patch | 13 ++++++------- 3 files changed, 10 insertions(+), 13 deletions(-)
New commits: commit 0a910746b19f10f184f6ff8f41c868994a472ca9 Author: Stephan Bergmann <[email protected]> Date: Mon Apr 16 16:48:41 2018 +0200 Clarify checking o_pbLink directly, versus missing dereferencing But given 18ab7abaa9426cd27092125637fdf5fb849b4a04 "MediaWindow::executeMediaURLDialog: add link button" documents that parameter as "if not 0, this is an 'insert' dialog", it looks plausible that all these places indeed want to check the pointer-to-bool for non-nullness, rather than checking the dereferenced bool. Change-Id: Ifbd5e4b711bee97f3ba0b6aab556f533574d21c6 Reviewed-on: https://gerrit.libreoffice.org/52992 Reviewed-by: Michael Stahl <[email protected]> Tested-by: Jenkins <[email protected]> diff --git a/avmedia/source/viewer/mediawindow.cxx b/avmedia/source/viewer/mediawindow.cxx index 2b2f0937f562..0fd10f508bca 100644 --- a/avmedia/source/viewer/mediawindow.cxx +++ b/avmedia/source/viewer/mediawindow.cxx @@ -211,7 +211,7 @@ void MediaWindow::getMediaFilters( FilterNameVector& rFilterNameVector ) bool MediaWindow::executeMediaURLDialog(weld::Window* pParent, OUString& rURL, bool *const o_pbLink) { - ::sfx2::FileDialogHelper aDlg(o_pbLink + ::sfx2::FileDialogHelper aDlg(o_pbLink != nullptr ? ui::dialogs::TemplateDescription::FILEOPEN_LINK_PREVIEW : ui::dialogs::TemplateDescription::FILEOPEN_SIMPLE, FileDialogFlags::NONE, pParent); @@ -220,7 +220,7 @@ bool MediaWindow::executeMediaURLDialog(weld::Window* pParent, OUString& rURL, b static const char aSeparator[] = ";"; OUString aAllTypes; - aDlg.SetTitle( AvmResId( o_pbLink + aDlg.SetTitle( AvmResId( o_pbLink != nullptr ? AVMEDIA_STR_INSERTMEDIA_DLG : AVMEDIA_STR_OPENMEDIA_DLG ) ); getMediaFilters( aFilters ); @@ -261,7 +261,7 @@ bool MediaWindow::executeMediaURLDialog(weld::Window* pParent, OUString& rURL, b uno::Reference<ui::dialogs::XFilePicker3> const xFP(aDlg.GetFilePicker()); uno::Reference<ui::dialogs::XFilePickerControlAccess> const xCtrlAcc(xFP, uno::UNO_QUERY_THROW); - if (o_pbLink) + if (o_pbLink != nullptr) { // for video link should be the default xCtrlAcc->setValue( @@ -278,7 +278,7 @@ bool MediaWindow::executeMediaURLDialog(weld::Window* pParent, OUString& rURL, b const INetURLObject aURL( aDlg.GetPath() ); rURL = aURL.GetMainURL( INetURLObject::DecodeMechanism::Unambiguous ); - if (o_pbLink) + if (o_pbLink != nullptr) { uno::Any const any = xCtrlAcc->getValue( ui::dialogs::ExtendedFilePickerElementIds::CHECKBOX_LINK, 0); commit 25764ffd4db0e5db6f9cc9f3da8691e607f48b83 Author: Stephan Bergmann <[email protected]> Date: Mon Apr 16 11:09:22 2018 +0200 external/firebird: Better workaround for Clang alignment expectations 8ea07101c1613d213fd7cea17f094a947b14cd00 "external/firebird: Work around operator new alignment violations" had misused DEBUG_GDS_ALLOC to work around the problem that recent Clang on Linux x86-64 assumes some storage allocated via Firebird's global operator new replacement to be 16-byte aligned, while Firebird only provides 8-byte alignment. This problem now also appears on macOS x86-64, at least with Apple's Clang version "Apple LLVM version 9.1.0 (clang-902.0.39.1)" from Xcode 9.3 (as well as with recent plain Clang trunk, towards Clang 7), when building --enable- optimized. However, while the DEBUG_GDS_ALLOC hack would still cause ICU_convert_init (workdir/UnpackedTarball/firebird/temp/Release/intl/cv_icu.o) to not use movaps on erroneously assumed to be 16-byte aligned memory, it would cause strange failures on macOS (pos being out of bounds in traRpbList::PopRpb, workdir/UnpackedTarball/firebird/src/jrd/rpb_chain.cpp, in the invocation of isql during the build of external/firebird) that I haven't tracked down further. As it happens, the recently added 14955ed91b282ccbb395cb47c6d76e3b42b34748 "external/firebird: Support Clang ASan" provides a different workaround for the underlying problem that appears to work well on both Linux and macOS x86-64, reusing USE_ASAN also in these cases to shut down most of Firebird's own memory management. I assume that affected Clang are plain Clang >= 4 (as I'd mentioned in my <https://sourceforge.net/p/firebird/mailman/message/35671804/> "Re: [Firebird- devel] alloc.h global operator new replacement violating alignment requirements") and Apple Clang >= 9 (for which __apple_build_version__ is defined). Because DEBUG_GDS_ALLOC is no longer passed in from the outside, its setting in external/firebird/asan.patch can be simplified (cf. commit message of 14955ed91b282ccbb395cb47c6d76e3b42b34748). (The given scenario in ICU_convert_init involves an allocation of 24 bytes, where Clang may or may not be allowed to assume 16-byte alignment, see <http://lists.llvm.org/pipermail/cfe-dev/2018-April/057669.html> "Re: [cfe-dev] operator new alignment assumptions". However, as reported at <https://sourceforge.net/p/firebird/mailman/message/35671750/> "Re: [Firebird- devel] alloc.h global operator new replacement violating alignment requirements", Firebird only supports 8-byte alignment, which would definitely be wrong in a similar scenario where the requested allocation was a multiple of 16 bytes.) Change-Id: I48884f9d008eaeaea369850e24f05b8806f9b676 Reviewed-on: https://gerrit.libreoffice.org/52956 Tested-by: Jenkins <[email protected]> Reviewed-by: Stephan Bergmann <[email protected]> diff --git a/external/firebird/ExternalProject_firebird.mk b/external/firebird/ExternalProject_firebird.mk index 5ca8c0b01418..d370df0d83ae 100644 --- a/external/firebird/ExternalProject_firebird.mk +++ b/external/firebird/ExternalProject_firebird.mk @@ -58,8 +58,6 @@ $(call gb_ExternalProject_get_state_target,firebird,build): " \ && export CXXFLAGS=" \ $(if $(filter MSC,$(COM)),$(if $(MSVC_USE_DEBUG_RUNTIME),-DMSVC_USE_DEBUG_RUNTIME)) \ - $(if $(filter LINUX/X86_64/TRUE,$(OS)/$(CPUNAME)/$(COM_IS_CLANG)), \ - -DDEBUG_GDS_ALLOC) \ $(if $(HAVE_GCC_FNO_SIZED_DEALLOCATION),-fno-sized-deallocation -fno-delete-null-pointer-checks) \ $(if $(SYSTEM_BOOST),$(BOOST_CPPFLAGS), \ $(BOOST_CPPFLAGS) \ diff --git a/external/firebird/asan.patch b/external/firebird/asan.patch index 4c40f52bfc87..b6ecec8f4caf 100644 --- a/external/firebird/asan.patch +++ b/external/firebird/asan.patch @@ -204,7 +204,7 @@ #ifdef DEBUG_GDS_ALLOC --- src/include/firebird.h +++ src/include/firebird.h -@@ -38,10 +38,19 @@ +@@ -38,8 +38,17 @@ #include "gen/autoconfig.h" #endif @@ -213,17 +213,16 @@ +#define USE_ASAN +#endif +#endif ++#if defined __clang__ && (defined __apple_build_version__ ? __clang_major__ >= 9 : __clang_major__ >= 4) ++#define USE_ASAN ++#endif + // Using our debugging code is pointless when we may use Valgrind features - #if defined(DEV_BUILD) && !defined(USE_VALGRIND) +-#if defined(DEV_BUILD) && !defined(USE_VALGRIND) ++#if defined(DEV_BUILD) && !(defined(USE_VALGRIND) || defined(USE_ASAN)) #define DEBUG_GDS_ALLOC #endif -+#if defined USE_ASAN -+#undef DEBUG_GDS_ALLOC -+#endif - #if defined(WIN_NT) - #define FB_DLL_EXPORT __declspec(dllexport) --- src/jrd/SimilarToMatcher.h +++ src/jrd/SimilarToMatcher.h @@ -338,7 +338,7 @@ _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
