Skip to content

Commit

Permalink
Fix crash with -Wall on new GCC
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Jongy committed Apr 15, 2020
1 parent 0633d83 commit 22d028d
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 12 deletions.
2 changes: 1 addition & 1 deletion Makefile
Expand Up @@ -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 $@
Expand Down
39 changes: 31 additions & 8 deletions plugin.c
Expand Up @@ -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<tree, va_gc> *v;
auto_vec<location_t> 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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)));

Expand All @@ -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)));

Expand Down Expand Up @@ -565,26 +588,26 @@ static tree make_assert_failed_body(location_t here, tree cond_expr) {
append_to_statement_list(make_conditional_expr_repr(&params, 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);

// add DECLs subexpressions repr
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);
Expand Down
3 changes: 0 additions & 3 deletions tests/test_assert_rewrite.py
Expand Up @@ -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)
Expand Down

0 comments on commit 22d028d

Please sign in to comment.