Date: Mon, 01 Jun 2009 18:07:52 +0300 From: Andriy Gapon <avg@freebsd.org> To: freebsd-stable@freebsd.org, freebsd-current@freebsd.org, Henri Hennebert <hlh@restart.be> Cc: Kip Macy <kmacy@freebsd.org> Subject: Re: libzpool assert vs libc assert Message-ID: <4A23EEC8.2040208@freebsd.org> In-Reply-To: <4A1FD687.5070502@freebsd.org> References: <3c1674c90905201643m540c8b1v8a8bd88f071c233d@mail.gmail.com> <4A1D0F2B.4030006@restart.be> <ed91d4a80905271104g2a824d0fna004d1c4f3126c67@mail.gmail.com> <3c1674c90905280052q281f6172j2409fe2d64db6914@mail.gmail.com> <4A1E90F7.2000000@restart.be> <4A1E97D8.4080901@icyb.net.ua> <4A1FD687.5070502@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4A23EEC8.2040208>