Skip to content

Commit

Permalink
[clang][deps] Fix handling of -MT in module command-line
Browse files Browse the repository at this point in the history
Follow-up to 6626f6f, this fixes the handling of -MT
* If no targets are provided, we need to invent one since cc1 expects
  the driver to have handled it. The default is to use -o, quoting as
  necessary for a make target.
* Fix the splitting for empty string, which was incorrectly treated as
  {""} instead of {}.
* Add a way to test this behaviour in clang-scan-deps.

Differential Revision: https://reviews.llvm.org/D129607
  • Loading branch information
benlangmuir committed Jul 13, 2022
1 parent 94e0f8e commit 3ce78cb
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 35 deletions.
23 changes: 23 additions & 0 deletions clang/include/clang/Basic/MakeSupport.h
@@ -0,0 +1,23 @@
//===- MakeSupport.h - Make Utilities ---------------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_CLANG_BASIC_MAKESUPPORT_H
#define LLVM_CLANG_BASIC_MAKESUPPORT_H

#include "clang/Basic/LLVM.h"
#include "llvm/ADT/StringRef.h"

namespace clang {

/// Quote target names for inclusion in GNU Make dependency files.
/// Only the characters '$', '#', ' ', '\t' are quoted.
void quoteMakeTarget(StringRef Target, SmallVectorImpl<char> &Res);

} // namespace clang

#endif // LLVM_CLANG_BASIC_MAKESUPPORT_H
Expand Up @@ -72,7 +72,7 @@ enum class ModuleOutputKind {
/// The path of the dependency file (.d), if any.
DependencyFile,
/// The null-separated list of names to use as the targets in the dependency
/// file, if any.
/// file, if any. Defaults to the value of \c ModuleFile, as in the driver.
DependencyTargets,
/// The path of the serialized diagnostic file (.dia), if any.
DiagnosticSerializationFile,
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Basic/CMakeLists.txt
Expand Up @@ -54,6 +54,7 @@ add_clang_library(clangBasic
IdentifierTable.cpp
LangOptions.cpp
LangStandards.cpp
MakeSupport.cpp
Module.cpp
ObjCRuntime.cpp
OpenCLOptions.cpp
Expand Down
35 changes: 35 additions & 0 deletions clang/lib/Basic/MakeSupport.cpp
@@ -0,0 +1,35 @@
//===-- MakeSuport.cpp --------------------------------------------------*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "clang/Basic/MakeSupport.h"

void clang::quoteMakeTarget(StringRef Target, SmallVectorImpl<char> &Res) {
for (unsigned i = 0, e = Target.size(); i != e; ++i) {
switch (Target[i]) {
case ' ':
case '\t':
// Escape the preceding backslashes
for (int j = i - 1; j >= 0 && Target[j] == '\\'; --j)
Res.push_back('\\');

// Escape the space/tab
Res.push_back('\\');
break;
case '$':
Res.push_back('$');
break;
case '#':
Res.push_back('\\');
break;
default:
break;
}

Res.push_back(Target[i]);
}
}
33 changes: 3 additions & 30 deletions clang/lib/Driver/ToolChains/Clang.cpp
Expand Up @@ -27,6 +27,7 @@
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/CodeGenOptions.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/MakeSupport.h"
#include "clang/Basic/ObjCRuntime.h"
#include "clang/Basic/Version.h"
#include "clang/Config/config.h"
Expand Down Expand Up @@ -97,34 +98,6 @@ static void EscapeSpacesAndBackslashes(const char *Arg,
}
}

// Quote target names for inclusion in GNU Make dependency files.
// Only the characters '$', '#', ' ', '\t' are quoted.
static void QuoteTarget(StringRef Target, SmallVectorImpl<char> &Res) {
for (unsigned i = 0, e = Target.size(); i != e; ++i) {
switch (Target[i]) {
case ' ':
case '\t':
// Escape the preceding backslashes
for (int j = i - 1; j >= 0 && Target[j] == '\\'; --j)
Res.push_back('\\');

// Escape the space/tab
Res.push_back('\\');
break;
case '$':
Res.push_back('$');
break;
case '#':
Res.push_back('\\');
break;
default:
break;
}

Res.push_back(Target[i]);
}
}

/// Apply \a Work on the current tool chain \a RegularToolChain and any other
/// offloading tool chain that is associated with the current action \a JA.
static void
Expand Down Expand Up @@ -1249,7 +1222,7 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
} else {
CmdArgs.push_back("-MT");
SmallString<128> Quoted;
QuoteTarget(A->getValue(), Quoted);
quoteMakeTarget(A->getValue(), Quoted);
CmdArgs.push_back(Args.MakeArgString(Quoted));
}
}
Expand All @@ -1274,7 +1247,7 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,

CmdArgs.push_back("-MT");
SmallString<128> Quoted;
QuoteTarget(DepTarget, Quoted);
quoteMakeTarget(DepTarget, Quoted);
CmdArgs.push_back(Args.MakeArgString(Quoted));
}

Expand Down
14 changes: 11 additions & 3 deletions clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Expand Up @@ -8,6 +8,7 @@

#include "clang/Tooling/DependencyScanning/ModuleDepCollector.h"

