Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interaction of error and -- param produces different results in output of 3c #343

Closed
kyleheadley opened this issue Dec 3, 2020 · 3 comments · Fixed by #412
Closed

Interaction of error and -- param produces different results in output of 3c #343

kyleheadley opened this issue Dec 3, 2020 · 3 comments · Fixed by #412
Assignees
Labels
bug Something isn't working command-line testing

Comments

@kyleheadley
Copy link
Member

When running 3c from the top directory of the clang project, I get the usual message about a missing compile_commands, and otherwise the following produces the expected output build/bin/3c clang/test/3C/allocator.c > outfile

However, running the equivalent command from the build directory, ie bin/3c ../clang/test/3C/allocator.c > outfile produces an non-rewritten "foo" function along with some errors starting with:

warning: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated [-Wdeprecated]
/Users/kyleheadley/Documents/Projects/3C/checkedc-testing/build/../clang/test/3C/allocator.c:13:1: error: 
      unknown type name '_Itype_for_any'
_Itype_for_any(T) void *malloc(size_t size) : itype(_Array_ptr<T>) byte_...
^

This could be a configuration issue on my machine, or could be a side affect of the compile_commands file in that directory.

The strange part is that when adding the dashes, i.e. bin/3c ../clang/test/3C/allocator.c -- > outfile all console messages are suppressed and the output is the correctly rewritten result.

So -- is either incorrectly suppressing an error (an undefined malloc should be marked WILD), or is selecting a different project configuration (choosing a malloc that is defined). Or something else. Either way this seems to be undesirable.

@kyleheadley
Copy link
Member Author

After a few more encounters with this bug, it seems that my system is running 3c in C++ mode, as mentioned in the error. The -- prevents this. I'm guessing that it's triggered by code that appears to be C++, like the additional features of checkedc over c. It shows up on code that's been at least partially converted.

@mattmccutchen-cci
Copy link
Member

I think I figured out what is going on here. A little-known feature of LibTooling (on which 3c is based) is that if the command line includes a --, then no compilation database is loaded and any arguments after the -- are taken as compiler options for every source file. If there is no --, then LibTooling tries to find a compilation database in an ancestor of the -p directory, or if -p is not given, an ancestor directory of the first source file.

LibTooling computes the absolute path of the first source file by prepending the working directory. In the first case, this is /PATH/TO/YOUR/WORKING/TREE/clang/test/3C/allocator.c, and none of the ancestor directories contains a compile_commands.json file. In the second case, the path is /PATH/TO/YOUR/WORKING/TREE/build/../clang/test/3C/allocator.c, so LibTooling walks up to the "ancestor" directory /PATH/TO/YOUR/WORKING/TREE/build (arguably a bug, like #327) and finds /PATH/TO/YOUR/WORKING/TREE/build/compile_commands.json. Since the input file ../clang/test/3C/allocator.c is not listed in the compilation database, LibTooling chooses a file in the compilation database according to some heuristics and uses its compiler options. Apparently it is choosing a C++ file (no surprise because most of Clang is written in C++) and getting compiler options that cause the 3C input file to be treated as C++. The Checked C parser extensions are not active in C++ mode, so we get parse errors.

My conclusion: If we want to run 3c without a compilation database, we should pass --. This will ensure that LibTooling doesn't automatically detect a compilation database we don't want to use and, as a bonus, suppress the error about not finding a compilation database. Also, anything that is currently running 3c -extra-arg=-foo ... without a compilation database should be able to run 3c ... -- -foo instead. Even though the .. bug that affected Kyle's particular case might be fixed in a newer version of Clang, it's good to get in the habit of using -- whenever we don't want to use a compilation database in order to avoid surprises. For example, I am converting the 3C regression tests that currently use -output-postfix to write to the lit temporary directory instead (#412), and that directory is under the build directory, so when the output files are passed back into 3C for idempotence tests, I get the same problem unless I use --.

Are we OK with keeping LibTooling's "standard" behavior (but documenting it better for our users in the 3c -help output), or do we want to try to override the command line processing in 3c?

@mattmccutchen-cci
Copy link
Member

We decided to keep the standard use of -- and document it in the 3c -help output. I will do that in #412.

mattmccutchen-cci added a commit that referenced this issue Feb 13, 2021
- Add -output-dir option to write updated files to a directory structure
  parallel to the base dir (#347). When -output-dir is used, a source
  file outside the base dir can't be handled because there is no way to
  compute its output path. For consistency, this is now an error even
  when -output-dir is not used.

- Convert all 3C regression tests from -output-postfix to -output-dir to
  avoid leaving temporary files in the clang/test/3C directory (#378).

- Expand "3c -help" documentation. In particular, direct the user to pass
  "--" when they don't want to use a compilation database to avoid
  accidentally using unwanted compiler options and suppress the warning
  if no compilation database is found (#343).

- For consistency, have stdout mode output the main file even if it is
  unchanged (#328).

- Fix bugs in matching of file paths against the base dir (#327).

- Other minor bug fixes: see the pull request description for details.

Co-authored-by: John Kastner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working command-line testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants