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 15:14:32 PST 2018
Should be fixed in r348211.
-- adrian
> On Dec 3, 2018, at 3:07 PM, Adrian Prantl via cfe-commits <cfe-commits at lists.llvm.org> wrote:
>
> No, your failures are Windows-specific (/ vs \), and I haven't fixed them yet. Thanks for letting me know!
>
> -- adrian
>
>> On Dec 3, 2018, at 3:06 PM, Galina Kistanova <gkistanova at gmail.com> wrote:
>>
>> Adrian, did not see your response, please ignore my email.
>>
>> Thanks
>>
>> Galina
>>
>> On Mon, Dec 3, 2018 at 3:04 PM Galina Kistanova <gkistanova at gmail.com> wrote:
>> Hello Adrian,
>>
>> This commit broke tests on couple of our builders:
>>
>> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/14371
>> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast
>>
>> . . .
>> Failing Tests (2):
>> Clang :: CodeGen/debug-prefix-map.c
>> Clang :: Modules/module-debuginfo-prefix.m
>>
>> The builders were already red and no notifications were sent on this.
>> Please have a look?
>>
>> Thanks
>>
>> Galina
>>
>> On Mon, Dec 3, 2018 at 9:58 AM Adrian Prantl via cfe-commits <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
>> 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
>>
>> 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
>> ==============================================================================
>> --- 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
>> ==============================================================================
>> --- 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
>> ==============================================================================
>> --- 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
>> ==============================================================================
>> --- 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
>> ==============================================================================
>> --- 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
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list