Kaydet (Commit) 25764ffd authored tarafından Stephan Bergmann's avatar Stephan Bergmann

external/firebird: Better workaround for Clang alignment expectations

8ea07101 "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 14955ed9
"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
14955ed9).

(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/52956Tested-by: 's avatarJenkins <ci@libreoffice.org>
Reviewed-by: 's avatarStephan Bergmann <sbergman@redhat.com>
üst defa12ce
...@@ -58,8 +58,6 @@ $(call gb_ExternalProject_get_state_target,firebird,build): ...@@ -58,8 +58,6 @@ $(call gb_ExternalProject_get_state_target,firebird,build):
" \ " \
&& export CXXFLAGS=" \ && export CXXFLAGS=" \
$(if $(filter MSC,$(COM)),$(if $(MSVC_USE_DEBUG_RUNTIME),-DMSVC_USE_DEBUG_RUNTIME)) \ $(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 $(HAVE_GCC_FNO_SIZED_DEALLOCATION),-fno-sized-deallocation -fno-delete-null-pointer-checks) \
$(if $(SYSTEM_BOOST),$(BOOST_CPPFLAGS), \ $(if $(SYSTEM_BOOST),$(BOOST_CPPFLAGS), \
$(BOOST_CPPFLAGS) \ $(BOOST_CPPFLAGS) \
......
...@@ -204,7 +204,7 @@ ...@@ -204,7 +204,7 @@
#ifdef DEBUG_GDS_ALLOC #ifdef DEBUG_GDS_ALLOC
--- src/include/firebird.h --- src/include/firebird.h
+++ src/include/firebird.h +++ src/include/firebird.h
@@ -38,10 +38,19 @@ @@ -38,8 +38,17 @@
#include "gen/autoconfig.h" #include "gen/autoconfig.h"
#endif #endif
...@@ -213,17 +213,16 @@ ...@@ -213,17 +213,16 @@
+#define USE_ASAN +#define USE_ASAN
+#endif +#endif
+#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 // 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 #define DEBUG_GDS_ALLOC
#endif #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
+++ src/jrd/SimilarToMatcher.h +++ src/jrd/SimilarToMatcher.h
@@ -338,7 +338,7 @@ @@ -338,7 +338,7 @@
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment