[llvm] r191407 - Unify pubsection/gnu pubsection printing.

[David Blaikie]

On Wed, Sep 25, 2013 at 4:02 PM, Eric Christopher <echristo at gmail.com>wrote:

> Author: echristo
> Date: Wed Sep 25 18:02:36 2013
> New Revision: 191407
>
> URL: http://llvm.org/viewvc/llvm-project?rev=191407&view=rev
> Log:
> Unify pubsection/gnu pubsection printing.
>
> Modified:
>     llvm/trunk/lib/DebugInfo/DWARFContext.cpp
>     llvm/trunk/test/DebugInfo/dwarfdump-pubnames.test
>
> Modified: llvm/trunk/lib/DebugInfo/DWARFContext.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARFContext.cpp?rev=191407&r1=191406&r2=191407&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/DebugInfo/DWARFContext.cpp (original)
> +++ llvm/trunk/lib/DebugInfo/DWARFContext.cpp Wed Sep 25 18:02:36 2013
> @@ -29,7 +29,7 @@ DWARFContext::~DWARFContext() {
>  }
>
>  static void dumpPubSection(raw_ostream &OS, StringRef Name, StringRef
> Data,
> -                           bool LittleEndian) {
> +                           bool LittleEndian, bool GnuStyle) {
>    OS << "\n." << Name << " contents:\n";
>    DataExtractor pubNames(Data, LittleEndian, 0);
>    uint32_t offset = 0;
> @@ -37,16 +37,25 @@ static void dumpPubSection(raw_ostream &
>    OS << "Version:               " << pubNames.getU16(&offset) << "\n";
>    OS << "Offset in .debug_info: " << pubNames.getU32(&offset) << "\n";
>    OS << "Size:                  " << pubNames.getU32(&offset) << "\n";
> -  OS << "Offset     Linkage  Kind     Name\n";
> +  if (GnuStyle)
> +    OS << "Offset     Linkage  Kind     Name\n";
> +  else
> +    OS << "Offset        Name\n";
> +
>    while (offset < Data.size()) {
>      uint32_t dieRef = pubNames.getU32(&offset);
>      if (dieRef == 0)
>        break;
> -    PubIndexEntryDescriptor desc(pubNames.getU8(&offset));
> -    OS << format("0x%8.8x ", dieRef)
> -       << format("%-8s", dwarf::GDBIndexEntryLinkageString(desc.Linkage))
> << ' '
> -       << format("%-8s", dwarf::GDBIndexEntryKindString(desc.Kind)) << ' '
> -       << '\"' << pubNames.getCStr(&offset) << "\"\n";
> +    if (GnuStyle) {
>

The two conditions here are basically the same for the Offset and Name
fields, except for two extra spaces of indent (and whether or not the name
is quoted?). Should we just make them actually the same & conditionalize
the extra field like this:

OS << format("0x%8.8x ", dieRef)
if (GnuStyle) {
  PubIndexEntryDescriptor desc(pubNames.getU8(&offset));
  OS << format("%-8s ", dwarf::GDBIndexEntryLinkageString(desc.Linkage))
        << format("%-8s ", dwarf::GDBIndexEntryKindString(desc.Kind));
}
OS << '\"' << pubNames.getCStr(&offset) << "\"\n";

(alternatively, rather than hiding the spaces at the end of the format
strings, they could be printed at the beginning of the field that requires
them - I don't feel strongly either way)


> +      PubIndexEntryDescriptor desc(pubNames.getU8(&offset));
> +      OS << format("0x%8.8x ", dieRef)
> +         << format("%-8s",
> dwarf::GDBIndexEntryLinkageString(desc.Linkage))
> +         << ' ' << format("%-8s",
> dwarf::GDBIndexEntryKindString(desc.Kind))
> +         << ' ' << '\"' << pubNames.getCStr(&offset) << "\"\n";
> +    } else {
> +      OS << format("0x%8.8x    ", dieRef);
> +      OS << pubNames.getCStr(&offset) << "\n";
> +    }
>    }
>  }
>
> @@ -130,31 +139,17 @@ void DWARFContext::dump(raw_ostream &OS,
>        rangeList.dump(OS);
>    }
>
> -  if (DumpType == DIDT_All || DumpType == DIDT_Pubnames) {
> -    OS << "\n.debug_pubnames contents:\n";
> -    DataExtractor pubNames(getPubNamesSection(), isLittleEndian(), 0);
> -    offset = 0;
> -    OS << "Length:                " << pubNames.getU32(&offset) << "\n";
> -    OS << "Version:               " << pubNames.getU16(&offset) << "\n";
> -    OS << "Offset in .debug_info: " << pubNames.getU32(&offset) << "\n";
> -    OS << "Size:                  " << pubNames.getU32(&offset) << "\n";
> -    OS << "\n  Offset    Name\n";
> -    while (offset < getPubNamesSection().size()) {
> -      uint32_t n = pubNames.getU32(&offset);
> -      if (n == 0)
> -        break;
> -      OS << format("%8x    ", n);
> -      OS << pubNames.getCStr(&offset) << "\n";
> -    }
> -  }
> +  if (DumpType == DIDT_All || DumpType == DIDT_Pubnames)
> +    dumpPubSection(OS, "debug_pubnames", getPubNamesSection(),
> +                   isLittleEndian(), false);
>
>    if (DumpType == DIDT_All || DumpType == DIDT_GnuPubnames)
>      dumpPubSection(OS, "debug_gnu_pubnames", getGnuPubNamesSection(),
> -                   isLittleEndian());
> +                   isLittleEndian(), true /* GnuStyle */);
>

If you feel the need to comment these parameters (which I don't disagree
with, as we've discussed) - perhaps just introduce a trivial two-state enum?


>
>    if (DumpType == DIDT_All || DumpType == DIDT_GnuPubtypes)
>      dumpPubSection(OS, "debug_gnu_pubtypes", getGnuPubTypesSection(),
> -                   isLittleEndian());
> +                   isLittleEndian(), true /* GnuStyle */);
>
>    if (DumpType == DIDT_All || DumpType == DIDT_AbbrevDwo) {
>      const DWARFDebugAbbrev *D = getDebugAbbrevDWO();
>
> Modified: llvm/trunk/test/DebugInfo/dwarfdump-pubnames.test
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/dwarfdump-pubnames.test?rev=191407&r1=191406&r2=191407&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/dwarfdump-pubnames.test (original)
> +++ llvm/trunk/test/DebugInfo/dwarfdump-pubnames.test Wed Sep 25 18:02:36
> 2013
> @@ -7,10 +7,11 @@ CHECK: Version:               2
>  CHECK: Offset in .debug_info: 0
>  CHECK: Size:                  321
>
> -CHECK:  Offset    Name
> -CHECK:      98    global_namespace_variable
> -CHECK:      a7    global_namespace_function
> -CHECK:      ec    static_member_function
> -CHECK:      7c    global_variable
> -CHECK:     103    global_function
> -CHECK:      c2    member_function
> +CHECK: Offset        Name
> +CHECK: 0x00000098    "global_namespace_variable"
>

Your change looked like it didn't include quoting here - but perhaps that
was added in one of your follow-on changes.


> +CHECK: 0x000000a7    "global_namespace_function"
> +CHECK: 0x000000ec    "static_member_function"
> +CHECK: 0x0000007c    "global_variable"
> +CHECK: 0x00000103    "global_function"
> +CHECK: 0x000000c2    "member_function"
> +
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130925/6e017191/attachment.html>