#include "clang/Basic/MakeSupport.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
Expand Down Expand Up @@ -112,7 +113,7 @@ serializeCompilerInvocation(const CompilerInvocation &CI) {

static std::vector<std::string> splitString(std::string S, char Separator) {
SmallVector<StringRef> Segments;
StringRef(S).split(Segments, Separator);
StringRef(S).split(Segments, Separator, /*MaxSplit=*/-1, /*KeepEmpty=*/false);
std::vector<std::string> Result;
Result.reserve(Segments.size());
for (StringRef Segment : Segments)
Expand All @@ -135,10 +136,17 @@ std::vector<std::string> ModuleDeps::getCanonicalCommandLine(
CI.getDiagnosticOpts().DiagnosticSerializationFile =
LookupModuleOutput(ID, ModuleOutputKind::DiagnosticSerializationFile);
if (HadDependencyFile) {
CI.getDependencyOutputOpts().OutputFile =
DependencyOutputOptions &DepOpts = CI.getDependencyOutputOpts();
DepOpts.OutputFile =
LookupModuleOutput(ID, ModuleOutputKind::DependencyFile);
CI.getDependencyOutputOpts().Targets = splitString(
DepOpts.Targets = splitString(
LookupModuleOutput(ID, ModuleOutputKind::DependencyTargets), '\0');
if (!DepOpts.OutputFile.empty() && DepOpts.Targets.empty()) {
// Fallback to -o as dependency target, as in the driver.
SmallString<128> Target;
quoteMakeTarget(FrontendOpts.OutputFile, Target);
DepOpts.Targets.push_back(std::string(Target));
}
}

for (ModuleID MID : ClangModuleDeps)
Expand Down
17 changes: 17 additions & 0 deletions clang/test/ClangScanDeps/generate-modules-path-args.c
Expand Up @@ -5,6 +5,12 @@
// RUN: clang-scan-deps -compilation-database %t/cdb.json \
// RUN: -format experimental-full -generate-modules-path-args > %t/deps.json
// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
// RUN: clang-scan-deps -compilation-database %t/cdb.json \
// RUN: -format experimental-full -generate-modules-path-args -dependency-target foo > %t/deps_mt1.json
// RUN: cat %t/deps_mt1.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s -check-prefix=DEPS_MT1
// RUN: clang-scan-deps -compilation-database %t/cdb.json \
// RUN: -format experimental-full -generate-modules-path-args -dependency-target foo -dependency-target bar > %t/deps_mt2.json
// RUN: cat %t/deps_mt2.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s -check-prefix=DEPS_MT2
// RUN: clang-scan-deps -compilation-database %t/cdb_without.json \
// RUN: -format experimental-full -generate-modules-path-args > %t/deps_without.json
// RUN: cat %t/deps_without.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t -check-prefix=WITHOUT %s
Expand All @@ -16,17 +22,28 @@
// CHECK-NEXT: "-cc1"
// CHECK: "-serialize-diagnostic-file"
// CHECK-NEXT: "[[PREFIX]]{{.*}}Mod{{.*}}.diag"
// CHECK: "-MT"
// CHECK-NEXT: "[[PREFIX]]{{.*}}Mod{{.*}}.pcm"
// CHECK: "-dependency-file"
// CHECK-NEXT: "[[PREFIX]]{{.*}}Mod{{.*}}.d"
// CHECK: ],

// DEPS_MT1: "-MT"
// DEPS_MT1-NEXT: "foo"

// DEPS_MT2: "-MT"
// DEPS_MT2-NEXT: "foo"
// DEPS_MT2-NEXT: "-MT"
// DEPS_MT2-NEXT: "bar"

// WITHOUT: {
// WITHOUT-NEXT: "modules": [
// WITHOUT-NEXT: {
// WITHOUT: "command-line": [
// WITHOUT-NEXT: "-cc1"
// WITHOUT-NOT: "-serialize-diagnostic-file"
// WITHOUT-NOT: "-dependency-file"
// WITHOUT-NOT: "-MT"
// WITHOUT: ],

//--- cdb.json.template
Expand Down
9 changes: 8 additions & 1 deletion clang/tools/clang-scan-deps/ClangScanDeps.cpp
Expand Up @@ -196,6 +196,12 @@ llvm::cl::opt<std::string> ModuleName(
llvm::cl::desc("the module of which the dependencies are to be computed"),
llvm::cl::cat(DependencyScannerCategory));

llvm::cl::list<std::string> ModuleDepTargets(
"dependency-target",
llvm::cl::desc("With '-generate-modules-path-args', the names of "
"dependency targets for the dependency file"),
llvm::cl::cat(DependencyScannerCategory));

enum ResourceDirRecipeKind {
RDRK_ModifyCompilerPath,
RDRK_InvokeCompiler,
Expand Down Expand Up @@ -367,7 +373,8 @@ class FullDeps {
case ModuleOutputKind::DependencyFile:
return PCMPath.first->second + ".d";
case ModuleOutputKind::DependencyTargets:
return ""; // Will get the default target name.
// Null-separate the list of targets.
return join(ModuleDepTargets, StringRef("\0", 1));
case ModuleOutputKind::DiagnosticSerializationFile:
return PCMPath.first->second + ".diag";
}
Expand Down

0 comments on commit 3ce78cb

Please sign in to comment.