Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add -Wformat=2 compiler flag #860

Merged
merged 1 commit into from
Jan 29, 2025
Merged

Add -Wformat=2 compiler flag #860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ if(NOT MSVC AND NOT IOS)
xcheck_add_c_compiler_flag(-Werror)
xcheck_add_c_compiler_flag(-Wextra)
endif()
xcheck_add_c_compiler_flag(-Wformat=2)
xcheck_add_c_compiler_flag(-Wno-implicit-fallthrough)
xcheck_add_c_compiler_flag(-Wno-sign-compare)
xcheck_add_c_compiler_flag(-Wno-missing-field-initializers)
xcheck_add_c_compiler_flag(-Wno-unused-parameter)
xcheck_add_c_compiler_flag(-Wno-unused-but-set-variable)
xcheck_add_c_compiler_flag(-Wno-array-bounds)
xcheck_add_c_compiler_flag(-Wno-format-truncation)
xcheck_add_c_compiler_flag(-funsigned-char)

# ClangCL is command line compatible with MSVC, so 'MSVC' is set.
Expand Down
14 changes: 6 additions & 8 deletions libbf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3962,22 +3962,20 @@ static char *bf_ftoa_internal(size_t *plen, const bf_t *a2, int radix,
n = 1;
if ((flags & BF_FTOA_FORCE_EXP) ||
n <= -6 || n > n_max) {
const char *fmt;
/* exponential notation */
output_digits(s, a1, radix, n_digits, 1, is_dec);
if (radix_bits != 0 && radix <= 16) {
slimb_t exp_n = (n - 1) * radix_bits;
if (flags & BF_FTOA_JS_QUIRKS)
fmt = "p%+" PRId_LIMB;
dbuf_printf(s, "p%+" PRId_LIMB, exp_n);
else
fmt = "p%" PRId_LIMB;
dbuf_printf(s, fmt, (n - 1) * radix_bits);
dbuf_printf(s, "p%" PRId_LIMB, exp_n);
} else {
const char c = radix <= 10 ? 'e' : '@';
if (flags & BF_FTOA_JS_QUIRKS)
fmt = "%c%+" PRId_LIMB;
dbuf_printf(s, "%c%+" PRId_LIMB, c, n - 1);
else
fmt = "%c%" PRId_LIMB;
dbuf_printf(s, fmt,
radix <= 10 ? 'e' : '@', n - 1);
dbuf_printf(s, "%c%" PRId_LIMB, c, n - 1);
}
} else if (n <= 0) {
/* 0.x */
Expand Down
14 changes: 8 additions & 6 deletions quickjs-libc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ static void js_set_thread_state(JSRuntime *rt, JSThreadState *ts)
js_std_cmd(/*SetOpaque*/1, rt, ts);
}

#pragma GCC diagnostic push
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are going to work around the compiler, might as well be straight about it.

#pragma GCC diagnostic ignored "-Wformat-nonliteral"
static JSValue js_printf_internal(JSContext *ctx,
int argc, JSValue *argv, FILE *fp)
{
Expand All @@ -207,7 +209,6 @@ static JSValue js_printf_internal(JSContext *ctx,
int64_t int64_arg;
double double_arg;
const char *string_arg;
int (*dbuf_printf_fun)(DynBuf *s, const char *fmt, ...) = dbuf_printf;

js_std_dbuf_init(ctx, &dbuf);

Expand Down Expand Up @@ -332,17 +333,17 @@ static JSValue js_printf_internal(JSContext *ctx,
q[0] = '6';
q[1] = '4';
q[3] = '\0';
dbuf_printf_fun(&dbuf, fmtbuf, (int64_t)int64_arg);
dbuf_printf(&dbuf, fmtbuf, (int64_t)int64_arg);
#else
if (q >= fmtbuf + sizeof(fmtbuf) - 2)
goto invalid;
q[1] = q[-1];
q[-1] = q[0] = 'l';
q[2] = '\0';
dbuf_printf_fun(&dbuf, fmtbuf, (long long)int64_arg);
dbuf_printf(&dbuf, fmtbuf, (long long)int64_arg);
#endif
} else {
dbuf_printf_fun(&dbuf, fmtbuf, (int)int64_arg);
dbuf_printf(&dbuf, fmtbuf, (int)int64_arg);
}
break;

Expand All @@ -353,7 +354,7 @@ static JSValue js_printf_internal(JSContext *ctx,
string_arg = JS_ToCString(ctx, argv[i++]);
if (!string_arg)
goto fail;
dbuf_printf_fun(&dbuf, fmtbuf, string_arg);
dbuf_printf(&dbuf, fmtbuf, string_arg);
JS_FreeCString(ctx, string_arg);
break;

Expand All @@ -369,7 +370,7 @@ static JSValue js_printf_internal(JSContext *ctx,
goto missing;
if (JS_ToFloat64(ctx, &double_arg, argv[i++]))
goto fail;
dbuf_printf_fun(&dbuf, fmtbuf, double_arg);
dbuf_printf(&dbuf, fmtbuf, double_arg);
break;

case '%':
Expand Down Expand Up @@ -406,6 +407,7 @@ static JSValue js_printf_internal(JSContext *ctx,
dbuf_free(&dbuf);
return JS_EXCEPTION;
}
#pragma GCC diagnostic pop // ignored "-Wformat-nonliteral"

uint8_t *js_load_file(JSContext *ctx, size_t *pbuf_len, const char *filename)
{
Expand Down
44 changes: 17 additions & 27 deletions quickjs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3197,7 +3197,6 @@ JSValue JS_NewSymbol(JSContext *ctx, const char *description, bool is_global)

#define ATOM_GET_STR_BUF_SIZE 64

/* Should only be used for debug. */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also used to print exception messages.

static const char *JS_AtomGetStrRT(JSRuntime *rt, char *buf, int buf_size,
JSAtom atom)
{
Expand All @@ -3220,13 +3219,6 @@ static const char *JS_AtomGetStrRT(JSRuntime *rt, char *buf, int buf_size,
/* encode surrogates correctly */
utf8_encode_buf16(buf, buf_size, str->u.str16, str->len);
} else {
/* special case ASCII strings */
int i, c = 0;
for(i = 0; i < str->len; i++) {
c |= str->u.str8[i];
}
if (c < 0x80)
return (const char *)str->u.str8;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This return broke the "contract" that the function filled the buffer in. Since this is hardly a hot path, let's be consistent and copy the content to the provided buffer.

utf8_encode_buf8(buf, buf_size, str->u.str8, str->len);
}
}
Expand Down Expand Up @@ -6856,8 +6848,9 @@ static JSValue JS_MakeError(JSContext *ctx, JSErrorEnum error_num,
}

/* fmt and arguments may be pure ASCII or UTF-8 encoded contents */
static JSValue JS_ThrowError2(JSContext *ctx, JSErrorEnum error_num,
const char *fmt, va_list ap, bool add_backtrace)
static JSValue JS_PRINTF_FORMAT_ATTR(4, 0)
JS_ThrowError2(JSContext *ctx, JSErrorEnum error_num,
bool add_backtrace, JS_PRINTF_FORMAT const char *fmt, va_list ap)
{
char buf[256];
JSValue obj;
Expand All @@ -6871,8 +6864,9 @@ static JSValue JS_ThrowError2(JSContext *ctx, JSErrorEnum error_num,
return JS_Throw(ctx, obj);
}

static JSValue JS_ThrowError(JSContext *ctx, JSErrorEnum error_num,
const char *fmt, va_list ap)
static JSValue JS_PRINTF_FORMAT_ATTR(3, 0)
JS_ThrowError(JSContext *ctx, JSErrorEnum error_num,
JS_PRINTF_FORMAT const char *fmt, va_list ap)
{
JSRuntime *rt = ctx->rt;
JSStackFrame *sf;
Expand All @@ -6882,7 +6876,7 @@ static JSValue JS_ThrowError(JSContext *ctx, JSErrorEnum error_num,
sf = rt->current_stack_frame;
add_backtrace = !rt->in_out_of_memory &&
(!sf || (JS_GetFunctionBytecode(sf->cur_func) == NULL));
return JS_ThrowError2(ctx, error_num, fmt, ap, add_backtrace);
return JS_ThrowError2(ctx, error_num, add_backtrace, fmt, ap);
}

JSValue JS_PRINTF_FORMAT_ATTR(2, 3) JS_ThrowPlainError(JSContext *ctx, JS_PRINTF_FORMAT const char *fmt, ...)
Expand Down Expand Up @@ -6933,26 +6927,22 @@ static int JS_PRINTF_FORMAT_ATTR(3, 4) JS_ThrowTypeErrorOrFalse(JSContext *ctx,
}
}

/* never use it directly */
static JSValue JS_PRINTF_FORMAT_ATTR(3, 4) __JS_ThrowTypeErrorAtom(JSContext *ctx, JSAtom atom, JS_PRINTF_FORMAT const char *fmt, ...)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
static JSValue JS_ThrowTypeErrorAtom(JSContext *ctx, const char *fmt, JSAtom atom)
{
char buf[ATOM_GET_STR_BUF_SIZE];
return JS_ThrowTypeError(ctx, fmt,
JS_AtomGetStr(ctx, buf, sizeof(buf), atom));
JS_AtomGetStr(ctx, buf, sizeof(buf), atom);
return JS_ThrowTypeError(ctx, fmt, buf);
}

/* never use it directly */
static JSValue JS_PRINTF_FORMAT_ATTR(3, 4) __JS_ThrowSyntaxErrorAtom(JSContext *ctx, JSAtom atom, JS_PRINTF_FORMAT const char *fmt, ...)
static JSValue JS_ThrowSyntaxErrorAtom(JSContext *ctx, const char *fmt, JSAtom atom)
{
char buf[ATOM_GET_STR_BUF_SIZE];
return JS_ThrowSyntaxError(ctx, fmt,
JS_AtomGetStr(ctx, buf, sizeof(buf), atom));
JS_AtomGetStr(ctx, buf, sizeof(buf), atom);
return JS_ThrowSyntaxError(ctx, fmt, buf);
}

/* %s is replaced by 'atom'. The macro is used so that gcc can check
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually didn't work, it errors in bellart/quickjs if you enable -Wformat=2

the format string. */
#define JS_ThrowTypeErrorAtom(ctx, fmt, atom) __JS_ThrowTypeErrorAtom(ctx, atom, fmt, "")
#define JS_ThrowSyntaxErrorAtom(ctx, fmt, atom) __JS_ThrowSyntaxErrorAtom(ctx, atom, fmt, "")
#pragma GCC diagnostic pop // ignored "-Wformat-nonliteral"

static int JS_ThrowTypeErrorReadOnly(JSContext *ctx, int flags, JSAtom atom)
{
Expand Down Expand Up @@ -19038,7 +19028,7 @@ int JS_PRINTF_FORMAT_ATTR(2, 3) js_parse_error(JSParseState *s, JS_PRINTF_FORMAT
int backtrace_flags;

va_start(ap, fmt);
JS_ThrowError2(ctx, JS_SYNTAX_ERROR, fmt, ap, false);
JS_ThrowError2(ctx, JS_SYNTAX_ERROR, false, fmt, ap);
va_end(ap);
backtrace_flags = 0;
if (s->cur_func && s->cur_func->backtrace_barrier)
Expand Down