[llvm] 325f760 - Revert "[DWARFYAML][debug_line] Replace `InitialLength` with `Format` and `Length`."

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 16 09:46:46 PDT 2020


On Mon, Jun 15, 2020 at 9:24 PM Xing GUO <higuoxing at gmail.com> wrote:
>
> On 6/16/20, David Blaikie <dblaikie at gmail.com> wrote:
> > Please include information about why a patch was reverted in the
> > commit message for the revert - this helps other folks following along
> > (if they're seeing local breakage it'll help them know if this is the
> > source and the revert is the fix they need to pick up - or if they're
> > interested in adding similar functionality at another time, they'll be
> > able to see the history of what happened when it's been tried in the
> > past, etc).
>
> Hi David,
>
> Sorry for the un-commented changes. The previous change
> fcc0c186e9cea0af644581069058f0e00469d20e
> (https://reviews.llvm.org/rGfcc0c186e9cea0af644581069058f0e00469d20e)
> intends to make the representation of DWARF sections clearer in YAML
> (We have already done it in other DWARF sections, See:
> https://reviews.llvm.org/D81063).
>
> But the change makes build bots unhappy. So I reverted it in
> 325f7607b0dd0015a5f8db445e165660f9372bf6
> (https://reviews.llvm.org/rG325f7607b0dd0015a5f8db445e165660f9372bf6)
> and re-commit it in 0431e4bcb27bba30156ac49be4c09ac632c5a03a
> (https://reviews.llvm.org/rG0431e4bcb27bba30156ac49be4c09ac632c5a03a).
> I've added a comment in the revert change
> (https://reviews.llvm.org/rG325f7607b0dd0015a5f8db445e165660f9372bf6).
>
> I'll add sufficient commit message next time. Sorry for the inconvenience.

Thanks!

Yeah, the ideal flow here would be to include a quote from and link to
the buildbot in the revert commit (in addition to the usual "Revert:
<title> \ this reverts <hash>" content) and then in the recommit,
including the original commit hash and the revert commit hash (so
folks can see the trail) and either the same buildbot details or a
shorter summary of it - including how the issue that motivated the
revert was addressed, and what extra testing was done to ensure that
class of issues has been validated (eg: if it failed in an optimized
build but you'd only previously tested the patch in a non-optimized
build, it may be worthwhile to test an optimized build to see if there
are any other similar issues hiding in there before recommitting - and
explaining in the commit that that extra testing has been done).

- Dave

>
> > On Sat, Jun 13, 2020 at 2:53 AM Xing GUO via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >>
> >>
> >> Author: Xing GUO
> >> Date: 2020-06-13T17:57:02+08:00
> >> New Revision: 325f7607b0dd0015a5f8db445e165660f9372bf6
> >>
> >> URL:
> >> https://github.com/llvm/llvm-project/commit/325f7607b0dd0015a5f8db445e165660f9372bf6
> >> DIFF:
> >> https://github.com/llvm/llvm-project/commit/325f7607b0dd0015a5f8db445e165660f9372bf6.diff
> >>
> >> LOG: Revert "[DWARFYAML][debug_line] Replace `InitialLength` with `Format`
> >> and `Length`."
> >>
> >> This reverts commit fcc0c186e9cea0af644581069058f0e00469d20e.
> >>
> >> Added:
> >>
> >>
> >> Modified:
> >>     llvm/include/llvm/ObjectYAML/DWARFYAML.h
> >>     llvm/lib/ObjectYAML/DWARFEmitter.cpp
> >>     llvm/lib/ObjectYAML/DWARFYAML.cpp
> >>     llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
> >>     llvm/test/ObjectYAML/MachO/DWARF-debug_line.yaml
> >>     llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
> >>     llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml
> >>     llvm/tools/obj2yaml/dwarf2yaml.cpp
> >>
> >> Removed:
> >>
> >>
> >>
> >> ################################################################################
> >> diff  --git a/llvm/include/llvm/ObjectYAML/DWARFYAML.h
> >> b/llvm/include/llvm/ObjectYAML/DWARFYAML.h
> >> index 08b02691ffc1..2f355b2a5b59 100644
> >> --- a/llvm/include/llvm/ObjectYAML/DWARFYAML.h
> >> +++ b/llvm/include/llvm/ObjectYAML/DWARFYAML.h
> >> @@ -144,8 +144,7 @@ struct LineTableOpcode {
> >>  };
> >>
> >>  struct LineTable {
> >> -  dwarf::DwarfFormat Format;
> >> -  uint64_t Length;
> >> +  InitialLength Length;
> >>    uint16_t Version;
> >>    uint64_t PrologueLength;
> >>    uint8_t MinInstLength;
> >>
> >> diff  --git a/llvm/lib/ObjectYAML/DWARFEmitter.cpp
> >> b/llvm/lib/ObjectYAML/DWARFEmitter.cpp
> >> index 9ab6aa5aeafc..b496e2a09386 100644
> >> --- a/llvm/lib/ObjectYAML/DWARFEmitter.cpp
> >> +++ b/llvm/lib/ObjectYAML/DWARFEmitter.cpp
> >> @@ -262,9 +262,8 @@ static void emitFileEntry(raw_ostream &OS, const
> >> DWARFYAML::File &File) {
> >>
> >>  Error DWARFYAML::emitDebugLine(raw_ostream &OS, const DWARFYAML::Data
> >> &DI) {
> >>    for (const auto &LineTable : DI.DebugLines) {
> >> -    writeInitialLength(LineTable.Format, LineTable.Length, OS,
> >> -                       DI.IsLittleEndian);
> >> -    uint64_t SizeOfPrologueLength = LineTable.Format == dwarf::DWARF64 ?
> >> 8 : 4;
> >> +    writeInitialLength(LineTable.Length, OS, DI.IsLittleEndian);
> >> +    uint64_t SizeOfPrologueLength = LineTable.Length.isDWARF64() ? 8 :
> >> 4;
> >>      writeInteger((uint16_t)LineTable.Version, OS, DI.IsLittleEndian);
> >>      writeVariableSizedInteger(LineTable.PrologueLength,
> >> SizeOfPrologueLength,
> >>                                OS, DI.IsLittleEndian);
> >>
> >> diff  --git a/llvm/lib/ObjectYAML/DWARFYAML.cpp
> >> b/llvm/lib/ObjectYAML/DWARFYAML.cpp
> >> index 8298047d8be9..257db357ca73 100644
> >> --- a/llvm/lib/ObjectYAML/DWARFYAML.cpp
> >> +++ b/llvm/lib/ObjectYAML/DWARFYAML.cpp
> >> @@ -175,7 +175,6 @@ void
> >> MappingTraits<DWARFYAML::LineTableOpcode>::mapping(
> >>
> >>  void MappingTraits<DWARFYAML::LineTable>::mapping(
> >>      IO &IO, DWARFYAML::LineTable &LineTable) {
> >> -  IO.mapOptional("Format", LineTable.Format, dwarf::DWARF32);
> >>    IO.mapRequired("Length", LineTable.Length);
> >>    IO.mapRequired("Version", LineTable.Version);
> >>    IO.mapRequired("PrologueLength", LineTable.PrologueLength);
> >>
> >> diff  --git a/llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
> >> b/llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
> >> index af997b010b2d..95f3eae597c2 100644
> >> --- a/llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
> >> +++ b/llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
> >> @@ -453,7 +453,8 @@ DWARF:
> >>          - AbbrCode:        0x00000000
> >>            Values:
> >>    debug_line:
> >> -    - Length:          65
> >> +    - Length:
> >> +        TotalLength:     65
> >>        Version:         2
> >>        PrologueLength:  36
> >>        MinInstLength:   1
> >>
> >> diff  --git a/llvm/test/ObjectYAML/MachO/DWARF-debug_line.yaml
> >> b/llvm/test/ObjectYAML/MachO/DWARF-debug_line.yaml
> >> index d24fe7c8a4b8..5d17deb2fac2 100644
> >> --- a/llvm/test/ObjectYAML/MachO/DWARF-debug_line.yaml
> >> +++ b/llvm/test/ObjectYAML/MachO/DWARF-debug_line.yaml
> >> @@ -492,7 +492,8 @@ DWARF:
> >>          - AbbrCode:        0x00000000
> >>            Values:
> >>    debug_line:
> >> -    - Length:          65
> >> +    - Length:
> >> +        TotalLength:     65
> >>        Version:         2
> >>        PrologueLength:  36
> >>        MinInstLength:   1
> >> @@ -533,7 +534,8 @@ DWARF:
> >>  ...
> >>
> >>  #CHECK:   debug_line:
> >> -#CHECK:     - Length:          65
> >> +#CHECK:     - Length:
> >> +#CHECK:         TotalLength:     65
> >>  #CHECK:       Version:         2
> >>  #CHECK:       PrologueLength:  36
> >>  #CHECK:       MinInstLength:   1
> >>
> >> diff  --git a/llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
> >> b/llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
> >> index 9e8865f3c08e..574796cbebda 100644
> >> --- a/llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
> >> +++ b/llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
> >> @@ -454,7 +454,8 @@ DWARF:
> >>          - AbbrCode:        0x00000000
> >>            Values:
> >>    debug_line:
> >> -    - Length:          65
> >> +    - Length:
> >> +        TotalLength:     65
> >>        Version:         2
> >>        PrologueLength:  36
> >>        MinInstLength:   1
> >>
> >> diff  --git a/llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml
> >> b/llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml
> >> index 020fadc57b34..22d5584351ac 100644
> >> --- a/llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml
> >> +++ b/llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml
> >> @@ -72,7 +72,8 @@ FileHeader:
> >>    Machine: EM_X86_64
> >>  DWARF:
> >>    debug_line:
> >> -    - Length:         0x70
> >> +    - Length:
> >> +        TotalLength:  0x70
> >>        Version:        2
> >>        PrologueLength: 50
> >>        MinInstLength:  1
> >> @@ -206,7 +207,8 @@ Sections:
> >>      Size: 0x10
> >>  DWARF:
> >>    debug_line:
> >> -    - Length:                0x70
> >> +    - Length:
> >> +        TotalLength:         0x70
> >>        Version:               2
> >>        PrologueLength:        50
> >>        MinInstLength:         1
> >> @@ -236,7 +238,8 @@ Sections:
> >>      Content: "00"
> >>  DWARF:
> >>    debug_line:
> >> -    - Length:                0x70
> >> +    - Length:
> >> +        TotalLength:         0x70
> >>        Version:               2
> >>        PrologueLength:        50
> >>        MinInstLength:         1
> >> @@ -305,7 +308,8 @@ Sections:
> >>      Type:         SHT_STRTAB
> >>  DWARF:
> >>    debug_line:
> >> -    - Length:                0x70
> >> +    - Length:
> >> +        TotalLength:         0x70
> >>        Version:               2
> >>        PrologueLength:        50
> >>        MinInstLength:         1
> >>
> >> diff  --git a/llvm/tools/obj2yaml/dwarf2yaml.cpp
> >> b/llvm/tools/obj2yaml/dwarf2yaml.cpp
> >> index 88f53c18f30b..12cb0c294d4a 100644
> >> --- a/llvm/tools/obj2yaml/dwarf2yaml.cpp
> >> +++ b/llvm/tools/obj2yaml/dwarf2yaml.cpp
> >> @@ -7,7 +7,6 @@
> >>
> >> //===----------------------------------------------------------------------===//
> >>
> >>  #include "Error.h"
> >> -#include "llvm/BinaryFormat/Dwarf.h"
> >>  #include "llvm/DebugInfo/DWARF/DWARFContext.h"
> >>  #include "llvm/DebugInfo/DWARF/DWARFDebugArangeSet.h"
> >>  #include "llvm/DebugInfo/DWARF/DWARFDebugRangeList.h"
> >> @@ -297,17 +296,9 @@ void dumpDebugLines(DWARFContext &DCtx,
> >> DWARFYAML::Data &Y) {
> >>        DataExtractor LineData(DCtx.getDWARFObj().getLineSection().Data,
> >>                               DCtx.isLittleEndian(),
> >> CU->getAddressByteSize());
> >>        uint64_t Offset = *StmtOffset;
> >> -      uint64_t LengthOrDWARF64Prefix = LineData.getU32(&Offset);
> >> -      if (LengthOrDWARF64Prefix == dwarf::DW_LENGTH_DWARF64) {
> >> -        DebugLines.Format = dwarf::DWARF64;
> >> -        DebugLines.Length = LineData.getU64(&Offset);
> >> -      } else {
> >> -        DebugLines.Format = dwarf::DWARF32;
> >> -        DebugLines.Length = LengthOrDWARF64Prefix;
> >> -      }
> >> -      uint64_t LineTableLength = DebugLines.Length;
> >> -      uint64_t SizeOfPrologueLength =
> >> -          DebugLines.Format == dwarf::DWARF64 ? 8 : 4;
> >> +      dumpInitialLength(LineData, Offset, DebugLines.Length);
> >> +      uint64_t LineTableLength = DebugLines.Length.getLength();
> >> +      uint64_t SizeOfPrologueLength = DebugLines.Length.isDWARF64() ? 8 :
> >> 4;
> >>        DebugLines.Version = LineData.getU16(&Offset);
> >>        DebugLines.PrologueLength =
> >>            LineData.getUnsigned(&Offset, SizeOfPrologueLength);
> >>
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
>
>
> --
> Cheers,
> Xing


More information about the llvm-commits mailing list