From 212c104ea3c834aa895c8baea471ecc6f539d47c Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Thu, 6 Dec 2018 18:44:50 +0000 Subject: [PATCH] Reapply "Avoid emitting redundant or unusable directories in DIFile metadata entries."" This reverts commit r348280 and reapplies D55085 without modifications. Original commit message: Avoid emitting redundant or unusable directories in DIFile metadata entries. As discussed on llvm-dev recently, Clang currently emits redundant directories in DIFile entries, such as .file 1 "/Volumes/Data/llvm" "/Volumes/Data/llvm/tools/clang/test/CodeGen/debug-info-abspath.c" This patch looks at any common prefix between the compilation directory and the (absolute) file path and strips the redundant part. More importantly it leaves the compilation directory empty if the two paths have no common prefix. After this patch the above entry is (assuming a compilation dir of "/Volumes/Data/llvm/_build"): .file 1 "/Volumes/Data/llvm" "tools/clang/test/CodeGen/debug-info-abspath.c" When building the FileCheck binary with debug info, this patch makes the build artifacts ~1kb smaller. Differential Revision: https://reviews.llvm.org/D55085 llvm-svn: 348513 --- clang/lib/CodeGen/CGDebugInfo.cpp | 47 ++++++++++++++++---- clang/lib/CodeGen/CodeGenAction.cpp | 16 ++++--- clang/test/CodeGen/debug-info-abspath.c | 15 +++++++ clang/test/CodeGen/debug-prefix-map.c | 18 +++++--- clang/test/Modules/module-debuginfo-prefix.m | 4 +- 5 files changed, 78 insertions(+), 22 deletions(-) create mode 100644 clang/test/CodeGen/debug-info-abspath.c diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 3492fbacd33a..c556996be09b 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -181,8 +181,7 @@ void CGDebugInfo::setLocation(SourceLocation Loc) { SourceManager &SM = CGM.getContext().getSourceManager(); auto *Scope = cast(LexicalBlockStack.back()); PresumedLoc PCLoc = SM.getPresumedLoc(CurLoc); - - if (PCLoc.isInvalid() || Scope->getFilename() == PCLoc.getFilename()) + if (PCLoc.isInvalid() || Scope->getFile() == getOrCreateFile(CurLoc)) return; if (auto *LBF = dyn_cast(Scope)) { @@ -410,13 +409,13 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) { SourceManager &SM = CGM.getContext().getSourceManager(); PresumedLoc PLoc = SM.getPresumedLoc(Loc); - if (PLoc.isInvalid() || StringRef(PLoc.getFilename()).empty()) + StringRef FileName = PLoc.getFilename(); + if (PLoc.isInvalid() || FileName.empty()) // If the location is not valid then use main input file. return getOrCreateMainFile(); // Cache the results. - const char *fname = PLoc.getFilename(); - auto It = DIFileCache.find(fname); + auto It = DIFileCache.find(FileName.data()); if (It != DIFileCache.end()) { // Verify that the information still exists. @@ -431,11 +430,41 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) { if (CSKind) CSInfo.emplace(*CSKind, Checksum); - llvm::DIFile *F = DBuilder.createFile( - remapDIPath(PLoc.getFilename()), remapDIPath(getCurrentDirname()), CSInfo, - getSource(SM, SM.getFileID(Loc))); + StringRef Dir; + StringRef File; + std::string RemappedFile = remapDIPath(FileName); + std::string CurDir = remapDIPath(getCurrentDirname()); + SmallString<128> DirBuf; + SmallString<128> FileBuf; + if (llvm::sys::path::is_absolute(RemappedFile)) { + // Strip the common prefix (if it is more than just "/") from current + // directory and FileName for a more space-efficient encoding. + auto FileIt = llvm::sys::path::begin(RemappedFile); + auto FileE = llvm::sys::path::end(RemappedFile); + auto CurDirIt = llvm::sys::path::begin(CurDir); + auto CurDirE = llvm::sys::path::end(CurDir); + for (; CurDirIt != CurDirE && *CurDirIt == *FileIt; ++CurDirIt, ++FileIt) + llvm::sys::path::append(DirBuf, *CurDirIt); + if (std::distance(llvm::sys::path::begin(CurDir), CurDirIt) == 1) { + // The common prefix only the root; stripping it would cause + // LLVM diagnostic locations to be more confusing. + Dir = {}; + File = RemappedFile; + } else { + for (; FileIt != FileE; ++FileIt) + llvm::sys::path::append(FileBuf, *FileIt); + Dir = DirBuf; + File = FileBuf; + } + } else { + Dir = CurDir; + File = RemappedFile; + } + llvm::DIFile *F = + DBuilder.createFile(File, Dir, CSInfo, + getSource(SM, SM.getFileID(Loc))); - DIFileCache[fname].reset(F); + DIFileCache[FileName.data()].reset(F); return F; } diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp index 1a2b0616dc77..fd4506f2d197 100644 --- a/clang/lib/CodeGen/CodeGenAction.cpp +++ b/clang/lib/CodeGen/CodeGenAction.cpp @@ -549,12 +549,16 @@ const FullSourceLoc BackendConsumer::getBestLocationFromDebugLoc( SourceLocation DILoc; if (D.isLocationAvailable()) { - D.getLocation(&Filename, &Line, &Column); - const FileEntry *FE = FileMgr.getFile(Filename); - if (FE && Line > 0) { - // If -gcolumn-info was not used, Column will be 0. This upsets the - // source manager, so pass 1 if Column is not set. - DILoc = SourceMgr.translateFileLineCol(FE, Line, Column ? Column : 1); + D.getLocation(Filename, Line, Column); + if (Line > 0) { + const FileEntry *FE = FileMgr.getFile(Filename); + if (!FE) + FE = FileMgr.getFile(D.getAbsolutePath()); + if (FE) { + // If -gcolumn-info was not used, Column will be 0. This upsets the + // source manager, so pass 1 if Column is not set. + DILoc = SourceMgr.translateFileLineCol(FE, Line, Column ? Column : 1); + } } BadDebugInfo = DILoc.isInvalid(); } diff --git a/clang/test/CodeGen/debug-info-abspath.c b/clang/test/CodeGen/debug-info-abspath.c new file mode 100644 index 000000000000..3ca170621f9b --- /dev/null +++ b/clang/test/CodeGen/debug-info-abspath.c @@ -0,0 +1,15 @@ +// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \ +// RUN: %s -emit-llvm -o - | FileCheck %s + +// RUN: cp %s %t.c +// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \ +// RUN: %t.c -emit-llvm -o - | FileCheck %s --check-prefix=INTREE +void foo() {} + +// Since %s is an absolute path, directory should be a nonempty +// prefix, but the CodeGen part should be part of the filename. + +// CHECK: DIFile(filename: "{{.*}}CodeGen{{.*}}debug-info-abspath.c" +// CHECK-SAME: directory: "{{.+}}") + +// INTREE: DIFile({{.*}}directory: "{{.+}}CodeGen{{.*}}") diff --git a/clang/test/CodeGen/debug-prefix-map.c b/clang/test/CodeGen/debug-prefix-map.c index dfb57bbe2e4b..e66866ba6c36 100644 --- a/clang/test/CodeGen/debug-prefix-map.c +++ b/clang/test/CodeGen/debug-prefix-map.c @@ -17,18 +17,24 @@ void test_rewrite_includes() { } // CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{/|\\5C}}" -// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}" -// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{[/\\]}}Inputs/stdio.h" +// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}", +// Dir should always be empty, but on Windows we can't recognize /var +// as being an absolute path. +// CHECK-NO-MAIN-FILE-NAME-SAME: directory: "{{()|(.*:.*)}}") +// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{[/\\]}}Inputs/stdio.h", +// CHECK-NO-MAIN-FILE-NAME-SAME: directory: "{{()|(.*:.*)}}") // CHECK-NO-MAIN-FILE-NAME-NOT: !DIFile(filename: // CHECK-EVIL: !DIFile(filename: "/var=empty{{[/\\]}}{{.*}}" -// CHECK-EVIL: !DIFile(filename: "/var=empty{{[/\\]}}Inputs/stdio.h" +// CHECK-EVIL: !DIFile(filename: "/var=empty{{[/\\]}}{{.*}}Inputs/stdio.h", +// CHECK-EVIL-SAME: directory: "{{()|(.*:.*)}}") // CHECK-EVIL-NOT: !DIFile(filename: // CHECK: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}" -// CHECK: !DIFile(filename: "/var/empty{{[/\\]}}Inputs/stdio.h" +// CHECK: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}Inputs/stdio.h", +// CHECK-SAME: directory: "{{()|(.*:.*)}}") // CHECK-NOT: !DIFile(filename: -// CHECK-COMPILATION-DIR: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}", directory: "/var/empty") -// CHECK-COMPILATION-DIR: !DIFile(filename: "/var/empty{{[/\\]}}Inputs/stdio.h", directory: "/var/empty") +// CHECK-COMPILATION-DIR: !DIFile(filename: "{{.*}}", directory: "/var/empty") +// CHECK-COMPILATION-DIR: !DIFile(filename: "{{.*}}Inputs/stdio.h", directory: "/var/empty") // CHECK-COMPILATION-DIR-NOT: !DIFile(filename: diff --git a/clang/test/Modules/module-debuginfo-prefix.m b/clang/test/Modules/module-debuginfo-prefix.m index c4a7d2b614a8..da5d86abefd0 100644 --- a/clang/test/Modules/module-debuginfo-prefix.m +++ b/clang/test/Modules/module-debuginfo-prefix.m @@ -20,4 +20,6 @@ @import DebugObjC; #endif -// CHECK: !DIFile({{.*}}"/OVERRIDE/DebugObjC.h" +// Dir should always be empty, but on Windows we can't recognize /var +// as being an absolute path. +// CHECK: !DIFile(filename: "/OVERRIDE/DebugObjC.h", directory: "{{()|(.*:.*)}}")