Failure rewriting static function declaration in macro

[john-h-kastner]

This error showed up in bc last night.

 /home/github/actions-runner/_work/actions/actions/b/ninja/bin/clang -g -O3 -w -D_ISOC99_SOURCE -c util.c
util.c:77:14: error: conflicting types for 'make_arg_str'
static char *make_arg_str(_Ptr<arg_list> args, int len, int commas) : itype(_Ptr<char>)
             ^
util.c:75:27: note: previous declaration is here
_PROTOTYPE (static char * make_arg_str, (arg_list * args, int len, int commas));
                          ^
util.c:84:26: error: passing '_Ptr<arg_list>' (aka '_Ptr<struct arg_list>') to parameter of incompatible type 'arg_list *' (aka 'struct arg_list *')
    temp = make_arg_str (_Assume_bounds_cast<_Ptr<arg_list>>(args->next), len+11, commas);
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
util.c:75:53: note: passing argument to parameter 'args' here
_PROTOTYPE (static char * make_arg_str, (arg_list * args, int len, int commas));
                                                    ^
util.c:106:28: error: passing '_Ptr<arg_list>' (aka '_Ptr<struct arg_list>') to parameter of incompatible type 'arg_list *' (aka 'struct arg_list *')
  arglist1 = make_arg_str (args, 1, commas);
                           ^~~~
util.c:75:53: note: passing argument to parameter 'args' here
_PROTOTYPE (static char * make_arg_str, (arg_list * args, int len, int commas));
                                                    ^
3 errors generated.

---

Reduced example:

#define _PROTOTYPE(func, args) func args
_PROTOTYPE (static void make_arg_str, (int * args));
static void make_arg_str (int * args) { }

To replicate you'll need to place this in a directory with the following compilation database (adjusted for your paths).

[{
  "arguments": [ "clang", "util.c" ],
  "directory": "/home/john/bc_test",
  "file": "util.c"
}]

When you 3c util.c you should see the definition of make_arg_str rewrite correctly while the prototype in the macro is unchanged. If you ignore the compilation database with 3c util.c -- conversion is correct.

[mattmccutchen-cci]

I'll at least try to find out why the behavior is different depending on the use of a compilation database, since I have some knowledge of LibTooling.

[mattmccutchen-cci]

@john-h-kastner I think I followed your directions for the reduced example (with the compilation database) and I can't reproduce the problem: I get the same output as without the compilation database. But if I run the workflow commands for bc locally, I can reproduce the problem. Do you have any ideas before I start the reduction over in my environment?

Never mind, I had the wrong filename in the compilation database. Your example works for me now.

[mattmccutchen-cci]

Several things come together to cause this problem:

1. When we use a compilation database, LibTooling uses the input file path as it is written in the compilation database (which is relative in the case of bc and the reduced example in this issue), rather than the canonical path 3C provides to LibTooling. This input file path then shows up in presumed locations.
2. Currently, PersistentSourceLoc uses the file path from SM.getFileEntryForID(TFSL.getFileID()) (which is always absolute) if it is available, otherwise it falls back to the presumed file path. And apparently the SM.getFileEntryForID(TFSL.getFileID()) works in the case without the macro but fails in the case with the macro in this example (I don't know if it depends on exactly how the macro is used).
3. 3C determines whether two static function declarations/definitions are in the same translation unit based on the file path in the PresumedSourceLoc.

Thus, when we use a compilation database, 3C thinks the declarations are in different translation units and the body of the second is not associated with the first, so the first has no body and shouldn't be converted. Given the timing of the breakage and the involvement of declaration merging, we can presume that the problem started with #505, but I haven't looked into exactly what #505 changed that introduced the problem.

I was already considering changing PersistentSourceLoc to always use the presumed location (not SM.getFileEntryForID(TFSL.getFileID())) in order to make 3C work correctly on preprocessed code, which would fix the immediate problem here. But if the presumed file path is relative, in general, that could cause ambiguity when different translation units have different working directories, so I think the better solution is to ensure that the path is absolute. And for the main input file, we can achieve that by using an argument adjuster to override the input file path from the compilation database and make it absolute again. (There is a similar problem with files included via relative -I paths, but the workaround that convert_project already has for #515 should help with this too.) I've implemented that in #532, which just needs a regression test.