[modules] Diagnose insufficient privileges when trying to load the modulemap

[davide]

Summary

I stumbled upon this yesterday. Without this patch in palce, the module map load is silently ignored, and this might cause subtle breakages.

[davide]

Typo.

[silvas]

Isn't there a read call or something that fails down the road? How can it be silent?

[davide]

Hmm, I think you're right. It's not actually that silent (it fails with a fatal error when trying to load/read).
That said, I think it might still be valuable to emit a diagnostic -- Richard what do you think?

I noticed you reported a very similar problem here: https://llvm.org/bugs/show_bug.cgi?id=20468

Also, I looked at the code more closely and I think the right place to diagnose this would be lib/Frontend/FrontendAction.cpp, in BeginSourceFile, around line 390, and not where I'm diagnosing it right now. Thoughts?

[rsmith]

I would prefer that we solve this problem generically for all files that Clang tries to open, not just for module map files. (Note, for instance, that PR20468 concerns files referenced by module map files, such as their nominated headers, not the module map files themselves.)

lib/Frontend/FrontendActions.cpp

269

Why don't we fail up here when trying to open the file for read?

include/clang/Basic/DiagnosticFrontendKinds.td

 def note_incompatible_analyzer_plugin_api : Note<
     "current API version is '%0', but plugin was compiled with version '%1'">;
 
 def warn_module_config_mismatch : Warning<
   "module file %0 cannot be loaded due to a configuration mismatch with the current "
   "compilation">, InGroup<DiagGroup<"module-file-config-mismatch">>, DefaultError;
 def err_module_map_not_found : Error<"module map file '%0' not found">, 
   DefaultFatal;
+def err_module_map_wrong_perms : Error<"insufficient privileges to read file '%0'">,
+  DefaultFatal;
+def err_module_map_missing_stat : Error<"cannot retrieve status infos for file '%0'">,
+  DefaultFatal;
 def err_missing_module_name : Error<
   "no module name provided; specify one with -fmodule-name=">, 
   DefaultFatal;
 def err_missing_module : Error<
   "no module named '%0' declared in module map file '%1'">, DefaultFatal;
 def err_no_submodule : Error<"no submodule named %0 in module '%1'">;
 def err_no_submodule_suggest : Error<
   "no submodule named %0 in module '%1'; did you mean '%2'?">;

lib/Frontend/FrontendActions.cpp

 }
 
 bool GenerateModuleAction::BeginSourceFileAction(CompilerInstance &CI, 
                                                  StringRef Filename) {
   CI.getLangOpts().CompilingModule = true;
 
   // Find the module map file.
   const FileEntry *ModuleMap =
       CI.getFileManager().getFile(Filename, /*openFile*/true);
   if (!ModuleMap)  {
     CI.getDiagnostics().Report(diag::err_module_map_not_found)
       << Filename;
     return false;
   }
-  
+  // Emit a diagnostic if we can't read the module map.
+  vfs::Status Result;
+  bool S = CI.getFileManager().getNoncachedStatValue(Filename, Result);
+  if (!S) {
+    llvm::sys::fs::perms P = Result.getPermissions();
+    if (!(P & llvm::sys::fs::perms::all_read)) {
+      CI.getDiagnostics().Report(diag::err_module_map_wrong_perms)
+        << Filename;
+      return false;
+    }
+  } else {
+    CI.getDiagnostics().Report(diag::err_module_map_missing_stat)
+      << Filename;
+    return false;
+  }
+
   // Set up embedding for any specified files. Do this before we load any
   // source files, including the primary module map for the compilation.
   for (const auto &F : CI.getFrontendOpts().ModulesEmbedFiles) {
     if (const auto *FE = CI.getFileManager().getFile(F, /*openFile*/true))
       CI.getSourceManager().setFileIsTransient(FE);
     else
       CI.getDiagnostics().Report(diag::err_modules_embed_file_not_found) << F;
   }