[PATCH] D73060: [Clang] Fix expansion of response files in -Wp after integrated-cc1 change

[Alexandre Ganea via Phabricator via cfe-commits]

aganea created this revision.
aganea added reviewers: hans, thakis, wenlei.
aganea added a project: clang.
Herald added a subscriber: cfe-commits.
aganea edited the summary of this revision.

After rGb4a99a061f517e60985667e39519f60186cbb469 
<https://reviews.llvm.org/rGb4a99a061f517e60985667e39519f60186cbb469>, passing 
a response file such as `-Wp,@a.rsp` wasn't working anymore because .rsp 
expansion happens inside clang's main() function.

I'm noticing along the way that explicit arguments in `-Wp` are parsed in the 
driver, whereas arguments inside a response file are parsed in the -cc1 tool. 
Is that the desired behavior? This was the previous behavior but I'm not sure 
it's the right behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73060

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Job.cpp
  clang/test/Driver/Wp-args.c
  clang/tools/driver/driver.cpp


Index: clang/tools/driver/driver.cpp
===================================================================
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -241,8 +241,6 @@
       *NumberSignPtr = '=';
 }
 
-static int ExecuteCC1Tool(ArrayRef<const char *> argv);
-
 static void SetBackdoorDriverOutputsFromEnvVars(Driver &TheDriver) {
   // Handle CC_PRINT_OPTIONS and CC_PRINT_OPTIONS_FILE.
   TheDriver.CCPrintOptions = !!::getenv("CC_PRINT_OPTIONS");
@@ -318,7 +316,6 @@
   // Driver::CC1Main), we need to clean up the options usage count. The options
   // are currently global, and they might have been used previously by the
   // driver.
-  llvm::cl::ResetAllOptionOccurrences();
   StringRef Tool = argv[1];
   void *GetExecutablePathVP = (void *)(intptr_t) GetExecutablePath;
   if (Tool == "-cc1")
@@ -334,17 +331,13 @@
   return 1;
 }
 
-int main(int argc_, const char **argv_) {
-  noteBottomOfStack();
-  llvm::InitLLVM X(argc_, argv_);
-  SmallVector<const char *, 256> argv(argv_, argv_ + argc_);
-
-  if (llvm::sys::Process::FixupStandardFileDescriptors())
-    return 1;
-
-  llvm::InitializeAllTargets();
+int ExecuteClangTool(SmallVectorImpl<const char *> &argv) {
   auto TargetAndMode = ToolChain::getTargetAndModeFromProgramName(argv[0]);
 
+  static unsigned ReenteranceCount;
+  if (ReenteranceCount++)
+    llvm::cl::ResetAllOptionOccurrences();
+
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
@@ -480,7 +473,7 @@
   SetBackdoorDriverOutputsFromEnvVars(TheDriver);
 
   if (!UseNewCC1Process) {
-    TheDriver.CC1Main = &ExecuteCC1Tool;
+    TheDriver.CC1Main = &ExecuteClangTool;
     // Ensure the CC1Command actually catches cc1 crashes
     llvm::CrashRecoveryContext::Enable();
   }
@@ -543,3 +536,15 @@
   // failing command.
   return Res;
 }
+
+int main(int argc_, const char **argv_) {
+  noteBottomOfStack();
+  llvm::InitLLVM X(argc_, argv_);
+  SmallVector<const char *, 256> argv(argv_, argv_ + argc_);
+
+  if (llvm::sys::Process::FixupStandardFileDescriptors())
+    return 1;
+
+  llvm::InitializeAllTargets();
+  return ExecuteClangTool(argv);
+}
Index: clang/test/Driver/Wp-args.c
===================================================================
--- clang/test/Driver/Wp-args.c
+++ clang/test/Driver/Wp-args.c
@@ -19,3 +19,9 @@
 // MMD: "-cc1"
 // MMD-NOT: -MMD
 // MMD: "-dependency-file" "Wp-args.d"
+
+// Ensure response files are properly expanded with -Wp
+// RUN: echo -rewrite-macros > %t.rsp
+// RUN: %clang -Wp,@%t.rsp -E %s | FileCheck -check-prefix RSP %s
+
+// RSP: inception
Index: clang/lib/Driver/Job.cpp
===================================================================
--- clang/lib/Driver/Job.cpp
+++ clang/lib/Driver/Job.cpp
@@ -384,7 +384,6 @@
   SmallVector<const char *, 128> Argv;
   Argv.push_back(getExecutable());
   Argv.append(getArguments().begin(), getArguments().end());
-  Argv.push_back(nullptr);
 
   // This flag simply indicates that the program couldn't start, which isn't
   // applicable here.
Index: clang/include/clang/Driver/Driver.h
===================================================================
--- clang/include/clang/Driver/Driver.h
+++ clang/include/clang/Driver/Driver.h
@@ -208,7 +208,7 @@
   /// When the clangDriver lib is used through clang.exe, this provides a
   /// shortcut for executing the -cc1 command-line directly, in the same
   /// process.
-  typedef int (*CC1ToolFunc)(ArrayRef<const char *> argv);
+  typedef int (*CC1ToolFunc)(SmallVectorImpl<const char *> &);
   CC1ToolFunc CC1Main = nullptr;
 
 private:

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits