r348154 - Avoid emitting redundant or unusable directories in DIFile metadata entries.
Adrian Prantl via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 3 13:27:31 PST 2018
I'll take a look right away. Thanks for letting me know!
-- adrian
> On Dec 3, 2018, at 1:26 PM, Vlad Tsyrklevich <vlad at tsyrklevich.net> wrote:
>
> This change appears to have broken a number of compiler-rt coverage tests, e.g. in this run <http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/17934/steps/test%20standalone%20compiler-rt/logs/stdio>. The source of the error appears to be that llvm-cov is now trying to use a relative path that's not relative to the directory it's in, though I'm not familiar enough with debuginfo/coverage to understand what an appropriate fix is. I've not yet reverted these changes to give you a chance to take a look.
>
> On Mon, Dec 3, 2018 at 9:58 AM Adrian Prantl via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
> Author: adrian
> Date: Mon Dec 3 09:55:27 2018
> New Revision: 348154
>
> URL: http://llvm.org/viewvc/llvm-project?rev=348154&view=rev <http://llvm.org/viewvc/llvm-project?rev=348154&view=rev>
> Log:
> 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 <https://reviews.llvm.org/D55085>
>
> Added:
> cfe/trunk/test/CodeGen/debug-info-abspath.c
> Modified:
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> cfe/trunk/lib/CodeGen/CodeGenAction.cpp
> cfe/trunk/test/CodeGen/debug-prefix-map.c
> cfe/trunk/test/Modules/module-debuginfo-prefix.m
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=348154&r1=348153&r2=348154&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=348154&r1=348153&r2=348154&view=diff>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Dec 3 09:55:27 2018
> @@ -181,8 +181,7 @@ void CGDebugInfo::setLocation(SourceLoca
> SourceManager &SM = CGM.getContext().getSourceManager();
> auto *Scope = cast<llvm::DIScope>(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<llvm::DILexicalBlockFile>(Scope)) {
> @@ -410,13 +409,13 @@ llvm::DIFile *CGDebugInfo::getOrCreateFi
> 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::getOrCreateFi
> 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;
> }
>
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=348154&r1=348153&r2=348154&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=348154&r1=348153&r2=348154&view=diff>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Mon Dec 3 09:55:27 2018
> @@ -549,12 +549,16 @@ const FullSourceLoc BackendConsumer::get
> 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();
> }
>
> Added: cfe/trunk/test/CodeGen/debug-info-abspath.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-abspath.c?rev=348154&view=auto <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-abspath.c?rev=348154&view=auto>
> ==============================================================================
> --- cfe/trunk/test/CodeGen/debug-info-abspath.c (added)
> +++ cfe/trunk/test/CodeGen/debug-info-abspath.c Mon Dec 3 09:55:27 2018
> @@ -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{{.*}}")
>
> Modified: cfe/trunk/test/CodeGen/debug-prefix-map.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-prefix-map.c?rev=348154&r1=348153&r2=348154&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-prefix-map.c?rev=348154&r1=348153&r2=348154&view=diff>
> ==============================================================================
> --- cfe/trunk/test/CodeGen/debug-prefix-map.c (original)
> +++ cfe/trunk/test/CodeGen/debug-prefix-map.c Mon Dec 3 09:55:27 2018
> @@ -17,18 +17,22 @@ void test_rewrite_includes() {
> }
>
> // CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{/|\\5C}}<stdin>"
> -// 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{{[/\\]}}{{.*}}",
> +// 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:
>
> Modified: cfe/trunk/test/Modules/module-debuginfo-prefix.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/module-debuginfo-prefix.m?rev=348154&r1=348153&r2=348154&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/module-debuginfo-prefix.m?rev=348154&r1=348153&r2=348154&view=diff>
> ==============================================================================
> --- cfe/trunk/test/Modules/module-debuginfo-prefix.m (original)
> +++ cfe/trunk/test/Modules/module-debuginfo-prefix.m Mon Dec 3 09:55:27 2018
> @@ -20,4 +20,4 @@
> @import DebugObjC;
> #endif
>
> -// CHECK: !DIFile({{.*}}"/OVERRIDE/DebugObjC.h"
> +// CHECK: !DIFile(filename: "/OVERRIDE/DebugObjC.h", directory: "")
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181203/c6ceca6a/attachment-0001.html>
More information about the cfe-commits
mailing list