[llvm] r340583 - DebugInfo: Improve debug location merging

Reid Kleckner via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 27 18:06:01 PDT 2018


I got the crash files and am running a reduction.

What's happening is that this file has a function with several CHECKs, and
their failure cases get commoned together. We're using this new merged
source location, and something goes wrong.

On Mon, Aug 27, 2018 at 5:18 PM Vitaly Buka <vitalybuka at google.com> wrote:

> I can't reproduce on Linux.
> +Reid Kleckner <rnk at google.com>
>
>
> On Mon, Aug 27, 2018 at 5:12 PM David Blaikie <dblaikie at gmail.com> wrote:
>
>> Yeah, I'm totally open to some ideas on investigating that - can you
>> reproduce this? Does it reproduce not on windows? I'd love some help!
>>
>> On Mon, Aug 27, 2018 at 5:09 PM Vitaly Buka <vitalybuka at google.com>
>> wrote:
>>
>>> This revision breaks windows bot
>>>
>>> http://lab.llvm.org:8011/builders/sanitizer-windows/builds/33862/steps/run%20check-sanitizer/logs/stdio
>>>
>>> FAILED: projects/compiler-rt/lib/sanitizer_common/tests/SANITIZER_TEST_OBJECTS.sanitizer_stacktrace_printer_test.cc.i386.o
>>> cmd.exe /C "cd /D C:\b\slave\sanitizer-windows\build\projects\compiler-rt\lib\sanitizer_common\tests && C:\b\slave\sanitizer-windows\build\.\bin\clang.exe -DWIN32 -D_WINDOWS -Wno-unknown-warning-option -fms-compatibility-version=19.00.24215.1 -D_HAS_EXCEPTIONS=0 -Wno-undefined-inline -DGTEST_NO_LLVM_RAW_OSTREAM=1 -DGTEST_HAS_RTTI=0 -IC:/b/slave/sanitizer-windows/llvm/utils/unittest/googletest/include -IC:/b/slave/sanitizer-windows/llvm/utils/unittest/googletest -DGTEST_HAS_SEH=0 -Wno-deprecated-declarations -IC:/b/slave/sanitizer-windows/llvm/projects/compiler-rt/include -IC:/b/slave/sanitizer-windows/llvm/projects/compiler-rt/lib -IC:/b/slave/sanitizer-windows/llvm/projects/compiler-rt/lib/sanitizer_common -fno-rtti -O2 -Werror=sign-compare -Wno-non-virtual-dtor -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -DGTEST_HAS_SEH=0 -gline-tables-only -gcodeview -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -c -o SANITIZER_TEST_OBJECTS.sanitizer_stacktrace_printer_test.cc.i386.o C:/b/slave/sanitizer-windows/llvm/projects/compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_printer_test.cc"
>>> Assertion failed: Result >= 0 && "negative label difference requested", file C:\b\slave\sanitizer-windows\llvm\lib\MC\MCCodeView.cpp, line 449
>>>
>>> Wrote crash dump file "C:\Users\buildbot\AppData\Local\Temp\clang.exe-30d2bf.dmp"
>>>
>>> LLVMSymbolizer: error reading file: PDB Error: Unable to load PDB.  Make sure the file exists and is readable.  Calling loadDataForExe
>>>
>>>
>>>
>>> #0 0x021cf367 (C:\b\slave\sanitizer-windows\build\bin\clang.exe+0x119f367)
>>>
>>> #1 0x72ec91d2 (C:\windows\SYSTEM32\ucrtbase.DLL+0x891d2)
>>>
>>> #2 0x72eca7e1 (C:\windows\SYSTEM32\ucrtbase.DLL+0x8a7e1)
>>>
>>> #3 0x72ec9c1e (C:\windows\SYSTEM32\ucrtbase.DLL+0x89c1e)
>>>
>>> #4 0x72eca8e6 (C:\windows\SYSTEM32\ucrtbase.DLL+0x8a8e6)
>>>
>>> #5 0x0201cad9 (C:\b\slave\sanitizer-windows\build\bin\clang.exe+0xfecad9)
>>>
>>> #6 0x0201e219 (C:\b\slave\sanitizer-windows\build\bin\clang.exe+0xfee219)
>>>
>>> #7 0x02010b71 (C:\b\slave\sanitizer-windows\build\bin\clang.exe+0xfe0b71)
>>>
>>> #8 0x020109f6 (C:\b\slave\sanitizer-windows\build\bin\clang.exe+0xfe09f6)
>>>
>>> #9 0x0201049d (C:\b\slave\sanitizer-windows\build\bin\clang.exe+0xfe049d)
>>>
>>> #10 0x01fee676 (C:\b\slave\sanitizer-windows\build\bin\clang.exe+0xfbe676)
>>>
>>> #11 0x021aa8b3 (C:\b\slave\sanitizer-windows\build\bin\clang.exe+0x117a8b3)
>>>
>>> #12 0x0200cd47 (C:\b\slave\sanitizer-windows\build\bin\clang.exe+0xfdcd47)
>>>
>>> clang: error: clang frontend command failed due to signal (use -v to see invocation)
>>>
>>> clang version 8.0.0 (trunk 340583)
>>>
>>> Target: i686-pc-windows-msvc
>>>
>>> Thread model: posix
>>>
>>> InstalledDir: C:\b\slave\sanitizer-windows\build\.\bin
>>>
>>> clang: note: diagnostic msg: PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.
>>>
>>> clang: note: diagnostic msg:
>>>
>>> ********************
>>>
>>>
>>> On Thu, Aug 23, 2018 at 3:36 PM David Blaikie via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Author: dblaikie
>>>> Date: Thu Aug 23 15:35:58 2018
>>>> New Revision: 340583
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=340583&view=rev
>>>> Log:
>>>> DebugInfo: Improve debug location merging
>>>>
>>>> Fix a set of related bugs:
>>>>
>>>> * Considering two locations as equivalent when their lines are the same
>>>> but their scopes are different causes erroneous debug info that
>>>> attributes a commoned call to be attributed to one of the two calls it
>>>> was commoned from.
>>>>
>>>> * The previous code to compute a new location's scope was inaccurate and
>>>> would use the inlinedAt that was the /parent/ of the inlinedAt that is
>>>> the nearest common one, and also used that parent scope instead of the
>>>> nearest common scope.
>>>>
>>>> * Not generating new locations generally seemed like a lower quality
>>>> choice
>>>>
>>>> There was some risk that generating more new locations could hurt object
>>>> size by making more fine grained line table entries, but it looks like
>>>> that was offset by the decrease in line table (& address & ranges) size
>>>> caused by more accurately computing the scope - which likely lead to
>>>> fewer range entries (more contiguous ranges) & reduced size that way.
>>>>
>>>> All up with these changes I saw minor reductions (-1.21%, -1.77%) in
>>>> .rela.debug_ranges and .rela.debug_addr (in a fission, compressed debug
>>>> info build) as well as other minor size changes (generally reductinos)
>>>> across the board (-1.32% debug_info.dwo, -1.28% debug_loc.dwo). Measured
>>>> in an optimized (-O2) build of the clang binary.
>>>>
>>>> If you are investigating a size regression in an optimized debug builds,
>>>> this is certainly a patch to look into - and I'd be happy to look into
>>>> any major regressions found & see what we can do to address them.
>>>>
>>>> Added:
>>>>     llvm/trunk/test/DebugInfo/X86/merge_inlined_loc.ll
>>>> Modified:
>>>>     llvm/trunk/include/llvm/IR/DebugInfoMetadata.h
>>>>     llvm/trunk/lib/IR/DebugInfo.cpp
>>>>     llvm/trunk/lib/IR/DebugInfoMetadata.cpp
>>>>     llvm/trunk/test/DebugInfo/COFF/local-variables.ll
>>>>
>>>> Modified: llvm/trunk/include/llvm/IR/DebugInfoMetadata.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfoMetadata.h?rev=340583&r1=340582&r2=340583&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/include/llvm/IR/DebugInfoMetadata.h (original)
>>>> +++ llvm/trunk/include/llvm/IR/DebugInfoMetadata.h Thu Aug 23 15:35:58
>>>> 2018
>>>> @@ -1479,19 +1479,6 @@ public:
>>>>      return getScope();
>>>>    }
>>>>
>>>> -  /// Check whether this can be discriminated from another location.
>>>> -  ///
>>>> -  /// Check \c this can be discriminated from \c RHS in a linetable
>>>> entry.
>>>> -  /// Scope and inlined-at chains are not recorded in the linetable,
>>>> so they
>>>> -  /// cannot be used to distinguish basic blocks.
>>>> -  bool canDiscriminate(const DILocation &RHS) const {
>>>> -    return getLine() != RHS.getLine() ||
>>>> -           getColumn() != RHS.getColumn() ||
>>>> -           getDiscriminator() != RHS.getDiscriminator() ||
>>>> -           getFilename() != RHS.getFilename() ||
>>>> -           getDirectory() != RHS.getDirectory();
>>>> -  }
>>>> -
>>>>    /// Get the DWARF discriminator.
>>>>    ///
>>>>    /// DWARF discriminators distinguish identical file locations between
>>>> @@ -1539,8 +1526,6 @@ public:
>>>>    /// discriminator.
>>>>    inline const DILocation *cloneWithDuplicationFactor(unsigned DF)
>>>> const;
>>>>
>>>> -  enum { NoGeneratedLocation = false, WithGeneratedLocation = true };
>>>> -
>>>>    /// When two instructions are combined into a single instruction we
>>>> also
>>>>    /// need to combine the original locations into a single location.
>>>>    ///
>>>> @@ -1555,9 +1540,8 @@ public:
>>>>    ///
>>>>    /// \p GenerateLocation: Whether the merged location can be
>>>> generated when
>>>>    /// \p LocA and \p LocB differ.
>>>> -  static const DILocation *
>>>> -  getMergedLocation(const DILocation *LocA, const DILocation *LocB,
>>>> -                    bool GenerateLocation = NoGeneratedLocation);
>>>> +  static const DILocation *getMergedLocation(const DILocation *LocA,
>>>> +                                             const DILocation *LocB);
>>>>
>>>>    /// Returns the base discriminator for a given encoded discriminator
>>>> \p D.
>>>>    static unsigned getBaseDiscriminatorFromDiscriminator(unsigned D) {
>>>>
>>>> Modified: llvm/trunk/lib/IR/DebugInfo.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=340583&r1=340582&r2=340583&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/IR/DebugInfo.cpp (original)
>>>> +++ llvm/trunk/lib/IR/DebugInfo.cpp Thu Aug 23 15:35:58 2018
>>>> @@ -690,8 +690,7 @@ unsigned llvm::getDebugMetadataVersionFr
>>>>
>>>>  void Instruction::applyMergedLocation(const DILocation *LocA,
>>>>                                        const DILocation *LocB) {
>>>> -  setDebugLoc(DILocation::getMergedLocation(LocA, LocB,
>>>> -
>>>> DILocation::WithGeneratedLocation));
>>>> +  setDebugLoc(DILocation::getMergedLocation(LocA, LocB));
>>>>  }
>>>>
>>>>
>>>>  //===----------------------------------------------------------------------===//
>>>>
>>>> Modified: llvm/trunk/lib/IR/DebugInfoMetadata.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfoMetadata.cpp?rev=340583&r1=340582&r2=340583&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/IR/DebugInfoMetadata.cpp (original)
>>>> +++ llvm/trunk/lib/IR/DebugInfoMetadata.cpp Thu Aug 23 15:35:58 2018
>>>> @@ -14,7 +14,7 @@
>>>>  #include "llvm/IR/DebugInfoMetadata.h"
>>>>  #include "LLVMContextImpl.h"
>>>>  #include "MetadataImpl.h"
>>>> -#include "llvm/ADT/SmallPtrSet.h"
>>>> +#include "llvm/ADT/SmallSet.h"
>>>>  #include "llvm/ADT/StringSwitch.h"
>>>>  #include "llvm/IR/DIBuilder.h"
>>>>  #include "llvm/IR/Function.h"
>>>> @@ -69,28 +69,40 @@ DILocation *DILocation::getImpl(LLVMCont
>>>>  }
>>>>
>>>>  const DILocation *DILocation::getMergedLocation(const DILocation *LocA,
>>>> -                                                const DILocation *LocB,
>>>> -                                                bool GenerateLocation)
>>>> {
>>>> +                                                const DILocation
>>>> *LocB) {
>>>>    if (!LocA || !LocB)
>>>>      return nullptr;
>>>>
>>>> -  if (LocA == LocB || !LocA->canDiscriminate(*LocB))
>>>> +  if (LocA == LocB)
>>>>      return LocA;
>>>>
>>>> -  if (!GenerateLocation)
>>>> -    return nullptr;
>>>> -
>>>>    SmallPtrSet<DILocation *, 5> InlinedLocationsA;
>>>>    for (DILocation *L = LocA->getInlinedAt(); L; L = L->getInlinedAt())
>>>>      InlinedLocationsA.insert(L);
>>>> +  SmallSet<std::pair<DIScope *, DILocation *>, 5> Locations;
>>>> +  DIScope *S = LocA->getScope();
>>>> +  DILocation *L = LocA->getInlinedAt();
>>>> +  while (S) {
>>>> +    Locations.insert(std::make_pair(S, L));
>>>> +    S = S->getScope().resolve();
>>>> +    if (!S && L) {
>>>> +      S = L->getScope();
>>>> +      L = L->getInlinedAt();
>>>> +    }
>>>> +  }
>>>>    const DILocation *Result = LocB;
>>>> -  for (DILocation *L = LocB->getInlinedAt(); L; L = L->getInlinedAt())
>>>> {
>>>> -    Result = L;
>>>> -    if (InlinedLocationsA.count(L))
>>>> +  S = LocB->getScope();
>>>> +  L = LocB->getInlinedAt();
>>>> +  while (S) {
>>>> +    if (Locations.count(std::make_pair(S, L)))
>>>>        break;
>>>> +    S = S->getScope().resolve();
>>>> +    if (!S && L) {
>>>> +      S = L->getScope();
>>>> +      L = L->getInlinedAt();
>>>> +    }
>>>>    }
>>>> -  return DILocation::get(Result->getContext(), 0, 0,
>>>> Result->getScope(),
>>>> -                         Result->getInlinedAt());
>>>> +  return DILocation::get(Result->getContext(), 0, 0, S, L);
>>>>  }
>>>>
>>>>  DINode::DIFlags DINode::getFlag(StringRef Flag) {
>>>>
>>>> Modified: llvm/trunk/test/DebugInfo/COFF/local-variables.ll
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/COFF/local-variables.ll?rev=340583&r1=340582&r2=340583&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/DebugInfo/COFF/local-variables.ll (original)
>>>> +++ llvm/trunk/test/DebugInfo/COFF/local-variables.ll Thu Aug 23
>>>> 15:35:58 2018
>>>> @@ -60,7 +60,7 @@
>>>>  ; ASM:         leaq    36(%rsp), %rcx
>>>>  ; ASM: [[else_end:\.Ltmp.*]]:
>>>>  ; ASM: .LBB0_3:                                # %if.end
>>>> -; ASM:         .cv_loc 0 1 17 1                # t.cpp:17:1
>>>> +; ASM:         .cv_loc 0 1 0 0                # t.cpp:0:0
>>>>  ; ASM:         callq   capture
>>>>  ; ASM:         nop
>>>>  ; ASM:         addq    $56, %rsp
>>>>
>>>> Added: llvm/trunk/test/DebugInfo/X86/merge_inlined_loc.ll
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/merge_inlined_loc.ll?rev=340583&view=auto
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/DebugInfo/X86/merge_inlined_loc.ll (added)
>>>> +++ llvm/trunk/test/DebugInfo/X86/merge_inlined_loc.ll Thu Aug 23
>>>> 15:35:58 2018
>>>> @@ -0,0 +1,100 @@
>>>> +; RUN: llc %s -mtriple=x86_64-unknown-unknown -o - | FileCheck %s
>>>> +
>>>> +; Generated with "clang -g -c -emit-llvm -S -O3"
>>>> +
>>>> +; This will test several features of merging debug locations.
>>>> Importantly,
>>>> +; locations with the same source line but different scopes should be
>>>> merged to
>>>> +; a line zero location at the nearest common scope and inlining. The
>>>> location
>>>> +; of the single call to "common" (the two calls are collapsed together
>>>> by
>>>> +; BranchFolding) should be attributed to line zero inside the wrapper2
>>>> inlined
>>>> +; scope within f1.
>>>> +
>>>> +; void common();
>>>> +; inline void wrapper() { common(); }
>>>> +; extern bool b;
>>>> +; void sink();
>>>> +; inline void wrapper2() {
>>>> +;   if (b) {
>>>> +;     sink();
>>>> +;     wrapper();
>>>> +;   } else
>>>> +;     wrapper();
>>>> +; }
>>>> +; void f1() { wrapper2(); }
>>>> +
>>>> +; Ensure there is only one inlined_subroutine (for wrapper2, none for
>>>> wrapper)
>>>> +; & that its address range includes the call to 'common'.
>>>> +
>>>> +; CHECK: jmp _Z6commonv
>>>> +; CHECK-NEXT: [[LABEL:.*]]:
>>>> +
>>>> +; CHECK: .section .debug_info
>>>> +; CHECK: DW_TAG_subprogram
>>>> +; CHECK: DW_TAG_subprogram
>>>> +; CHECK-NOT: {{DW_TAG\|End Of Children}}
>>>> +; CHECK:   DW_TAG_inlined_subroutine
>>>> +; CHECK-NOT: {{DW_TAG\|End Of Children}}
>>>> +; CHECK:     [[LABEL]]-{{.*}} DW_AT_high_pc
>>>> +; CHECK-NOT: DW_TAG
>>>> +
>>>> +
>>>> +
>>>> + at b = external dso_local local_unnamed_addr global i8, align 1
>>>> +
>>>> +; Function Attrs: uwtable
>>>> +define dso_local void @_Z2f1v() local_unnamed_addr !dbg !7 {
>>>> +entry:
>>>> +  %0 = load i8, i8* @b, align 1, !dbg !10, !tbaa !14, !range !18
>>>> +  %tobool.i = icmp eq i8 %0, 0, !dbg !10
>>>> +  br i1 %tobool.i, label %if.else.i, label %if.then.i, !dbg !19
>>>> +
>>>> +if.then.i:                                        ; preds = %entry
>>>> +  tail call void @_Z4sinkv(), !dbg !20
>>>> +  tail call void @_Z6commonv(), !dbg !22
>>>> +  br label %_Z8wrapper2v.exit, !dbg !25
>>>> +
>>>> +if.else.i:                                        ; preds = %entry
>>>> +  tail call void @_Z6commonv(), !dbg !26
>>>> +  br label %_Z8wrapper2v.exit
>>>> +
>>>> +_Z8wrapper2v.exit:                                ; preds =
>>>> %if.then.i, %if.else.i
>>>> +  ret void, !dbg !28
>>>> +}
>>>> +
>>>> +declare dso_local void @_Z4sinkv() local_unnamed_addr
>>>> +
>>>> +declare dso_local void @_Z6commonv() local_unnamed_addr
>>>> +
>>>> +!llvm.dbg.cu = !{!0}
>>>> +!llvm.module.flags = !{!3, !4, !5}
>>>> +!llvm.ident = !{!6}
>>>> +
>>>> +!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1,
>>>> producer: "clang version 8.0.0 (trunk 340559) (llvm/trunk 340572)",
>>>> isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2,
>>>> nameTableKind: None)
>>>> +!1 = !DIFile(filename: "merge_loc.cpp", directory:
>>>> "/usr/local/google/home/blaikie/dev/scratch")
>>>> +!2 = !{}
>>>> +!3 = !{i32 2, !"Dwarf Version", i32 4}
>>>> +!4 = !{i32 2, !"Debug Info Version", i32 3}
>>>> +!5 = !{i32 1, !"wchar_size", i32 4}
>>>> +!6 = !{!"clang version 8.0.0 (trunk 340559) (llvm/trunk 340572)"}
>>>> +!7 = distinct !DISubprogram(name: "f1", linkageName: "_Z2f1v", scope:
>>>> !1, file: !1, line: 12, type: !8, isLocal: false, isDefinition: true,
>>>> scopeLine: 12, flags: DIFlagPrototyped, isOptimized: true, unit: !0,
>>>> retainedNodes: !2)
>>>> +!8 = !DISubroutineType(types: !9)
>>>> +!9 = !{null}
>>>> +!10 = !DILocation(line: 6, column: 7, scope: !11, inlinedAt: !13)
>>>> +!11 = distinct !DILexicalBlock(scope: !12, file: !1, line: 6, column:
>>>> 7)
>>>> +!12 = distinct !DISubprogram(name: "wrapper2", linkageName:
>>>> "_Z8wrapper2v", scope: !1, file: !1, line: 5, type: !8, isLocal: false,
>>>> isDefinition: true, scopeLine: 5, flags: DIFlagPrototyped, isOptimized:
>>>> true, unit: !0, retainedNodes: !2)
>>>> +!13 = distinct !DILocation(line: 13, column: 3, scope: !7)
>>>> +!14 = !{!15, !15, i64 0}
>>>> +!15 = !{!"bool", !16, i64 0}
>>>> +!16 = !{!"omnipotent char", !17, i64 0}
>>>> +!17 = !{!"Simple C++ TBAA"}
>>>> +!18 = !{i8 0, i8 2}
>>>> +!19 = !DILocation(line: 6, column: 7, scope: !12, inlinedAt: !13)
>>>> +!20 = !DILocation(line: 7, column: 5, scope: !21, inlinedAt: !13)
>>>> +!21 = distinct !DILexicalBlock(scope: !11, file: !1, line: 6, column:
>>>> 10)
>>>> +!22 = !DILocation(line: 2, column: 25, scope: !23, inlinedAt: !24)
>>>> +!23 = distinct !DISubprogram(name: "wrapper", linkageName:
>>>> "_Z7wrapperv", scope: !1, file: !1, line: 2, type: !8, isLocal: false,
>>>> isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped, isOptimized:
>>>> true, unit: !0, retainedNodes: !2)
>>>> +!24 = distinct !DILocation(line: 8, column: 5, scope: !21, inlinedAt:
>>>> !13)
>>>> +!25 = !DILocation(line: 9, column: 3, scope: !21, inlinedAt: !13)
>>>> +!26 = !DILocation(line: 2, column: 25, scope: !23, inlinedAt: !27)
>>>> +!27 = distinct !DILocation(line: 10, column: 5, scope: !11, inlinedAt:
>>>> !13)
>>>> +!28 = !DILocation(line: 14, column: 1, scope: !7)
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180827/b275cd55/attachment.html>


More information about the llvm-commits mailing list