[Bug plugins/94664] New: build_function_call() on functions with __attribute__((format (printf, ...))) may lead to ICE: SIGSEGV

[yon.goldschmidt at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94664

            Bug ID: 94664
           Summary: build_function_call() on functions with
                    __attribute__((format (printf, ...))) may lead to ICE:
                    SIGSEGV
           Product: gcc
           Version: 9.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: plugins
          Assignee: unassigned at gcc dot gnu.org
          Reporter: yon.goldschmidt at gmail dot com
  Target Milestone: ---

I encountered an ICE while developing a plugin. Started by sending a question
about it to g...@gnu.org [1], but I didn't get any reply and I'm almost certain
it's a bug, so I went ahead to create a bug report. I'll give a short
description here, the full story can be found in the mailing list.

Constructing function calls with build_function_call() uses vNULL for the
"argument locations" vector.
When compiling with "-Wformat" enabled, and calling functions marked with
"__attribute__((format(printf, ...)))" we reach check_format_types() in
c-format.c, which contains this snippet:

      if (EXPR_HAS_LOCATION (cur_param))
        param_loc = EXPR_LOCATION (cur_param);
      else if (arglocs)
        {
          /* arg_num is 1-based.  */
          gcc_assert (types->arg_num > 0);
          param_loc = (*arglocs)[types->arg_num - 1];
        }

arglocs here is 'vec<location_t> *', that is, pointing to the vNULL in our
case. If any of the parameters doesn't have an attached location (e.g
INTEGER_CST doesn't have), arglocs is dereferenced, then the vector is
accessed. The access is invalid since it's vNULL, and GCC dies with an ICE.

In my plugin code I've avoided this problem by using
c_build_function_call_vec() directly with a non-vNULL arg_loc vector, you can
see my workaround in [2].

This API is used only by objc code in GCC itself, but as far as I understand
it, it's the main API to be used by plugin code (has the most concrete name and
the simplest signature relating to "calling functions") - so wouldn't expect it
to crash :)

I compiled GCC 9.2.0 with the following patch (can also be applied on master)

3126c3126
<       else if (arglocs)
---
>       else if (arglocs && *arglocs != vNULL)

It solved the problem. Not sure that's the correct fix; Another fix may be
to pass "arg_locs" as 'vec<location_t> *' starting from build_function_call(),
then the check "if (arglocs)" suffices.

Reference plugin exhibiting the error, taken from the mail I sent (tested with
GCC 9.2.0 + 9.1.0):
```
$ cat bug.c
#include <stdio.h>

#include <gcc-plugin.h>
#include <tree.h>
#include <c-family/c-common.h>

int plugin_is_GPL_compatible;

static void finish_decl_callback(void *event_data, void *user_data) {
    tree decl = (tree)event_data;

    if (TREE_CODE(decl) == FUNCTION_DECL && 0 == strcmp("printf",
IDENTIFIER_POINTER(DECL_NAME(decl)))) {

        build_function_call(BUILTINS_LOCATION, decl,
            tree_cons(NULL_TREE, build_string_literal(4, "%d\n"),
            tree_cons(NULL_TREE, integer_zero_node, NULL_TREE)));
    }
}

int plugin_init(struct plugin_name_args *plugin_info,
struct plugin_gcc_version *version) {
    register_callback(plugin_info->base_name, PLUGIN_FINISH_DECL,
finish_decl_callback, NULL);

    return 0;
}

$ cat trigger.c
#include <stdio.h>

$ g++ -g -Wall -Werror -I`gcc -print-file-name=plugin`/include -fpic -shared -o
bug.so bug.c
$ gcc -fplugin=./bug.so -c trigger.c && echo okay
okay # no crash!
$ gcc -Wformat -fplugin=./bug.so -c trigger.c
...
internal compiler error: Segmentation fault
...
```

Is my fix acceptable?

Thanks,
Yonatan

[1]: https://gcc.gnu.org/pipermail/gcc/2020-April/232127.html
[2]:
https://github.com/Jongy/gcc_assert_introspect/commit/22d028dab0e96eba0892058edcfdc3b7e3d34689