From c7787cbaa1daf4fa53fd53b679584b67b0d858f2 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sun, 10 Jan 2021 09:17:52 -0800 Subject: [PATCH] Fixed a bunch of Clang warnings. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unfortunately a few of the Clang warnings did not have easy fixes: ../../../../ext/google/protobuf_c/ruby-upb.c: In function ‘fastdecode_err’: ../../../../ext/google/protobuf_c/ruby-upb.c:353:13: warning: function might be candidate for attribute ‘noreturn’ [-Wsuggest-attribute=noreturn] 353 | const char *fastdecode_err(upb_decstate *d) { | ^~~~~~~~~~~~~~ ../../../../ext/google/protobuf_c/ruby-upb.c: In function ‘_upb_decode’: ../../../../ext/google/protobuf_c/ruby-upb.c:867:30: warning: argument ‘buf’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered] 867 | bool _upb_decode(const char *buf, size_t size, void *msg, I even tried to suppress the first error, but it still shows up. --- cmake/upb/json/parser.c | 144 ++++++++++++++++++++-------------------- upb/decode.c | 25 +++++-- upb/decode.int.h | 2 +- upb/def.c | 4 +- upb/json/parser.rl | 6 +- upb/json_decode.c | 9 +-- upb/json_encode.c | 4 +- upb/port_def.inc | 3 + upb/upb.h | 9 ++- 9 files changed, 118 insertions(+), 88 deletions(-) diff --git a/cmake/upb/json/parser.c b/cmake/upb/json/parser.c index 26996d8a59..bbd66d33eb 100644 --- a/cmake/upb/json/parser.c +++ b/cmake/upb/json/parser.c @@ -1382,7 +1382,8 @@ static bool end_stringval_nontop(upb_json_parser *p) { ok = true; /* TODO(teboring): Should also clean this field. */ } else { - upb_status_seterrf(p->status, "Enum value unknown: '%.*s'", len, buf); + upb_status_seterrf(p->status, "Enum value unknown: '%.*s'", (int)len, + buf); } } @@ -2579,11 +2580,11 @@ static bool does_fieldmask_end(upb_json_parser *p) { * final state once, when the closing '"' is seen. */ -#line 2784 "upb/json/parser.rl" +#line 2785 "upb/json/parser.rl" -#line 2587 "upb/json/parser.c" +#line 2588 "upb/json/parser.c" static const char _json_actions[] = { 0, 1, 0, 1, 1, 1, 3, 1, 4, 1, 6, 1, 7, 1, 8, 1, @@ -2838,7 +2839,7 @@ static const int json_en_value_machine = 78; static const int json_en_main = 1; -#line 2787 "upb/json/parser.rl" +#line 2788 "upb/json/parser.rl" size_t parse(void *closure, const void *hd, const char *buf, size_t size, const upb_bufhandle *handle) { @@ -2861,7 +2862,7 @@ size_t parse(void *closure, const void *hd, const char *buf, size_t size, capture_resume(parser, buf); -#line 2865 "upb/json/parser.c" +#line 2866 "upb/json/parser.c" { int _klen; unsigned int _trans; @@ -2936,147 +2937,147 @@ _match: switch ( *_acts++ ) { case 1: -#line 2592 "upb/json/parser.rl" +#line 2593 "upb/json/parser.rl" { p--; {cs = stack[--top]; goto _again;} } break; case 2: -#line 2594 "upb/json/parser.rl" +#line 2595 "upb/json/parser.rl" { p--; {stack[top++] = cs; cs = 23;goto _again;} } break; case 3: -#line 2598 "upb/json/parser.rl" +#line 2599 "upb/json/parser.rl" { start_text(parser, p); } break; case 4: -#line 2599 "upb/json/parser.rl" +#line 2600 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_text(parser, p)); } break; case 5: -#line 2605 "upb/json/parser.rl" +#line 2606 "upb/json/parser.rl" { start_hex(parser); } break; case 6: -#line 2606 "upb/json/parser.rl" +#line 2607 "upb/json/parser.rl" { hexdigit(parser, p); } break; case 7: -#line 2607 "upb/json/parser.rl" +#line 2608 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_hex(parser)); } break; case 8: -#line 2613 "upb/json/parser.rl" +#line 2614 "upb/json/parser.rl" { CHECK_RETURN_TOP(escape(parser, p)); } break; case 9: -#line 2619 "upb/json/parser.rl" +#line 2620 "upb/json/parser.rl" { p--; {cs = stack[--top]; goto _again;} } break; case 10: -#line 2624 "upb/json/parser.rl" +#line 2625 "upb/json/parser.rl" { start_year(parser, p); } break; case 11: -#line 2625 "upb/json/parser.rl" +#line 2626 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_year(parser, p)); } break; case 12: -#line 2629 "upb/json/parser.rl" +#line 2630 "upb/json/parser.rl" { start_month(parser, p); } break; case 13: -#line 2630 "upb/json/parser.rl" +#line 2631 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_month(parser, p)); } break; case 14: -#line 2634 "upb/json/parser.rl" +#line 2635 "upb/json/parser.rl" { start_day(parser, p); } break; case 15: -#line 2635 "upb/json/parser.rl" +#line 2636 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_day(parser, p)); } break; case 16: -#line 2639 "upb/json/parser.rl" +#line 2640 "upb/json/parser.rl" { start_hour(parser, p); } break; case 17: -#line 2640 "upb/json/parser.rl" +#line 2641 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_hour(parser, p)); } break; case 18: -#line 2644 "upb/json/parser.rl" +#line 2645 "upb/json/parser.rl" { start_minute(parser, p); } break; case 19: -#line 2645 "upb/json/parser.rl" +#line 2646 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_minute(parser, p)); } break; case 20: -#line 2649 "upb/json/parser.rl" +#line 2650 "upb/json/parser.rl" { start_second(parser, p); } break; case 21: -#line 2650 "upb/json/parser.rl" +#line 2651 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_second(parser, p)); } break; case 22: -#line 2655 "upb/json/parser.rl" +#line 2656 "upb/json/parser.rl" { start_duration_base(parser, p); } break; case 23: -#line 2656 "upb/json/parser.rl" +#line 2657 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_duration_base(parser, p)); } break; case 24: -#line 2658 "upb/json/parser.rl" +#line 2659 "upb/json/parser.rl" { p--; {cs = stack[--top]; goto _again;} } break; case 25: -#line 2663 "upb/json/parser.rl" +#line 2664 "upb/json/parser.rl" { start_timestamp_base(parser); } break; case 26: -#line 2665 "upb/json/parser.rl" +#line 2666 "upb/json/parser.rl" { start_timestamp_fraction(parser, p); } break; case 27: -#line 2666 "upb/json/parser.rl" +#line 2667 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_timestamp_fraction(parser, p)); } break; case 28: -#line 2668 "upb/json/parser.rl" +#line 2669 "upb/json/parser.rl" { start_timestamp_zone(parser, p); } break; case 29: -#line 2669 "upb/json/parser.rl" +#line 2670 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_timestamp_zone(parser, p)); } break; case 30: -#line 2671 "upb/json/parser.rl" +#line 2672 "upb/json/parser.rl" { p--; {cs = stack[--top]; goto _again;} } break; case 31: -#line 2676 "upb/json/parser.rl" +#line 2677 "upb/json/parser.rl" { start_fieldmask_path_text(parser, p); } break; case 32: -#line 2677 "upb/json/parser.rl" +#line 2678 "upb/json/parser.rl" { end_fieldmask_path_text(parser, p); } break; case 33: -#line 2682 "upb/json/parser.rl" +#line 2683 "upb/json/parser.rl" { start_fieldmask_path(parser); } break; case 34: -#line 2683 "upb/json/parser.rl" +#line 2684 "upb/json/parser.rl" { end_fieldmask_path(parser); } break; case 35: -#line 2689 "upb/json/parser.rl" +#line 2690 "upb/json/parser.rl" { p--; {cs = stack[--top]; goto _again;} } break; case 36: -#line 2694 "upb/json/parser.rl" +#line 2695 "upb/json/parser.rl" { if (is_wellknown_msg(parser, UPB_WELLKNOWN_TIMESTAMP)) { {stack[top++] = cs; cs = 47;goto _again;} @@ -3090,11 +3091,11 @@ _match: } break; case 37: -#line 2707 "upb/json/parser.rl" +#line 2708 "upb/json/parser.rl" { p--; {stack[top++] = cs; cs = 78;goto _again;} } break; case 38: -#line 2712 "upb/json/parser.rl" +#line 2713 "upb/json/parser.rl" { if (is_wellknown_msg(parser, UPB_WELLKNOWN_ANY)) { start_any_member(parser, p); @@ -3104,11 +3105,11 @@ _match: } break; case 39: -#line 2719 "upb/json/parser.rl" +#line 2720 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_membername(parser)); } break; case 40: -#line 2722 "upb/json/parser.rl" +#line 2723 "upb/json/parser.rl" { if (is_wellknown_msg(parser, UPB_WELLKNOWN_ANY)) { end_any_member(parser, p); @@ -3118,7 +3119,7 @@ _match: } break; case 41: -#line 2733 "upb/json/parser.rl" +#line 2734 "upb/json/parser.rl" { if (is_wellknown_msg(parser, UPB_WELLKNOWN_ANY)) { start_any_object(parser, p); @@ -3128,7 +3129,7 @@ _match: } break; case 42: -#line 2742 "upb/json/parser.rl" +#line 2743 "upb/json/parser.rl" { if (is_wellknown_msg(parser, UPB_WELLKNOWN_ANY)) { CHECK_RETURN_TOP(end_any_object(parser, p)); @@ -3138,54 +3139,54 @@ _match: } break; case 43: -#line 2754 "upb/json/parser.rl" +#line 2755 "upb/json/parser.rl" { CHECK_RETURN_TOP(start_array(parser)); } break; case 44: -#line 2758 "upb/json/parser.rl" +#line 2759 "upb/json/parser.rl" { end_array(parser); } break; case 45: -#line 2763 "upb/json/parser.rl" +#line 2764 "upb/json/parser.rl" { CHECK_RETURN_TOP(start_number(parser, p)); } break; case 46: -#line 2764 "upb/json/parser.rl" +#line 2765 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_number(parser, p)); } break; case 47: -#line 2766 "upb/json/parser.rl" +#line 2767 "upb/json/parser.rl" { CHECK_RETURN_TOP(start_stringval(parser)); } break; case 48: -#line 2767 "upb/json/parser.rl" +#line 2768 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_stringval(parser)); } break; case 49: -#line 2769 "upb/json/parser.rl" +#line 2770 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_bool(parser, true)); } break; case 50: -#line 2771 "upb/json/parser.rl" +#line 2772 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_bool(parser, false)); } break; case 51: -#line 2773 "upb/json/parser.rl" +#line 2774 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_null(parser)); } break; case 52: -#line 2775 "upb/json/parser.rl" +#line 2776 "upb/json/parser.rl" { CHECK_RETURN_TOP(start_subobject_full(parser)); } break; case 53: -#line 2776 "upb/json/parser.rl" +#line 2777 "upb/json/parser.rl" { end_subobject_full(parser); } break; case 54: -#line 2781 "upb/json/parser.rl" +#line 2782 "upb/json/parser.rl" { p--; {cs = stack[--top]; goto _again;} } break; -#line 3189 "upb/json/parser.c" +#line 3190 "upb/json/parser.c" } } @@ -3202,32 +3203,32 @@ _again: while ( __nacts-- > 0 ) { switch ( *__acts++ ) { case 0: -#line 2590 "upb/json/parser.rl" +#line 2591 "upb/json/parser.rl" { p--; {cs = stack[--top]; if ( p == pe ) goto _test_eof; goto _again;} } break; case 46: -#line 2764 "upb/json/parser.rl" +#line 2765 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_number(parser, p)); } break; case 49: -#line 2769 "upb/json/parser.rl" +#line 2770 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_bool(parser, true)); } break; case 50: -#line 2771 "upb/json/parser.rl" +#line 2772 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_bool(parser, false)); } break; case 51: -#line 2773 "upb/json/parser.rl" +#line 2774 "upb/json/parser.rl" { CHECK_RETURN_TOP(end_null(parser)); } break; case 53: -#line 2776 "upb/json/parser.rl" +#line 2777 "upb/json/parser.rl" { end_subobject_full(parser); } break; -#line 3231 "upb/json/parser.c" +#line 3232 "upb/json/parser.c" } } } @@ -3235,10 +3236,11 @@ goto _again;} } _out: {} } -#line 2809 "upb/json/parser.rl" +#line 2810 "upb/json/parser.rl" if (p != pe) { - upb_status_seterrf(parser->status, "Parse error at '%.*s'\n", pe - p, p); + upb_status_seterrf(parser->status, "Parse error at '%.*s'\n", (int)(pe - p), + p); } else { capture_suspend(parser, &p); } @@ -3278,13 +3280,13 @@ static void json_parser_reset(upb_json_parser *p) { /* Emit Ragel initialization of the parser. */ -#line 3282 "upb/json/parser.c" +#line 3284 "upb/json/parser.c" { cs = json_start; top = 0; } -#line 2851 "upb/json/parser.rl" +#line 2853 "upb/json/parser.rl" p->current_state = cs; p->parser_top = top; accumulate_clear(p); diff --git a/upb/decode.c b/upb/decode.c index cc18d7dc17..0ca62680c3 100644 --- a/upb/decode.c +++ b/upb/decode.c @@ -150,11 +150,23 @@ static const char *decode_msg(upb_decstate *d, const char *ptr, upb_msg *msg, UPB_NORETURN static void decode_err(upb_decstate *d) { UPB_LONGJMP(d->err, 1); } +// We don't want to mark this NORETURN, see comment in .h. +// Unfortunately this code to suppress the warning doesn't appear to be working. +#ifdef __clang__ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunknown-warning-option" +#pragma clang diagnostic ignored "-Wsuggest-attribute" +#endif + const char *fastdecode_err(upb_decstate *d) { longjmp(d->err, 1); return NULL; } +#ifdef __clang__ +#pragma clang diagnostic pop +#endif + const uint8_t upb_utf8_offsets[] = { 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, @@ -652,6 +664,14 @@ const char *fastdecode_generic(struct upb_decstate *d, const char *ptr, return decode_msg(d, ptr, msg, decode_totablep(table)); } +static bool decode_top(struct upb_decstate *d, const char *buf, void *msg, + const upb_msglayout *l) { + if (!decode_tryfastdispatch(d, &buf, msg, l)) { + decode_msg(d, buf, msg, l); + } + return d->end_group == DECODE_NOGROUP; +} + bool _upb_decode(const char *buf, size_t size, void *msg, const upb_msglayout *l, upb_arena *arena, int options) { bool ok; @@ -685,10 +705,7 @@ bool _upb_decode(const char *buf, size_t size, void *msg, if (UPB_UNLIKELY(UPB_SETJMP(state.err))) { ok = false; } else { - if (!decode_tryfastdispatch(&state, &buf, msg, l)) { - decode_msg(&state, buf, msg, l); - } - ok = state.end_group == DECODE_NOGROUP; + ok = decode_top(&state, buf, msg, l); } arena->head.ptr = state.arena.head.ptr; diff --git a/upb/decode.int.h b/upb/decode.int.h index e286b9cdd5..7376e21df6 100644 --- a/upb/decode.int.h +++ b/upb/decode.int.h @@ -14,7 +14,7 @@ /* Must be last. */ #include "upb/port_def.inc" -#define DECODE_NOGROUP -1 +#define DECODE_NOGROUP (uint32_t)-1 typedef struct upb_decstate { const char *end; /* Can read up to 16 bytes slop beyond this. */ diff --git a/upb/def.c b/upb/def.c index cdb5a5e5bd..8d5e502ed1 100644 --- a/upb/def.c +++ b/upb/def.c @@ -929,7 +929,7 @@ typedef struct { jmp_buf err; /* longjmp() on error. */ } symtab_addctx; -UPB_NORETURN UPB_NOINLINE +UPB_NORETURN UPB_NOINLINE UPB_PRINTF(2, 3) static void symtab_errf(symtab_addctx *ctx, const char *fmt, ...) { va_list argp; va_start(argp, fmt); @@ -1543,7 +1543,7 @@ static void parse_default(symtab_addctx *ctx, const char *str, size_t len, return; invalid: - symtab_errf(ctx, "Invalid default '%.*s' for field %f", (int)len, str, + symtab_errf(ctx, "Invalid default '%.*s' for field %s", (int)len, str, upb_fielddef_fullname(f)); } diff --git a/upb/json/parser.rl b/upb/json/parser.rl index a7d75ff158..e6a701a542 100644 --- a/upb/json/parser.rl +++ b/upb/json/parser.rl @@ -1380,7 +1380,8 @@ static bool end_stringval_nontop(upb_json_parser *p) { ok = true; /* TODO(teboring): Should also clean this field. */ } else { - upb_status_seterrf(p->status, "Enum value unknown: '%.*s'", len, buf); + upb_status_seterrf(p->status, "Enum value unknown: '%.*s'", (int)len, + buf); } } @@ -2808,7 +2809,8 @@ size_t parse(void *closure, const void *hd, const char *buf, size_t size, %% write exec; if (p != pe) { - upb_status_seterrf(parser->status, "Parse error at '%.*s'\n", pe - p, p); + upb_status_seterrf(parser->status, "Parse error at '%.*s'\n", (int)(pe - p), + p); } else { capture_suspend(parser, &p); } diff --git a/upb/json_decode.c b/upb/json_decode.c index bb33744324..760f96b671 100644 --- a/upb/json_decode.c +++ b/upb/json_decode.c @@ -62,6 +62,7 @@ UPB_NORETURN static void jsondec_err(jsondec *d, const char *msg) { UPB_LONGJMP(d->err, 1); } +UPB_PRINTF(2, 3) UPB_NORETURN static void jsondec_errf(jsondec *d, const char *fmt, ...) { va_list argp; upb_status_seterrf(d->status, "Error parsing JSON @%d:%d: ", d->line, @@ -678,7 +679,7 @@ static upb_msgval jsondec_int(jsondec *d, const upb_fielddef *f) { } val.int64_val = dbl; /* must be guarded, overflow here is UB */ if (val.int64_val != dbl) { - jsondec_errf(d, "JSON number was not integral (%d != %" PRId64 ")", dbl, + jsondec_errf(d, "JSON number was not integral (%f != %" PRId64 ")", dbl, val.int64_val); } break; @@ -704,7 +705,7 @@ static upb_msgval jsondec_int(jsondec *d, const upb_fielddef *f) { /* Parse UINT32 or UINT64 value. */ static upb_msgval jsondec_uint(jsondec *d, const upb_fielddef *f) { - upb_msgval val; + upb_msgval val = {0}; switch (jsondec_peek(d)) { case JD_NUMBER: { @@ -714,7 +715,7 @@ static upb_msgval jsondec_uint(jsondec *d, const upb_fielddef *f) { } val.uint64_val = dbl; /* must be guarded, overflow here is UB */ if (val.uint64_val != dbl) { - jsondec_errf(d, "JSON number was not integral (%d != %" PRIu64 ")", dbl, + jsondec_errf(d, "JSON number was not integral (%f != %" PRIu64 ")", dbl, val.uint64_val); } break; @@ -741,7 +742,7 @@ static upb_msgval jsondec_uint(jsondec *d, const upb_fielddef *f) { /* Parse DOUBLE or FLOAT value. */ static upb_msgval jsondec_double(jsondec *d, const upb_fielddef *f) { upb_strview str; - upb_msgval val; + upb_msgval val = {0}; switch (jsondec_peek(d)) { case JD_NUMBER: diff --git a/upb/json_encode.c b/upb/json_encode.c index 9217c65b25..7bdda517ac 100644 --- a/upb/json_encode.c +++ b/upb/json_encode.c @@ -40,6 +40,7 @@ UPB_NORETURN static void jsonenc_err(jsonenc *e, const char *msg) { longjmp(e->err, 1); } +UPB_PRINTF(2, 3) UPB_NORETURN static void jsonenc_errf(jsonenc *e, const char *fmt, ...) { va_list argp; va_start(argp, fmt); @@ -72,6 +73,7 @@ static void jsonenc_putstr(jsonenc *e, const char *str) { jsonenc_putbytes(e, str, strlen(str)); } +UPB_PRINTF(2, 3) static void jsonenc_printf(jsonenc *e, const char *fmt, ...) { size_t n; size_t have = e->end - e->ptr; @@ -102,7 +104,7 @@ static void jsonenc_nanos(jsonenc *e, int32_t nanos) { digits -= 3; } - jsonenc_printf(e, ".%0.*" PRId32, digits, nanos); + jsonenc_printf(e, ".%.*" PRId32, digits, nanos); } static void jsonenc_timestamp(jsonenc *e, const upb_msg *msg, diff --git a/upb/port_def.inc b/upb/port_def.inc index 2cd1bb6985..8e243a9d6a 100644 --- a/upb/port_def.inc +++ b/upb/port_def.inc @@ -80,14 +80,17 @@ #define UPB_FORCEINLINE __inline__ __attribute__((always_inline)) #define UPB_NOINLINE __attribute__((noinline)) #define UPB_NORETURN __attribute__((__noreturn__)) +#define UPB_PRINTF(str, first_vararg) __attribute__((format (printf, str, first_vararg))) #elif defined(_MSC_VER) #define UPB_NOINLINE #define UPB_FORCEINLINE #define UPB_NORETURN __declspec(noreturn) +#define UPB_PRINTF(str, first_vararg) #else /* !defined(__GNUC__) */ #define UPB_FORCEINLINE #define UPB_NOINLINE #define UPB_NORETURN +#define UPB_PRINTF(str, first_vararg) #endif #define UPB_MAX(x, y) ((x) > (y) ? (x) : (y)) diff --git a/upb/upb.h b/upb/upb.h index 11c0e4dc99..6c54e87b27 100644 --- a/upb/upb.h +++ b/upb/upb.h @@ -33,9 +33,12 @@ bool upb_ok(const upb_status *status); /* These are no-op if |status| is NULL. */ void upb_status_clear(upb_status *status); void upb_status_seterrmsg(upb_status *status, const char *msg); -void upb_status_seterrf(upb_status *status, const char *fmt, ...); -void upb_status_vseterrf(upb_status *status, const char *fmt, va_list args); -void upb_status_vappenderrf(upb_status *status, const char *fmt, va_list args); +void upb_status_seterrf(upb_status *status, const char *fmt, ...) + UPB_PRINTF(2, 3); +void upb_status_vseterrf(upb_status *status, const char *fmt, va_list args) + UPB_PRINTF(2, 0); +void upb_status_vappenderrf(upb_status *status, const char *fmt, va_list args) + UPB_PRINTF(2, 0); /** upb_strview ************************************************************/