[llvm-objdump] Suppress spurious warnings when parsing Mach-O binaries.

[mtrent]

Summary

llvm-objdump started warning when asked to disassemble a section that
isn't present in the input files, in Yuanfang Chen's change:
d16c162c9453db855503134fe29ae4a3c0bec936. The problem is that the
logic was restricted only to the generic llvm-objdump parser, not to the
Mach-O-specific parser used for Apple toolchain compatibility. The
solution is to log section names from the Mach-O parser.

The macho-cstring-dump.test has been updated to fail if it encounters
this new warning in the future.

[grimar]

llvm/test/tools/llvm-objdump/X86/macho-cstring-dump.test

5

I'd suggest using --implicit-check-not="warning:" FileCheck option instead. Testing the whole message is too brittle approach I think.

llvm/tools/llvm-objdump/MachODump.cpp

1753

Why this line was changed?

[grimar]

Also we usually add a corresponding tag to the patch name:
"Suppress spurious warnings when parsing Mach-O bianries." -> "[llvm-readobj] - Suppress spurious warnings when parsing Mach-O bianries."

+ "bianries" -> "binaries"

[jhenderson]

llvm/test/tools/llvm-objdump/X86/macho-cstring-dump.test

5

+1. It also means you only need to specify the pattern in one place, rather than the current two.

[mtrent]

llvm/test/tools/llvm-objdump/X86/macho-cstring-dump.test

5

Sounds good, I'll update the patch.

llvm/tools/llvm-objdump/MachODump.cpp

1753

Looks like a remnant from an earlier attempt. Probably not needed.

[mtrent]

Review feedback: use --implicit-check-not to catch all unexpected warnings.
Remove unnecessary initialization.

[MaskRay]

LG, but I'll defer to @jhenderson and @grimar to approve.

MachODump.cpp needs more refactorings and probably even a rewrite of some parts. @mtrent If you know someone interested in this tool, please let them know:)

[grimar]

LGTM. Please also wait for @jhenderson approve.

[jhenderson]

LGTM.

llvm/test/tools/llvm-objdump/X86/macho-cstring-dump.test

-RUN: llvm-objdump -m -section __TEXT,__cstring %p/Inputs/hello.obj.macho-x86_64 | FileCheck %s
-RUN: llvm-objdump -m -section __TEXT,__cstring -no-leading-addr %p/Inputs/hello.obj.macho-x86_64 | FileCheck %s -check-prefix=NO_ADDR
-RUN: llvm-objdump -m -section __TEXT,__cstring -non-verbose %p/Inputs/hello.obj.macho-x86_64 | FileCheck %s -check-prefix=NON_VERBOSE
+RUN: llvm-objdump -m -section __TEXT,__cstring %p/Inputs/hello.obj.macho-x86_64 2>&1 | FileCheck %s --implicit-check-not="warning:"
+RUN: llvm-objdump -m -section __TEXT,__cstring -no-leading-addr %p/Inputs/hello.obj.macho-x86_64 | FileCheck %s -check-prefix=NO_ADDR --implicit-check-not="warning:"
+RUN: llvm-objdump -m -section __TEXT,__cstring -non-verbose %p/Inputs/hello.obj.macho-x86_64 | FileCheck %s -check-prefix=NON_VERBOSE --implicit-check-not="warning:"
 
 CHECK: Contents of (__TEXT,__cstring) section
-CHECK: 000000000000003b  Hello world\n
+CHECK-NEXT: 000000000000003b  Hello world\n
 
 NO_ADDR: Contents of (__TEXT,__cstring) section
 NO_ADDR: Hello world\n
 NO_ADDR-NOT: 000000000000003b
 
 NON_VERBOSE: Contents of (__TEXT,__cstring) section
 NON_VERBOSE: 000000000000003b 48 65 6c 6c 6f 20 77 6f 72 6c 64 0a 00 

llvm/tools/llvm-objdump/MachODump.cpp

 //===-- MachODump.cpp - Object file dumping utility for llvm --------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the MachO-specific dumper for llvm-objdump.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm-objdump.h"
 #include "llvm-c/Disassembler.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/BinaryFormat/MachO.h"
 #include "llvm/Config/config.h"
 #include "llvm/DebugInfo/DIContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/Demangle/Demangle.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCContext.h"
static cl::opt<bool> NoSymbolicOperands(
     "no-symbolic-operands",
     cl::desc("do not symbolic operands when disassembling (requires -macho)"),
     cl::cat(MachOCat));
 
 static cl::list<std::string>
     ArchFlags("arch", cl::desc("architecture(s) from a Mach-O file to dump"),
               cl::ZeroOrMore, cl::cat(MachOCat));
 
+extern StringSet<> FoundSectionSet;
+
 bool ArchAll = false;
 
 static std::string ThumbTripleName;
 
 static const Target *GetTarget(const MachOObjectFile *MachOObj,
                                const char **McpuDefault,
                                const Target **ThumbTarget) {
   // Figure out the target triple.
for (unsigned i = 0; i < FilterSections.size(); ++i) {
     if (!DumpSegSectName.second.empty()) {
       DumpSegName = DumpSegSectName.first;
       DumpSectName = DumpSegSectName.second;
     } else {
       DumpSegName = "";
       DumpSectName = DumpSegSectName.first;
     }
     for (const SectionRef &Section : O->sections()) {
       StringRef SectName;
       Expected<StringRef> SecNameOrErr = Section.getName();
       if (SecNameOrErr)
         SectName = *SecNameOrErr;
       else
         consumeError(SecNameOrErr.takeError());
 
+      if (!DumpSection.empty())
+        FoundSectionSet.insert(DumpSection);
+
       DataRefImpl Ref = Section.getRawDataRefImpl();
       StringRef SegName = O->getSectionFinalSegmentName(Ref);
       if ((DumpSegName.empty() || SegName == DumpSegName) &&
           (SectName == DumpSectName)) {
 
         uint32_t section_flags;
         if (O->is64Bit()) {
           const MachO::section_64 Sec = O->getSection64(Ref);

llvm/tools/llvm-objdump/llvm-objdump.cpp

static cl::opt<bool>
     Wide("wide", cl::desc("Ignored for compatibility with GNU objdump"),
          cl::cat(ObjdumpCat));
 static cl::alias WideShort("w", cl::Grouping, cl::aliasopt(Wide));
 
 static cl::extrahelp
     HelpResponse("\nPass @FILE as argument to read options from FILE.\n");
 
 static StringSet<> DisasmFuncsSet;
-static StringSet<> FoundSectionSet;
+StringSet<> FoundSectionSet;
 static StringRef ToolName;
 
 typedef std::vector<std::tuple<uint64_t, StringRef, uint8_t>> SectionSymbolsTy;
 
 namespace {
 struct FilterResult {
   // True if the section should not be skipped.
   bool Keep;