From 22d028dab0e96eba0892058edcfdc3b7e3d34689 Mon Sep 17 00:00:00 2001 From: Yonatan Goldschmidt Date: Thu, 16 Apr 2020 01:39:01 +0300 Subject: [PATCH] Fix crash with -Wall on new GCC build_function_call() passes improper arguments to some further function, leading to a crash when -Wall is enabled. I sent a question about it to the GCC mailing lists, can read further here: https://gcc.gnu.org/pipermail/gcc/2020-April/232127.html --- Makefile | 2 +- plugin.c | 39 ++++++++++++++++++++++++++++-------- tests/test_assert_rewrite.py | 3 --- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 046a6f2..b7194f9 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,7 @@ $(PLUGIN): plugin.c $(GXX) -g -Wall -Werror -I`$(GCC) -print-file-name=plugin`/include -fpic -shared -o $@ $< plugin_test.o: plugin_test.c $(PLUGIN) - $(GCC) -O2 -fplugin=./$(PLUGIN) plugin_test.c -c + $(GCC) -Wall -Werror -O2 -fplugin=./$(PLUGIN) plugin_test.c -c tester: tester.c plugin_test.o $(GCC) $^ -o $@ diff --git a/plugin.c b/plugin.c index 0d78b68..e23b591 100644 --- a/plugin.c +++ b/plugin.c @@ -43,6 +43,29 @@ static tree printf_decl = NULL_TREE; static tree abort_decl = NULL_TREE; static tree sprintf_decl = NULL_TREE; +// based on build_function_call(), with some fixes. +// see https://gcc.gnu.org/pipermail/gcc/2020-April/232127.html for explanation. +tree my_build_function_call(location_t loc, tree function, tree params) +{ + const int len = list_length(params); + + vec *v; + auto_vec argloc(len); + tree ret; + + vec_alloc(v, len); + for (; params; params = TREE_CHAIN(params)) { + tree param = TREE_VALUE(params); + v->quick_push(param); + argloc.quick_push(EXPR_HAS_LOCATION(param) ? EXPR_LOCATION(param) : loc); + } + + ret = c_build_function_call_vec(loc, argloc, function, v, NULL); + vec_free(v); + + return ret; +} + // heuristic to check if cond_expr is the expression generated by glibc's "assert" macro, which is: // ((void) sizeof ((EXPR) ? 1 : 0), __extension__ ({ if (EXPR) ; else __assert_fail ("EXPR", "file.c", line, __extension__ __PRETTY_FUNCTION__); })) // the generated cond_expr has COND_EXPR_COND as the assert's condition, @@ -153,12 +176,12 @@ static void make_assert_expr_printf(location_t here, tree call__assert_fail, tre TREE_STRING_POINTER(file_arg), TREE_INT_CST_LOW(line_arg)); tree header_line = build_string_literal(strlen(buf) + 1, buf); - append_to_statement_list(build_function_call(here, printf_decl, + append_to_statement_list(my_build_function_call(here, printf_decl, tree_cons(NULL_TREE, header_line, tree_cons(NULL_TREE, function_arg, NULL_TREE))), stmts); tree format_str = build_string_literal_from_literal("> assert(%s)\n"); - append_to_statement_list(build_function_call(here, printf_decl, + append_to_statement_list(my_build_function_call(here, printf_decl, tree_cons(NULL_TREE, format_str, // can just use the original argument directly in our call.. tree_cons(NULL_TREE, CALL_EXPR_ARG(call__assert_fail, 0), NULL_TREE))), stmts); @@ -326,7 +349,7 @@ static void make_decl_subexpressions_repr(location_t here, struct expr_list *lis (void)snprintf(buf, sizeof(buf), " %s = %s\n", get_format_for_expr(expr), IDENTIFIER_POINTER(DECL_NAME(raw_expr))); - tree printf_call = build_function_call(here, printf_decl, + tree printf_call = my_build_function_call(here, printf_decl, tree_cons(NULL_TREE, build_string_literal(strlen(buf) + 1, buf), tree_cons(NULL_TREE, expr, NULL_TREE))); @@ -344,7 +367,7 @@ static tree make_buf_ref_addr(location_t here, tree buf_param, tree buf_pos) { // creats a `buf_pos += sprintf(...)` with given arguments. static tree make_repr_sprintf(location_t here, tree buf_param, tree buf_pos, const char *format, tree args) { - tree sprintf_call = build_function_call(here, sprintf_decl, + tree sprintf_call = my_build_function_call(here, sprintf_decl, tree_cons(NULL_TREE, make_buf_ref_addr(here, buf_param, buf_pos), tree_cons(NULL_TREE, build_string_literal(strlen(format) + 1, format), args))); @@ -565,12 +588,12 @@ static tree make_assert_failed_body(location_t here, tree cond_expr) { append_to_statement_list(make_conditional_expr_repr(¶ms, COND_EXPR_COND(cond_expr)), &stmts); // print the repr buf now - tree printf_call = build_function_call(here, printf_decl, + tree printf_call = my_build_function_call(here, printf_decl, tree_cons(NULL_TREE, build_string_literal_from_literal(" assert(%s)\n"), tree_cons(NULL_TREE, buf_param, NULL_TREE))); append_to_statement_list(printf_call, &stmts); - append_to_statement_list(build_function_call(here, printf_decl, + append_to_statement_list(my_build_function_call(here, printf_decl, tree_cons(NULL_TREE, build_string_literal_from_literal("> subexpressions:\n"), NULL_TREE)), &stmts); @@ -578,13 +601,13 @@ static tree make_assert_failed_body(location_t here, tree cond_expr) { make_decl_subexpressions_repr(here, &repr_exprs, &stmts); // print call buf repr - printf_call = build_function_call(here, printf_decl, + printf_call = my_build_function_call(here, printf_decl, tree_cons(NULL_TREE, build_string_literal_from_literal("%s"), tree_cons(NULL_TREE, call_buf_param, NULL_TREE))); append_to_statement_list(printf_call, &stmts); // finally, an abort call - tree abort_call = build_function_call(here, abort_decl, NULL_TREE); + tree abort_call = my_build_function_call(here, abort_decl, NULL_TREE); append_to_statement_list(abort_call, &stmts); return c_build_bind_expr(here, block, stmts); diff --git a/tests/test_assert_rewrite.py b/tests/test_assert_rewrite.py index 8035b8b..a7f8164 100644 --- a/tests/test_assert_rewrite.py +++ b/tests/test_assert_rewrite.py @@ -23,9 +23,6 @@ def run_tester(test_prototype, test_code, calling_code, *, extra_test="", test.write(HEADERS + extra_test + "{0} {{ {1} }}".format(test_prototype, test_code)) test.flush() extra_opts = ["-Werror", "-Wall", ] - if b"9.1.0" in subprocess.check_output([GCC, "--version"]): - # TODO -Werror kills GCC 9.1.0 if the plugin is active - extra_opts = [] subprocess.check_call([GCC, "-fplugin={}".format(ASSERT_INTROSPECT_SO), "-c", test.name, "-o", obj.name] + extra_opts)