Skip to content

Revert "[Coverage] Store compilation dir separately in coverage mapping" #3212

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
Aug 31, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions clang/include/clang/Basic/CodeGenOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,6 @@ class CodeGenOptions : public CodeGenOptionsBase {
/// The string to embed in debug information as the current working directory.
std::string DebugCompilationDir;

/// The string to embed in coverage mapping as the current working directory.
std::string CoverageCompilationDir;

/// The string to embed in the debug information for the compile unit, if
/// non-empty.
std::string DwarfDebugFlags;
Expand Down
4 changes: 0 additions & 4 deletions clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -1176,10 +1176,6 @@ def fdebug_compilation_dir_EQ : Joined<["-"], "fdebug-compilation-dir=">,
def fdebug_compilation_dir : Separate<["-"], "fdebug-compilation-dir">,
Group<f_Group>, Flags<[CC1Option, CC1AsOption, CoreOption]>,
Alias<fdebug_compilation_dir_EQ>;
def fcoverage_compilation_dir_EQ : Joined<["-"], "fcoverage-compilation-dir=">,
Group<f_Group>, Flags<[CC1Option, CC1AsOption, CoreOption]>,
HelpText<"The compilation directory to embed in the coverage mapping.">,
MarshallingInfoString<CodeGenOpts<"CoverageCompilationDir">>;
def ffile_compilation_dir_EQ : Joined<["-"], "ffile-compilation-dir=">, Group<f_Group>,
Flags<[CoreOption]>,
HelpText<"The compilation directory to embed in the debug info and coverage mapping.">;
Expand Down
30 changes: 12 additions & 18 deletions clang/lib/CodeGen/CoverageMappingGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1563,17 +1563,9 @@ CoverageMappingModuleGen::CoverageMappingModuleGen(
CoveragePrefixMap = CGM.getCodeGenOpts().CoveragePrefixMap;
}

std::string CoverageMappingModuleGen::getCurrentDirname() {
if (!CGM.getCodeGenOpts().CoverageCompilationDir.empty())
return CGM.getCodeGenOpts().CoverageCompilationDir;

SmallString<256> CWD;
llvm::sys::fs::current_path(CWD);
return CWD.str().str();
}

std::string CoverageMappingModuleGen::normalizeFilename(StringRef Filename) {
llvm::SmallString<256> Path(Filename);
llvm::sys::fs::make_absolute(Path);
llvm::sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
for (const auto &Entry : CoveragePrefixMap) {
if (llvm::sys::path::replace_path_prefix(Path, Entry.first, Entry.second))
Expand Down Expand Up @@ -1654,17 +1646,18 @@ void CoverageMappingModuleGen::addFunctionMappingRecord(
// also processed by the CoverageMappingWriter which performs
// additional minimization operations such as reducing the number of
// expressions.
llvm::SmallVector<std::string, 16> FilenameStrs;
std::vector<StringRef> Filenames;
std::vector<CounterExpression> Expressions;
std::vector<CounterMappingRegion> Regions;
FilenameStrs.resize(FileEntries.size() + 1);
FilenameStrs[0] = normalizeFilename(getCurrentDirname());
llvm::SmallVector<std::string, 16> FilenameStrs;
llvm::SmallVector<StringRef, 16> FilenameRefs;
FilenameStrs.resize(FileEntries.size());
FilenameRefs.resize(FileEntries.size());
for (const auto &Entry : FileEntries) {
auto I = Entry.second;
FilenameStrs[I] = normalizeFilename(Entry.first->getName());
FilenameRefs[I] = FilenameStrs[I];
}
ArrayRef<std::string> FilenameRefs = llvm::makeArrayRef(FilenameStrs);
RawCoverageMappingReader Reader(CoverageMapping, FilenameRefs, Filenames,
Expressions, Regions);
if (Reader.read())
Expand All @@ -1681,18 +1674,19 @@ void CoverageMappingModuleGen::emit() {

// Create the filenames and merge them with coverage mappings
llvm::SmallVector<std::string, 16> FilenameStrs;
FilenameStrs.resize(FileEntries.size() + 1);
// The first filename is the current working directory.
FilenameStrs[0] = normalizeFilename(getCurrentDirname());
llvm::SmallVector<StringRef, 16> FilenameRefs;
FilenameStrs.resize(FileEntries.size());
FilenameRefs.resize(FileEntries.size());
for (const auto &Entry : FileEntries) {
auto I = Entry.second;
FilenameStrs[I] = normalizeFilename(Entry.first->getName());
FilenameRefs[I] = FilenameStrs[I];
}

std::string Filenames;
{
llvm::raw_string_ostream OS(Filenames);
CoverageFilenamesSectionWriter(FilenameStrs).write(OS);
CoverageFilenamesSectionWriter(FilenameRefs).write(OS);
}
auto *FilenamesVal =
llvm::ConstantDataArray::getString(Ctx, Filenames, false);
Expand Down Expand Up @@ -1750,7 +1744,7 @@ unsigned CoverageMappingModuleGen::getFileID(const FileEntry *File) {
auto It = FileEntries.find(File);
if (It != FileEntries.end())
return It->second;
unsigned FileID = FileEntries.size() + 1;
unsigned FileID = FileEntries.size();
FileEntries.insert(std::make_pair(File, FileID));
return FileID;
}
Expand Down
1 change: 0 additions & 1 deletion clang/lib/CodeGen/CoverageMappingGen.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ class CoverageMappingModuleGen {
std::vector<FunctionInfo> FunctionRecords;
std::map<std::string, std::string> CoveragePrefixMap;

std::string getCurrentDirname();
std::string normalizeFilename(StringRef Filename);

/// Emit a function record.
Expand Down
12 changes: 0 additions & 12 deletions clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -885,18 +885,6 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
CmdArgs.push_back("-fcoverage-mapping");
}

if (Arg *A = Args.getLastArg(options::OPT_ffile_compilation_dir_EQ,
options::OPT_fcoverage_compilation_dir_EQ)) {
if (A->getOption().matches(options::OPT_ffile_compilation_dir_EQ))
CmdArgs.push_back(Args.MakeArgString(
Twine("-fcoverage-compilation-dir=") + A->getValue()));
else
A->render(Args, CmdArgs);
} else if (llvm::ErrorOr<std::string> CWD =
D.getVFS().getCurrentWorkingDirectory()) {
CmdArgs.push_back(Args.MakeArgString("-fcoverage-compilation-dir=" + *CWD));
}

if (Args.hasArg(options::OPT_fprofile_exclude_files_EQ)) {
auto *Arg = Args.getLastArg(options::OPT_fprofile_exclude_files_EQ);
if (!Args.hasArg(options::OPT_coverage))
Expand Down
3 changes: 3 additions & 0 deletions clang/test/CodeGen/coverage-compilation-dir.c
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// XFAIL: *
// See rdar://82543962.
//
// RUN: mkdir -p %t.dir && cd %t.dir
// RUN: cp %s rel.c
// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-compilation-dir=/nonsense -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false rel.c -o - | FileCheck -check-prefix=CHECK-NONSENSE %s
Expand Down
12 changes: 4 additions & 8 deletions clang/test/CoverageMapping/abspath.cpp
Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -mllvm -enable-name-compression=false -emit-llvm -main-file-name abspath.cpp %S/Inputs/../abspath.cpp -o - | FileCheck -check-prefix=RMDOTS %s

// RMDOTS: @__llvm_coverage_mapping = {{.*}}"\02
// RMDOTS: @__llvm_coverage_mapping = {{.*}}"\01
// RMDOTS-NOT: Inputs
// RMDOTS: "

// RUN: mkdir -p %t/test && cd %t/test
// RUN: echo "void f1() {}" > f1.c
// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -mllvm -enable-name-compression=false -emit-llvm -main-file-name abspath.cpp %t/test/f1.c -o - | FileCheck -check-prefix=ABSPATH %s
// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -mllvm -enable-name-compression=false -emit-llvm -main-file-name abspath.cpp ../test/f1.c -o - | FileCheck -check-prefix=RELPATH %s

// RELPATH: @__llvm_coverage_mapping = {{.*}}"\02
// RELPATH: {{..(/|\\\\)test(/|\\\\)f1}}.c
// RELPATH: @__llvm_coverage_mapping = {{.*}}"\01
// RELPATH: {{[/\\].*(/|\\\\)test(/|\\\\)f1}}.c
// RELPATH: "

// ABSPATH: @__llvm_coverage_mapping = {{.*}}"\02
// ABSPATH: {{[/\\].*(/|\\\\)test(/|\\\\)f1}}.c
// ABSPATH: "

void f1() {}
9 changes: 5 additions & 4 deletions clang/test/Driver/clang_f_opts.c
Original file line number Diff line number Diff line change
Expand Up @@ -524,10 +524,11 @@
// CHECK-DEBUG-COMPILATION-DIR: "-fdebug-compilation-dir=."
// CHECK-DEBUG-COMPILATION-DIR-NOT: "-ffile-compilation-dir=."

// RUN: %clang -### -S -fprofile-instr-generate -fcoverage-compilation-dir=. %s 2>&1 | FileCheck -check-prefix=CHECK-COVERAGE-COMPILATION-DIR %s
// RUN: %clang -### -S -fprofile-instr-generate -ffile-compilation-dir=. %s 2>&1 | FileCheck -check-prefix=CHECK-COVERAGE-COMPILATION-DIR %s
// CHECK-COVERAGE-COMPILATION-DIR: "-fcoverage-compilation-dir=."
// CHECK-COVERAGE-COMPILATION-DIR-NOT: "-ffile-compilation-dir=."
// See rdar://82543962.
// XXX: %clang -### -S -fprofile-instr-generate -fcoverage-compilation-dir=. %s 2>&1 | FileCheck -check-prefix=CHECK-COVERAGE-COMPILATION-DIR %s
// XXX: %clang -### -S -fprofile-instr-generate -ffile-compilation-dir=. %s 2>&1 | FileCheck -check-prefix=CHECK-COVERAGE-COMPILATION-DIR %s
// XXX-COVERAGE-COMPILATION-DIR: "-fcoverage-compilation-dir=."
// XXX-COVERAGE-COMPILATION-DIR-NOT: "-ffile-compilation-dir=."

// RUN: %clang -### -S -fdiscard-value-names %s 2>&1 | FileCheck -check-prefix=CHECK-DISCARD-NAMES %s
// RUN: %clang -### -S -fno-discard-value-names %s 2>&1 | FileCheck -check-prefix=CHECK-NO-DISCARD-NAMES %s
Expand Down
3 changes: 3 additions & 0 deletions clang/test/Profile/coverage-prefix-map.c
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// XFAIL: *
// See rdar://82543962.
//
// %s expands to an absolute path, so to test relative paths we need to create a
// clean directory, put the source there, and cd into it.
// RUN: rm -rf %t
Expand Down
17 changes: 17 additions & 0 deletions clang/test/Profile/profile-prefix-map.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// XFAIL: *
// See rdar://82543962.
//
// %s expands to an absolute path, so to test relative paths we need to create a
// clean directory, put the source there, and cd into it.
// RUN: rm -rf %t
// RUN: mkdir -p %t/root/nested
// RUN: echo "void f1() {}" > %t/root/nested/profile-prefix-map.c
// RUN: cd %t/root

// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c nested/profile-prefix-map.c -o - | FileCheck --check-prefix=ABSOLUTE %s
//
// ABSOLUTE: @__llvm_coverage_mapping = {{.*"\\01.*root.*nested.*profile-prefix-map\.c}}

// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c nested/profile-prefix-map.c -fprofile-prefix-map=%/t/root=. -o - | FileCheck --check-prefix=PROFILE-PREFIX-MAP %s --implicit-check-not=root
//
// PROFILE-PREFIX-MAP: @__llvm_coverage_mapping = {{.*"\\01[^/]*}}.{{/|\\+}}nested{{.*profile-prefix-map\.c}}
2 changes: 1 addition & 1 deletion compiler-rt/include/profile/InstrProfData.inc
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
/* Indexed profile format version (start from 1). */
#define INSTR_PROF_INDEX_VERSION 7
/* Coverage mapping format version (start from 0). */
#define INSTR_PROF_COVMAP_VERSION 5
#define INSTR_PROF_COVMAP_VERSION 4

/* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
* version for other variants of profile. We set the lowest bit of the upper 8
Expand Down
11 changes: 1 addition & 10 deletions llvm/docs/CoverageMappingFormat.rst
Original file line number Diff line number Diff line change
Expand Up @@ -266,16 +266,7 @@ too deeply).
[32 x i8] c"..." ; Encoded data (dissected later)
}, section "__llvm_covmap", align 8

The current version of the format is version 6.

There is one difference between versions 6 and 5:

* The first entry in the filename list is the compilation directory. When the
filename is relative, the compilation directory is combined with the relative
path to get an absolute path. This can reduce size by omitting the duplicate
prefix in filenames.

There is one difference between versions 5 and 4:
The current version of the format is version 5. There is one difference from version 4:

* The notion of branch region has been introduced along with a corresponding
region kind. Branch regions encode two counters, one to track how many
Expand Down
5 changes: 1 addition & 4 deletions llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
Original file line number Diff line number Diff line change
Expand Up @@ -1003,10 +1003,7 @@ enum CovMapVersion {
Version4 = 3,
// Branch regions referring to two counters are added
Version5 = 4,
// Compilation directory is stored separately and combined with relative
// filenames to produce an absolute file path.
Version6 = 5,
// The current version is Version6.
// The current version is Version5.
CurrentVersion = INSTR_PROF_COVMAP_VERSION
};

Expand Down
27 changes: 15 additions & 12 deletions llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,14 @@ class RawCoverageMappingDummyChecker : public RawCoverageReader {

/// Reader for the raw coverage mapping data.
class RawCoverageMappingReader : public RawCoverageReader {
ArrayRef<std::string> &TranslationUnitFilenames;
ArrayRef<StringRef> TranslationUnitFilenames;
std::vector<StringRef> &Filenames;
std::vector<CounterExpression> &Expressions;
std::vector<CounterMappingRegion> &MappingRegions;

public:
RawCoverageMappingReader(StringRef MappingData,
ArrayRef<std::string> &TranslationUnitFilenames,
ArrayRef<StringRef> TranslationUnitFilenames,
std::vector<StringRef> &Filenames,
std::vector<CounterExpression> &Expressions,
std::vector<CounterMappingRegion> &MappingRegions)
Expand Down Expand Up @@ -179,10 +179,12 @@ class BinaryCoverageReader : public CoverageMappingReader {
FilenamesBegin(FilenamesBegin), FilenamesSize(FilenamesSize) {}
};

using DecompressedData = std::vector<std::unique_ptr<SmallVector<char, 0>>>;

using FuncRecordsStorage = std::unique_ptr<MemoryBuffer>;

private:
std::vector<std::string> Filenames;
std::vector<StringRef> Filenames;
std::vector<ProfileMappingRecord> MappingRecords;
InstrProfSymtab ProfileNames;
size_t CurrentRecord = 0;
Expand All @@ -195,6 +197,10 @@ class BinaryCoverageReader : public CoverageMappingReader {
// D69471, which can split up function records into multiple sections on ELF.
FuncRecordsStorage FuncRecords;

// Used to tie the lifetimes of decompressed strings to the lifetime of this
// BinaryCoverageReader instance.
DecompressedData Decompressed;

BinaryCoverageReader(FuncRecordsStorage &&FuncRecords)
: FuncRecords(std::move(FuncRecords)) {}

Expand All @@ -220,23 +226,20 @@ class BinaryCoverageReader : public CoverageMappingReader {

/// Reader for the raw coverage filenames.
class RawCoverageFilenamesReader : public RawCoverageReader {
std::vector<std::string> &Filenames;
StringRef CompilationDir;
std::vector<StringRef> &Filenames;

// Read an uncompressed sequence of filenames.
Error readUncompressed(CovMapVersion Version, uint64_t NumFilenames);
Error readUncompressed(uint64_t NumFilenames);

public:
RawCoverageFilenamesReader(StringRef Data,
std::vector<std::string> &Filenames,
StringRef CompilationDir = "")
: RawCoverageReader(Data), Filenames(Filenames),
CompilationDir(CompilationDir) {}
RawCoverageFilenamesReader(StringRef Data, std::vector<StringRef> &Filenames)
: RawCoverageReader(Data), Filenames(Filenames) {}
RawCoverageFilenamesReader(const RawCoverageFilenamesReader &) = delete;
RawCoverageFilenamesReader &
operator=(const RawCoverageFilenamesReader &) = delete;

Error read(CovMapVersion Version);
Error read(CovMapVersion Version,
BinaryCoverageReader::DecompressedData &Decompressed);
};

} // end namespace coverage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ namespace coverage {
/// Writer of the filenames section for the instrumentation
/// based code coverage.
class CoverageFilenamesSectionWriter {
ArrayRef<std::string> Filenames;
ArrayRef<StringRef> Filenames;

public:
CoverageFilenamesSectionWriter(ArrayRef<std::string> Filenames);
CoverageFilenamesSectionWriter(ArrayRef<StringRef> Filenames);

/// Write encoded filenames to the given output stream. If \p Compress is
/// true, attempt to compress the filenames.
Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/ProfileData/InstrProfData.inc
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
/* Indexed profile format version (start from 1). */
#define INSTR_PROF_INDEX_VERSION 7
/* Coverage mapping format version (start from 0). */
#define INSTR_PROF_COVMAP_VERSION 5
#define INSTR_PROF_COVMAP_VERSION 4

/* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
* version for other variants of profile. We set the lowest bit of the upper 8
Expand Down
Loading