#107 Please add __attribute__((__format__ (__printf__, ))) to _ck_assert_failed
_ck_assert_failed doesn't use attribute((format (printf, ...))) to ensure that the compiler warns/errors out when auditing the arguments passed into _ck_assert_failed. See this link for more details: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-g_t_0040code_007bformat_007d-function-attribute-3084 .
284 void _ck_assert_failed(const char file, int line, const char expr, ...)
285 {
If I passed in something like this:
_ck_assert_failed("some-file", 1, "I am a bad format string: %s");
It would print out uninitialized garbage. Example:
$ cat bad_va_arg.c
include <sys cdefs.h="">
include <stdarg.h>
include <stdio.h>
//void callme(const char, ...) __printflike(1, 2);
void
callme(const char fmt, ...)
{
va_list ap;
char buf[BUFSIZ];
int n;
va_start(ap, fmt);
n = vsnprintf(buf, sizeof(buf), fmt, ap);
va_end(ap);
if (n <= 0)
return;
fputs(buf, stdout);
}
int
main(void)
{
callme("I am a fun string %s\n");
callme("I am a fun string %s\n", "now");
return (0);
}
$ clang -Wall -o bad_va_arg bad_va_arg.c
$ ./bad_va_arg
I am a fun string �����
I am a fun string now
$ gcc -Wall -o bad_va_arg bad_va_arg.c
$ ./bad_va_arg
I am a fun string �����
I am a fun string now
It would be nice if _ck_assert_failed generated a compiler warning, instead of making potentially hard to debug asserts impossible to debug, so devs knew that their code was broken.
Example uncommenting the commented out code:
$ gcc -Wall -o bad_va_arg bad_va_arg.c
$ ./bad_va_arg
I am a fun string �����
I am a fun string now
$ clang -Wall -o bad_va_arg bad_va_arg.c
bad_va_arg.c:25:29: warning: more '%' conversions than data arguments [-Wformat]
callme("I am a fun string %s\n");
~^
1 warning generated.
Add on -Werror and magic, now the calling code will error out when provided bad format strings.
I attempted to give this a shot on clang. I found that if one were to
invoke _ck_assert_failed() directly, for example:
_ck_assert_failed(FILE, LINE, "Something bad happened: %s", "generic
message", "extra");
that a warning is produced as expected:
check_nofork.c:21:90: warning: data argument not used by format string
...LINE, "Something bad happened: %s", "generic message", "extra");
However, using any call that eventually uses _ck_assert_failed(), either
with or without a formatting problem, always results in the following
warning being produced:
check_nofork.c:19:3: warning: data argument not used by format string
ck_assert_str_eq("test", s);
../src/check.h:679:32: note: expanded from macro 'ck_assert_str_eq'
define ck_assert_str_eq(X, Y) _ck_assert_str(X, ==, Y)
../src/check.h:665:5: note: expanded from macro '_ck_assert_str'
_ck_x, #Y, _ck_y); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/check.h:466:78: note: expanded from macro 'ck_assert_msg'
...LINE, "Assertion '"#expr"' failed" , ## VA_ARGS, NULL)
A further example:
check_nofork.c:22:25: warning: data argument not used by format string
ck_assert_msg(1 == 0, "Something bad happened: %s", "generic message",
"should be an int");
~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/check.h:466:78: note: expanded from macro 'ck_assert_msg'
...LINE, "Assertion '"#expr"' failed" , ## VA_ARGS, NULL)
That is, it appears that using the format attribute with clang results in
clang not agreeing with VA_ARGS and always producing a warning.
It appears that something would need to be done to how check handles
variable arguments before the format attribute could be enabled in a
meaningful way. If you happen to have suggestions or are interested in
attempting this, patches are always welcome. (:
On Tue, Jun 30, 2015 at 8:37 PM, Garrett Cooper yaberauneya@users.sf.net
wrote:
Related
Bugs:
#107There has been no further feedback on this issue. If anyone is interested in following it up with a proposal, kindly submit a pull requests against Check in its new home:
https://github.com/libcheck/check