]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
include/ceph_assert.h: do not pack assert params if WITH_ASAN 24692/head
authorKefu Chai <kchai@redhat.com>
Mon, 22 Oct 2018 08:41:45 +0000 (16:41 +0800)
committerKefu Chai <kchai@redhat.com>
Mon, 22 Oct 2018 15:01:36 +0000 (23:01 +0800)
we pack the asset() params for smaller code size, but this creates a
inlined `assert_data_ctx` instance for every compilation unit which
call ceph_assert() defined in .h .

__PRETTY_FUNCTION__ is likely to be referenced by `assert_data_ctx`
sections which are included by different compiled object files. if the
ceph_assert() call is used by header file, then there will be multiple
`assert_data_ctx` sections sharing the same identifier. these sections are
defined as "COMDAT" group sections, i.e. common data sections. when linker
see multiple COMDAT sections with the same identifer, it will simply discard
the duplicated ones, and only keep a single copy of them. without enabling
ASan, GCC can always handle this problem just fine. but the dedup feature
does not work well with ASan. if ASan is enabled, and we link the objects
with the wrong order, some references will be pointing to the discarded
sections.

to address this issue, we could audit the link command line and inspect
all .o files to make sure they are properly ordered. but this is
non-trivial. as a workaround, in this change, the assert params are not
packed, and sent to the  __ceph_assert_fail() overrides which accepts
unpacked params directly, so the COMDAT section is not created.

Signed-off-by: Kefu Chai <kchai@redhat.com>
src/include/ceph_assert.h

index a022d2f2936bd44b8cac06ebdf14c9a86f264837..36d6c430ebf6fc917993b3f396b59bb4f1db4dd4 100644 (file)
@@ -92,21 +92,39 @@ using namespace ceph;
 #define ceph_abort_msgf(...)                                             \
   __ceph_abortf( __FILE__, __LINE__, __CEPH_ASSERT_FUNCTION, __VA_ARGS__)
 
+#ifdef __SANITIZE_ADDRESS__
+#define ceph_assert(expr)                           \
+  do {                                              \
+    ((expr))                                        \
+    ? _CEPH_ASSERT_VOID_CAST (0)                    \
+    : __ceph_assert_fail(__STRING(expr), __FILE__, __LINE__, __CEPH_ASSERT_FUNCTION); \
+  } while (false)
+#else
 #define ceph_assert(expr)                                                      \
   do { static const ceph::assert_data assert_data_ctx = \
    {__STRING(expr), __FILE__, __LINE__, __CEPH_ASSERT_FUNCTION}; \
    ((expr) \
    ? _CEPH_ASSERT_VOID_CAST (0) \
    : __ceph_assert_fail(assert_data_ctx)); } while(false)
+#endif
 
 // this variant will *never* get compiled out to NDEBUG in the future.
 // (ceph_assert currently doesn't either, but in the future it might.)
+#ifdef __SANITIZE_ADDRESS__
+#define ceph_assert_always(expr)                    \
+  do {                                              \
+    ((expr))                                        \
+    ? _CEPH_ASSERT_VOID_CAST (0)                    \
+    : __ceph_assert_fail(__STRING(expr), __FILE__, __LINE__, __CEPH_ASSERT_FUNCTION); \
+  } while(false)
+#else
 #define ceph_assert_always(expr)                                                       \
   do { static const ceph::assert_data assert_data_ctx = \
    {__STRING(expr), __FILE__, __LINE__, __CEPH_ASSERT_FUNCTION}; \
    ((expr) \
    ? _CEPH_ASSERT_VOID_CAST (0) \
    : __ceph_assert_fail(assert_data_ctx)); } while(false)
+#endif
 
 // Named by analogy with printf.  Along with an expression, takes a format
 // string and parameters which are printed if the assertion fails.