Skip to content

[InstallAPI] Cleanup I/O error handling for input lists #90664

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

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

cyndyishida
Copy link
Member

Add validation in the FileList reader to check that the headers exist and use similar diagnostics in Options.cpp

Add validation in FileList reader to actually check that the files
exist and refactor common code in Options.cpp
@cyndyishida cyndyishida added the skip-precommit-approval PR for CI feedback, not intended for review label Apr 30, 2024
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 30, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 30, 2024

@llvm/pr-subscribers-clang

Author: Cyndy Ishida (cyndyishida)

Changes

Add validation in the FileList reader to check that the headers exist and use similar diagnostics in Options.cpp


Full diff: https://github.com/llvm/llvm-project/pull/90664.diff

4 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticInstallAPIKinds.td (+1-1)
  • (modified) clang/include/clang/InstallAPI/FileList.h (+2-1)
  • (modified) clang/lib/InstallAPI/FileList.cpp (+9-1)
  • (modified) clang/tools/clang-installapi/Options.cpp (+5-4)
diff --git a/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td b/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td
index 91a40cd589b385..6896e0f5aa593c 100644
--- a/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td
+++ b/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td
@@ -24,7 +24,7 @@ def err_no_matching_target : Error<"no matching target found for target variant
 def err_unsupported_vendor : Error<"vendor '%0' is not supported: '%1'">;
 def err_unsupported_environment : Error<"environment '%0' is not supported: '%1'">;
 def err_unsupported_os : Error<"os '%0' is not supported: '%1'">;
-def err_cannot_read_alias_list : Error<"could not read alias list '%0': %1">;
+def err_cannot_read_input_list : Error<"could not read %select{alias list|filelist}0 '%1': %2">;
 } // end of command line category.
 
 let CategoryName = "Verification" in {
diff --git a/clang/include/clang/InstallAPI/FileList.h b/clang/include/clang/InstallAPI/FileList.h
index 460af003b6a0ac..913734b8dc7c88 100644
--- a/clang/include/clang/InstallAPI/FileList.h
+++ b/clang/include/clang/InstallAPI/FileList.h
@@ -29,9 +29,10 @@ class FileListReader {
   ///
   /// \param InputBuffer JSON input data.
   /// \param Destination Container to load headers into.
+  /// \param FM Optional File Manager to validate input files exist.
   static llvm::Error
   loadHeaders(std::unique_ptr<llvm::MemoryBuffer> InputBuffer,
-              HeaderSeq &Destination);
+              HeaderSeq &Destination, clang::FileManager *FM = nullptr);
 
   FileListReader() = delete;
 };
diff --git a/clang/lib/InstallAPI/FileList.cpp b/clang/lib/InstallAPI/FileList.cpp
index 8a01248659b7d7..65610903840af7 100644
--- a/clang/lib/InstallAPI/FileList.cpp
+++ b/clang/lib/InstallAPI/FileList.cpp
@@ -51,6 +51,7 @@ class Implementation {
 
 public:
   std::unique_ptr<MemoryBuffer> InputBuffer;
+  clang::FileManager *FM;
   unsigned Version;
   HeaderSeq HeaderList;
 
@@ -124,6 +125,12 @@ Error Implementation::parseHeaders(Array &Headers) {
           HeaderFile{PathStr, *Type, /*IncludeName=*/"", Language});
       continue;
     }
+
+    if (FM)
+      if (!FM->getOptionalFileRef(PathStr))
+        return createFileError(
+            PathStr, make_error_code(std::errc::no_such_file_or_directory));
+
     auto IncludeName = createIncludeHeaderName(PathStr);
     HeaderList.emplace_back(PathStr, *Type,
                             IncludeName.has_value() ? IncludeName.value() : "",
@@ -170,9 +177,10 @@ Error Implementation::parse(StringRef Input) {
 
 llvm::Error
 FileListReader::loadHeaders(std::unique_ptr<MemoryBuffer> InputBuffer,
-                            HeaderSeq &Destination) {
+                            HeaderSeq &Destination, clang::FileManager *FM) {
   Implementation Impl;
   Impl.InputBuffer = std::move(InputBuffer);
+  Impl.FM = FM;
 
   if (llvm::Error Err = Impl.parse(Impl.InputBuffer->getBuffer()))
     return Err;
diff --git a/clang/tools/clang-installapi/Options.cpp b/clang/tools/clang-installapi/Options.cpp
index ae5b697b8eb9e0..21f04a291b2f1f 100644
--- a/clang/tools/clang-installapi/Options.cpp
+++ b/clang/tools/clang-installapi/Options.cpp
@@ -697,8 +697,8 @@ InstallAPIContext Options::createContext() {
     }
     Expected<AliasMap> Result = parseAliasList(Buffer.get());
     if (!Result) {
-      Diags->Report(diag::err_cannot_read_alias_list)
-          << ListPath << toString(Result.takeError());
+      Diags->Report(diag::err_cannot_read_input_list)
+          << /*IsFileList=*/false << ListPath << toString(Result.takeError());
       return Ctx;
     }
     Aliases.insert(Result.get().begin(), Result.get().end());
@@ -717,8 +717,9 @@ InstallAPIContext Options::createContext() {
       return Ctx;
     }
     if (auto Err = FileListReader::loadHeaders(std::move(Buffer.get()),
-                                               Ctx.InputHeaders)) {
-      Diags->Report(diag::err_cannot_open_file) << ListPath << std::move(Err);
+                                               Ctx.InputHeaders, FM)) {
+      Diags->Report(diag::err_cannot_read_input_list)
+          << /*IsFileList=*/true << ListPath << std::move(Err);
       return Ctx;
     }
   }

@cyndyishida cyndyishida merged commit 278774e into llvm:main Apr 30, 2024
@cyndyishida cyndyishida deleted the eng/PR-installAPICleanup branch April 30, 2024 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category skip-precommit-approval PR for CI feedback, not intended for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants