Page MenuHomePhabricator

[flang][driver] Disallow non-existent input files in the frontend driver
ClosedPublic

Authored by awarzynski on Jan 21 2021, 4:28 AM.

Details

Summary

This patch adds a check that verifies that the input file used when
calling the frontend driver (i.e. flang-new -fc1) actually exists.
This was not required for the compiler driver, flang-new, as that's
already handled in libclangDriver.

Once all input/output file management is moved to the driver, we should
also check that for input from stdin the corresponding file descriptor
was successfully acquired.

This patch also makes sure that the default action in the frontend is
ParseSyntaxOnly. This is consistent with Clang. Before this change
flang-new -fc1 would do nothing, which makes testing changes like the
one introduced here a bit tricky.

Diff Detail

Event Timeline

awarzynski created this revision.Jan 21 2021, 4:28 AM
awarzynski requested review of this revision.Jan 21 2021, 4:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 21 2021, 4:28 AM
tskeith added inline comments.
flang/lib/Frontend/FrontendAction.cpp
40

What happens if the argument exists but is not a file?

42

It looks like this only says "error reading <file>". It would be nice to say what the error is.

flang/test/Flang-Driver/missing-input.f90
13

NOEXISITANT -> NONEXISTENT ?

Update diagnostic message and add a check for directories

awarzynski marked 2 inline comments as done.Jan 25 2021, 7:42 AM

@tskeith Thank for you reviewing! I think that this covers all of your suggestions.

flang/lib/Frontend/FrontendAction.cpp
40

Updated - this will now make sure that the input:

  • exists
  • is not a directory

If any of these ^^^ is not true, a diagnostic is generated (different for each condition).

tskeith added inline comments.Jan 25 2021, 7:53 AM
flang/lib/Frontend/FrontendAction.cpp
37

The input may be a regular file but still not readable, e.g. because of permissions. Should that be caught here too for consistency?

Why is this checking needed at all? The front end will report an error if it can't read the file.

flang/test/Flang-Driver/missing-input.f90
14

NONEXISITENT -> NONEXISTENT ?

awarzynski added inline comments.Jan 25 2021, 8:13 AM
flang/lib/Frontend/FrontendAction.cpp
37

The input may be a regular file but still not readable, e.g. because of permissions. Should that be caught here too for consistency?

Initially I wanted to keep this simple and catch the most obvious things.

Why is this checking needed at all? The front end will report an error if it can't read the file.

The long term goal is for the driver makes sure that the frontend is not entered before all input is validated. This includes managing input files. This is a small step in that direction.

Fix typo in test

SouraVX accepted this revision.Jan 28 2021, 3:30 AM
SouraVX added a subscriber: SouraVX.

LGTM! Please wait for @sameeranjoshi to have a look.

This revision is now accepted and ready to land.Jan 28 2021, 3:30 AM
This revision was landed with ongoing or failed builds.Tue, Feb 2, 2:05 AM
This revision was automatically updated to reflect the committed changes.

@sameeranjoshi @SouraVX This is now merged so that I can focus on other patches. But if you have further comments, I'll happily address them!

Thank you for reviewing!