From owner-freebsd-current@FreeBSD.ORG Mon Jun 1 15:07:56 2009 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 241DB106566B; Mon, 1 Jun 2009 15:07:56 +0000 (UTC) (envelope-from avg@freebsd.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id D48528FC15; Mon, 1 Jun 2009 15:07:54 +0000 (UTC) (envelope-from avg@freebsd.org) Received: from odyssey.starpoint.kiev.ua (alpha-e.starpoint.kiev.ua [212.40.38.101]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id SAA06754; Mon, 01 Jun 2009 18:07:52 +0300 (EEST) (envelope-from avg@freebsd.org) Message-ID: <4A23EEC8.2040208@freebsd.org> Date: Mon, 01 Jun 2009 18:07:52 +0300 From: Andriy Gapon User-Agent: Thunderbird 2.0.0.21 (X11/20090406) MIME-Version: 1.0 To: freebsd-stable@freebsd.org, freebsd-current@freebsd.org, Henri Hennebert References: <3c1674c90905201643m540c8b1v8a8bd88f071c233d@mail.gmail.com> <4A1D0F2B.4030006@restart.be> <3c1674c90905280052q281f6172j2409fe2d64db6914@mail.gmail.com> <4A1E90F7.2000000@restart.be> <4A1E97D8.4080901@icyb.net.ua> <4A1FD687.5070502@freebsd.org> In-Reply-To: <4A1FD687.5070502@freebsd.org> X-Enigmail-Version: 0.95.7 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Kip Macy Subject: Re: libzpool assert vs libc assert X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Jun 2009 15:07:56 -0000 on 29/05/2009 15:35 Andriy Gapon said the following: > So anyone else feels that this is a bug? > > on 28/05/2009 16:55 Andriy Gapon said the following: >> on 28/05/2009 16:26 Henri Hennebert said the following: >>> (gdb) bt >>> #0 0x00000008012a6f22 in strlen () from /lib/libc.so.7 >>> #1 0x00000008012a0feb in open () from /lib/libc.so.7 >>> #2 0x000000080129ea59 in open () from /lib/libc.so.7 >>> #3 0x00000008012a1f2e in vfprintf () from /lib/libc.so.7 >>> #4 0x0000000801291158 in fprintf () from /lib/libc.so.7 >>> #5 0x0000000801290fb0 in __assert () from /lib/libc.so.7 >> I find the above part interesting. >> Could this be because of the following discrepancy: >> >> 1) >> cddl/contrib/opensolaris/lib/libzpool/common/sys/zfs_context.h: >> extern void __assert(const char *, const char *, int); >> 2) >> lib/libc/gen/assert.c: >> void >> __assert(func, file, line, failedexpr) >> const char *func, *file; >> int line; >> const char *failedexpr; >> >>> #6 0x0000000800fef120 in zmutex_destroy () from /lib/libzpool.so.1 >>> #7 0x000000080102e1a0 in dsl_dataset_fast_stat () from /lib/libzpool.so.1 >>> #8 0x0000000801045ffa in dbuf_find () from /lib/libzpool.so.1 >>> #9 0x0000000801047bf3 in dmu_buf_rele () from /lib/libzpool.so.1 >>> #10 0x0000000801027546 in dsl_pool_open () from /lib/libzpool.so.1 >>> #11 0x000000080101bcec in spa_create () from /lib/libzpool.so.1 >>> #12 0x000000080101c820 in spa_tryimport () from /lib/libzpool.so.1 I propose the following patch for this issue. It fixes mismatch between __assert extern declaration in zfs code and actual signature in libc code. I also took liberty of dropping __STDC__ and __STDC_VERSION__ checks. I think that those checks are not needed with compilers that can be used to compile FreeBSD. Besides, both branches of __STDC_VERSION__ check were exactly the same. Henri, if you still experience that crash of zpool command, could you please try the patch and see if you have a nicer assert message and stacktrace now? Sorry, that this is still not a fix for the real issue. diff --git a/cddl/contrib/opensolaris/head/assert.h b/cddl/contrib/opensolaris/head/assert.h index 394820a..c2a4936 100644 --- a/cddl/contrib/opensolaris/head/assert.h +++ b/cddl/contrib/opensolaris/head/assert.h @@ -37,15 +37,7 @@ extern "C" { #endif -#if defined(__STDC__) -#if __STDC_VERSION__ - 0 >= 199901L -extern void __assert(const char *, const char *, int); -#else -extern void __assert(const char *, const char *, int); -#endif /* __STDC_VERSION__ - 0 >= 199901L */ -#else -extern void _assert(); -#endif +extern void __assert(const char *, const char *, int, const char *); #ifdef __cplusplus } @@ -68,14 +60,6 @@ extern void _assert(); #else -#if defined(__STDC__) -#if __STDC_VERSION__ - 0 >= 199901L -#define assert(EX) (void)((EX) || (__assert(#EX, __FILE__, __LINE__), 0)) -#else -#define assert(EX) (void)((EX) || (__assert(#EX, __FILE__, __LINE__), 0)) -#endif /* __STDC_VERSION__ - 0 >= 199901L */ -#else -#define assert(EX) (void)((EX) || (_assert("EX", __FILE__, __LINE__), 0)) -#endif /* __STDC__ */ +#define assert(EX) (void)((EX) || (__assert(__func__, __FILE__, __LINE__, #EX), 0)) #endif /* NDEBUG */ diff --git a/cddl/contrib/opensolaris/lib/libzpool/common/sys/zfs_context.h b/cddl/contrib/opensolaris/lib/libzpool/common/sys/zfs_context.h index 7ae7f9d..631e302 100644 --- a/cddl/contrib/opensolaris/lib/libzpool/common/sys/zfs_context.h +++ b/cddl/contrib/opensolaris/lib/libzpool/common/sys/zfs_context.h @@ -120,21 +120,12 @@ extern void vpanic(const char *, __va_list); #define fm_panic panic /* This definition is copied from assert.h. */ -#if defined(__STDC__) -#if __STDC_VERSION__ - 0 >= 199901L -#define verify(EX) (void)((EX) || (__assert(#EX, __FILE__, __LINE__), 0)) -#else -#define verify(EX) (void)((EX) || (__assert(#EX, __FILE__, __LINE__), 0)) -#endif /* __STDC_VERSION__ - 0 >= 199901L */ -#else -#define verify(EX) (void)((EX) || (_assert("EX", __FILE__, __LINE__), 0)) -#endif /* __STDC__ */ - +#define verify(EX) (void)((EX) || (__assert(__func__, __FILE__, __LINE__, #EX), 0)) #define VERIFY verify #define ASSERT assert -extern void __assert(const char *, const char *, int); +extern void __assert(const char *, const char *, int, const char *); #ifdef lint #define VERIFY3_IMPL(x, y, z, t) if (x == z) ((void)0) @@ -148,7 +139,7 @@ extern void __assert(const char *, const char *, int); (void) snprintf(__buf, 256, "%s %s %s (0x%llx %s 0x%llx)", \ #LEFT, #OP, #RIGHT, \ (u_longlong_t)__left, #OP, (u_longlong_t)__right); \ - __assert(__buf, __FILE__, __LINE__); \ + __assert(__func__, __FILE__, __LINE__, __buf); \ } \ _NOTE(CONSTCOND) } while (0) /* END CSTYLED */ -- Andriy Gapon