Omit "Contents of" headers when -no-leading-headers is specified.

[mtrent]

Summary

llvm-objdump -macho will no longer print "Contents of" headers when
disassembling section contents when -no-leading-headers is specified.
For historical reasons, this flag is independent of -no-leading-addr.

[jhenderson]

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

6

As the two switches are separate, let's not specify them both. I think this test case can just specify no-leading-headers.

If you think the combination of them both is worth testing, do that in addition to the individual cases.

[mtrent]

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

6

A reasonable point. I'll update the patch.

[mtrent]

Review feedback: Test -no-leading-headers separately from -no-leading-addr.

[jhenderson]

LGTM.

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

10–11

Not related to this patch, but I just spotted a bug here, which you might want to fix in a follow-up. This doesn't actually prove that the address is not printed, because it only shows that the address is not printed after "Hello world\n". I think just moving the line up a row would be enough, possibly in conjunction with --match-full-lines or similar.

13–15

This is fine, but I could suggest a number of improvements to reduce duplication of the checks, by using multile check prefixes. No need to do that though, unless you want to.

[mtrent]

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

10–11

So all of these FileCheck scripts are fragile in one way or another.

Changing the second line to "NO_ADDR-NEXT" would address the specific issue you address. It doesn't address the problem of the address being printed head of the "Contents of" line, or if the address is printed using some other format. What's more, FileCheck is 'resilient' to some things, such as differing whitespace: this is both a really good thing and a really bad thing depending on what production tools expect.

I'm happy to receive the feedback and to consider approaches in the future. Additional points of view polishes my understanding of the problem.

[jhenderson]

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

10–11

FileCheck's --implicit-check-not is useful for making sure some pattern doesn't appear anywhere in the output. That is often the way to go, though it has some downsides, especially with shorter strings (e.g. "bar" appearing in somebody's username, which in turn appears in a file path). You can also use --strict-whitespace and --match-full-lines if formatting is important for a given test case.

Regarding the NO_ADDR-NEXT suggestion, my personal view is that given there is a test case that shows the address is printed between the headers and the string when it is enabled, I think we only need to show that it is not printed at that location when the switch is specified.

13–15

More concrete suggestion (no need to adopt it though). Use something like the following patterns:

FILE: Inputs/hello.obj.macho-x86_64:
HEADER: Contents of (__TEXT,__cstring) section
ADDR: 000000000000003b
CHECK: {{^}}  Hello world\n

You then specify the combination of prefixes that correspond to the case, with --implicit-check-not to show the other strings don't appear. E.g. for the default case (i.e. print everything), use --check-prefixes=FILE,HEADER,ADDR,CHECK. The {{^}} ensures that the "Hello world" string matches the start of a line, or the end of the previous match (I think - not verified that this definitely will work), which means you'd want --match-full-lines to show the string appears on the same line as the address.

As to whether this is more readable or not is up for debate, but it does reduce the amount of checking.

It's also given me an idea for a new FileCheck feature. I'll post something on the mailing list.

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

 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 -no-leading-headers %p/Inputs/hello.obj.macho-x86_64 | FileCheck %s -check-prefix=NO_HEADERS --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-NEXT: 000000000000003b  Hello world\n
 
 NO_ADDR: Contents of (__TEXT,__cstring) section
 NO_ADDR: Hello world\n
 NO_ADDR-NOT: 000000000000003b
 
+NO_HEADERS-NOT: Inputs/hello.obj.macho-x86_64:
+NO_HEADERS-NOT: Contents of (__TEXT,__cstring) section
+NO_HEADERS: 000000000000003b  Hello world\n
+
 NON_VERBOSE: Contents of (__TEXT,__cstring) section
-NON_VERBOSE: 000000000000003b 48 65 6c 6c 6f 20 77 6f 72 6c 64 0a 00 
+NON_VERBOSE: 000000000000003b 48 65 6c 6c 6f 20 77 6f 72 6c 64 0a 00

llvm/tools/llvm-objdump/MachODump.cpp

for (const SectionRef &Section : O->sections()) {
         uint32_t section_type = section_flags & MachO::SECTION_TYPE;
 
         StringRef BytesStr =
             unwrapOrError(Section.getContents(), O->getFileName());
         const char *sect = reinterpret_cast<const char *>(BytesStr.data());
         uint32_t sect_size = BytesStr.size();
         uint64_t sect_addr = Section.getAddress();
 
+        if (!NoLeadingHeaders)
           outs() << "Contents of (" << SegName << "," << SectName
                  << ") section\n";
 
         if (verbose) {
           if ((section_flags & MachO::S_ATTR_PURE_INSTRUCTIONS) ||
               (section_flags & MachO::S_ATTR_SOME_INSTRUCTIONS)) {
             DisassembleMachO(Filename, O, SegName, SectName);
             continue;
           }
           if (SegName == "__TEXT" && SectName == "__info_plist") {