[llvm] r374931 - [llvm-objdump] Use a counter for llvm-objdump -h instead of the section index.

Jordan Rupprecht via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 17 10:35:57 PDT 2019


Well, that's weird. Thanks for reverting!

It's probably some weird corruption with the format string (uint64_t passed
to "%d"). I'll try to reland it with some changes and watch the 32bit build
bots.

On Thu, Oct 17, 2019 at 1:50 AM Hans Wennborg <hans at chromium.org> wrote:

> This somehow broke a bunch of tests in 32-bit builds, e.g.
> http://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/10925
>
> I've reverted in r375088.
>
> On Tue, Oct 15, 2019 at 8:10 PM Jordan Rupprecht via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> >
> > Author: rupprecht
> > Date: Tue Oct 15 11:13:20 2019
> > New Revision: 374931
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=374931&view=rev
> > Log:
> > [llvm-objdump] Use a counter for llvm-objdump -h instead of the section
> index.
> >
> > Summary:
> > When listing the index in `llvm-objdump -h`, use a zero-based counter
> instead of the actual section index (e.g. shdr->sh_index for ELF).
> >
> > While this is effectively a noop for now (except one unit test for
> XCOFF), the index values will change in a future patch that filters certain
> sections out (e.g. symbol tables). See D68669 for more context. Note: the
> test case in `test/tools/llvm-objdump/X86/section-index.s` already covers
> the case of incrementing the section index counter when sections are
> skipped.
> >
> > Reviewers: grimar, jhenderson, espindola
> >
> > Reviewed By: grimar
> >
> > Subscribers: emaste, sbc100, arichardson, aheejin, arphaman, seiya,
> llvm-commits, MaskRay
> >
> > Tags: #llvm
> >
> > Differential Revision: https://reviews.llvm.org/D68848
> >
> > Modified:
> >     llvm/trunk/test/tools/llvm-objdump/xcoff-section-headers.test
> >     llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp
> >     llvm/trunk/tools/llvm-objdump/llvm-objdump.h
> >
> > Modified: llvm/trunk/test/tools/llvm-objdump/xcoff-section-headers.test
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/xcoff-section-headers.test?rev=374931&r1=374930&r2=374931&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/tools/llvm-objdump/xcoff-section-headers.test
> (original)
> > +++ llvm/trunk/test/tools/llvm-objdump/xcoff-section-headers.test Tue
> Oct 15 11:13:20 2019
> > @@ -13,11 +13,11 @@
> >  # CHECK: xcoff-section-headers.o:      file format aixcoff-rs6000
> >  # CHECK: Sections:
> >  # CHECK: Idx Name          Size     VMA      Type
> > -# CHECK:   1 .text         00000080 00000000 TEXT
> > -# CHECK:   2 .data         00000024 00000080 DATA
> > -# CHECK:   3 .bss          00000004 000000a4 BSS
> > -# CHECK:   4 .tdata        00000008 00000000 DATA
> > -# CHECK:   5 .tbss         00000004 00000008 BSS
> > +# CHECK:   0 .text         00000080 00000000 TEXT
> > +# CHECK:   1 .data         00000024 00000080 DATA
> > +# CHECK:   2 .bss          00000004 000000a4 BSS
> > +# CHECK:   3 .tdata        00000008 00000000 DATA
> > +# CHECK:   4 .tbss         00000004 00000008 BSS
> >
> >  # xcoff-section-headers.o Compiled with IBM XL C/C++ for AIX, V16.1.0
> >  # test.c:
> > @@ -32,10 +32,10 @@
> >
> >  # LONG: xcoff-long-sec-names.o:      file format aixcoff-rs6000
> >  # LONG: Sections:
> > -# LONG: Idx Name        Size     VMA      Type
> > -# LONG: 1 .dwarnge      00000004 00000000
> > -# LONG: 2 .dwpbnms      00000004 00000000
> > -# LONG: 3 .dwpbtyp      00000004 00000000
> > +# LONG: Idx Name          Size     VMA      Type
> > +# LONG:   0 .dwarnge      00000004 00000000
> > +# LONG:   1 .dwpbnms      00000004 00000000
> > +# LONG:   2 .dwpbtyp      00000004 00000000
> >
> >  # xcoff-long-sec-names.o was generated by assembling the following .s
> file:
> >  #  .dwsect 0x30000 # .dwpbnms section
> >
> > Modified: llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp?rev=374931&r1=374930&r2=374931&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp (original)
> > +++ llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp Tue Oct 15 11:13:20
> 2019
> > @@ -342,14 +342,27 @@ static StringRef ToolName;
> >
> >  typedef std::vector<std::tuple<uint64_t, StringRef, uint8_t>>
> SectionSymbolsTy;
> >
> > -static bool shouldKeep(object::SectionRef S) {
> > +namespace {
> > +struct FilterResult {
> > +  // True if the section should not be skipped.
> > +  bool Keep;
> > +
> > +  // True if the index counter should be incremented, even if the
> section should
> > +  // be skipped. For example, sections may be skipped if they are not
> included
> > +  // in the --section flag, but we still want those to count toward the
> section
> > +  // count.
> > +  bool IncrementIndex;
> > +};
> > +} // namespace
> > +
> > +static FilterResult checkSectionFilter(object::SectionRef S) {
> >    if (FilterSections.empty())
> > -    return true;
> > +    return {/*Keep=*/true, /*IncrementIndex=*/true};
> >
> >    Expected<StringRef> SecNameOrErr = S.getName();
> >    if (!SecNameOrErr) {
> >      consumeError(SecNameOrErr.takeError());
> > -    return false;
> > +    return {/*Keep=*/false, /*IncrementIndex=*/false};
> >    }
> >    StringRef SecName = *SecNameOrErr;
> >
> > @@ -357,11 +370,26 @@ static bool shouldKeep(object::SectionRe
> >    // no name (such as the section with index 0) here.
> >    if (!SecName.empty())
> >      FoundSectionSet.insert(SecName);
> > -  return is_contained(FilterSections, SecName);
> > -}
> >
> > -SectionFilter ToolSectionFilter(object::ObjectFile const &O) {
> > -  return SectionFilter([](object::SectionRef S) { return shouldKeep(S);
> }, O);
> > +  // Only show the section if it's in the FilterSections list, but
> always
> > +  // increment so the indexing is stable.
> > +  return {/*Keep=*/is_contained(FilterSections, SecName),
> > +          /*IncrementIndex=*/true};
> > +}
> > +
> > +SectionFilter ToolSectionFilter(object::ObjectFile const &O, uint64_t
> *Idx) {
> > +  // Start at UINT64_MAX so that the first index returned after an
> increment is
> > +  // zero (after the unsigned wrap).
> > +  if (Idx)
> > +    *Idx = UINT64_MAX;
> > +  return SectionFilter(
> > +      [Idx](object::SectionRef S) {
> > +        FilterResult Result = checkSectionFilter(S);
> > +        if (Idx != nullptr && Result.IncrementIndex)
> > +          *Idx += 1;
> > +        return Result.Keep;
> > +      },
> > +      O);
> >  }
> >
> >  std::string getFileNameForError(const object::Archive::Child &C,
> > @@ -967,7 +995,7 @@ getRelocsMap(object::ObjectFile const &O
> >    std::map<SectionRef, std::vector<RelocationRef>> Ret;
> >    for (SectionRef Sec : Obj.sections()) {
> >      section_iterator Relocated = Sec.getRelocatedSection();
> > -    if (Relocated == Obj.section_end() || !shouldKeep(*Relocated))
> > +    if (Relocated == Obj.section_end() ||
> !checkSectionFilter(*Relocated).Keep)
> >        continue;
> >      std::vector<RelocationRef> &V = Ret[*Relocated];
> >      for (const RelocationRef &R : Sec.relocations())
> > @@ -1676,7 +1704,8 @@ void printSectionHeaders(const ObjectFil
> >             << left_justify("Name", NameWidth) << " Size     "
> >             << left_justify("VMA", AddressWidth) << " Type\n";
> >
> > -  for (const SectionRef &Section : ToolSectionFilter(*Obj)) {
> > +  uint64_t Idx;
> > +  for (const SectionRef &Section : ToolSectionFilter(*Obj, &Idx)) {
> >      StringRef Name = unwrapOrError(Section.getName(),
> Obj->getFileName());
> >      uint64_t VMA = Section.getAddress();
> >      if (shouldAdjustVA(Section))
> > @@ -1691,14 +1720,14 @@ void printSectionHeaders(const ObjectFil
> >        Type += Type.empty() ? "BSS" : " BSS";
> >
> >      if (HasLMAColumn)
> > -      outs() << format("%3d %-*s %08" PRIx64 " ",
> (unsigned)Section.getIndex(),
> > -                       NameWidth, Name.str().c_str(), Size)
> > +      outs() << format("%3d %-*s %08" PRIx64 " ", Idx, NameWidth,
> > +                       Name.str().c_str(), Size)
> >               << format_hex_no_prefix(VMA, AddressWidth) << " "
> >               << format_hex_no_prefix(getELFSectionLMA(Section),
> AddressWidth)
> >               << " " << Type << "\n";
> >      else
> > -      outs() << format("%3d %-*s %08" PRIx64 " ",
> (unsigned)Section.getIndex(),
> > -                       NameWidth, Name.str().c_str(), Size)
> > +      outs() << format("%3d %-*s %08" PRIx64 " ", Idx, NameWidth,
> > +                       Name.str().c_str(), Size)
> >               << format_hex_no_prefix(VMA, AddressWidth) << " " << Type
> << "\n";
> >    }
> >    outs() << "\n";
> >
> > Modified: llvm/trunk/tools/llvm-objdump/llvm-objdump.h
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/llvm-objdump.h?rev=374931&r1=374930&r2=374931&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/tools/llvm-objdump/llvm-objdump.h (original)
> > +++ llvm/trunk/tools/llvm-objdump/llvm-objdump.h Tue Oct 15 11:13:20 2019
> > @@ -31,6 +31,8 @@ extern cl::opt<bool> Demangle;
> >
> >  typedef std::function<bool(llvm::object::SectionRef const &)>
> FilterPredicate;
> >
> > +/// A filtered iterator for SectionRefs that skips sections based on
> some given
> > +/// predicate.
> >  class SectionFilterIterator {
> >  public:
> >    SectionFilterIterator(FilterPredicate P,
> > @@ -60,6 +62,8 @@ private:
> >    llvm::object::section_iterator End;
> >  };
> >
> > +/// Creates an iterator range of SectionFilterIterators for a given
> Object and
> > +/// predicate.
> >  class SectionFilter {
> >  public:
> >    SectionFilter(FilterPredicate P, llvm::object::ObjectFile const &O)
> > @@ -79,7 +83,15 @@ private:
> >  };
> >
> >  // Various helper functions.
> > -SectionFilter ToolSectionFilter(llvm::object::ObjectFile const &O);
> > +
> > +/// Creates a SectionFilter with a standard predicate that
> conditionally skips
> > +/// sections when the --section objdump flag is provided.
> > +///
> > +/// Idx is an optional output parameter that keeps track of which
> section index
> > +/// this is. This may be different than the actual section number, as
> some
> > +/// sections may be filtered (e.g. symbol tables).
> > +SectionFilter ToolSectionFilter(llvm::object::ObjectFile const &O,
> > +                                uint64_t *Idx = nullptr);
> >
> >  Error getELFRelocationValueString(const object::ELFObjectFileBase *Obj,
> >                                    const object::RelocationRef &Rel,
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > https://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/20191017/55677593/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4849 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191017/55677593/attachment.bin>


More information about the llvm-commits mailing list