Re: libzpool assert vs libc assert

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
_at__at_ -37,15 +37,7 _at__at_
 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
 }
_at__at_ -68,14 +60,6 _at__at_ 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
_at__at_ -120,21 +120,12 _at__at_ 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)
_at__at_ -148,7 +139,7 _at__at_ 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