[llvm-readobj][NFC] Don't use startLine in a middle of a line in ObjDumper.

[cjacek]

Currently, startLine() and getOSStream() are mixed in ObjDumper. This is harmless in practice: startLine prints additional indention, but the code is always executed when there is no indention.

I noticed the mismatch when implementing support for dumping ARM64X files. The way I implemented it (see cjacek@f70491b for a draft), I run this code for the hybrid image view as well, which is indented and then the mismatch causes formatting issues.

[llvmbot]

@llvm/pr-subscribers-llvm-binary-utilities

Author: Jacek Caban (cjacek)

Changes

Currently, startLine() and getOSStream() are mixed in ObjDumper. This is harmless in practice: startLine prints additional indention, but the code is always executed when there is no indention.

I noticed the mismatch when implementing support for dumping ARM64X files. The way I implemented it (see cjacek@f70491b for a draft), I run this code for the hybrid image view as well, which is indented and then the mismatch causes formatting issues.

---

Full diff: https://github.com/llvm/llvm-project/pull/102071.diff

1 Files Affected:

• (modified) llvm/tools/llvm-readobj/ObjDumper.cpp (+16-14)

diff --git a/llvm/tools/llvm-readobj/ObjDumper.cpp b/llvm/tools/llvm-readobj/ObjDumper.cpp
index 0980d2ad3a852..20e99d9d97f3a 100644
--- a/llvm/tools/llvm-readobj/ObjDumper.cpp
+++ b/llvm/tools/llvm-readobj/ObjDumper.cpp
@@ -82,8 +82,8 @@ void ObjDumper::printAsStringList(StringRef StringContent,
       continue;
     }
     W.startLine() << format("[%6tx] ", CurrentWord - StrContent);
-    printAsPrintable(W.startLine(), CurrentWord, WordSize);
-    W.startLine() << '\n';
+    printAsPrintable(W.getOStream(), CurrentWord, WordSize);
+    W.getOStream() << '\n';
     CurrentWord += WordSize + 1;
   }
 }
@@ -91,7 +91,7 @@ void ObjDumper::printAsStringList(StringRef StringContent,
 void ObjDumper::printFileSummary(StringRef FileStr, object::ObjectFile &Obj,
                                  ArrayRef<std::string> InputFilenames,
                                  const object::Archive *A) {
-  W.startLine() << "\n";
+  W.getOStream() << "\n";
   W.printString("File", FileStr);
   W.printString("Format", Obj.getFileFormatName());
   W.printString("Arch", Triple::getArchTypeName(Obj.getArch()));
@@ -163,7 +163,8 @@ void ObjDumper::printSectionsAsString(const object::ObjectFile &Obj,
   for (object::SectionRef Section :
        getSectionRefsByNameOrIndex(Obj, Sections)) {
     StringRef SectionName = unwrapOrError(Obj.getFileName(), Section.getName());
-    W.startLine() << "\nString dump of section '" << SectionName << "':\n";
+    W.getOStream() << '\n';
+    W.startLine() << "String dump of section '" << SectionName << "':\n";
 
     StringRef SectionContent =
         unwrapOrError(Obj.getFileName(), Section.getContents());
@@ -180,7 +181,8 @@ void ObjDumper::printSectionsAsHex(const object::ObjectFile &Obj,
   for (object::SectionRef Section :
        getSectionRefsByNameOrIndex(Obj, Sections)) {
     StringRef SectionName = unwrapOrError(Obj.getFileName(), Section.getName());
-    W.startLine() << "\nHex dump of section '" << SectionName << "':\n";
+    W.getOStream() << '\n';
+    W.startLine() << "Hex dump of section '" << SectionName << "':\n";
 
     StringRef SectionContent =
         unwrapOrError(Obj.getFileName(), Section.getContents());
@@ -196,13 +198,13 @@ void ObjDumper::printSectionsAsHex(const object::ObjectFile &Obj,
 
       W.startLine() << format_hex(Section.getAddress() + (SecPtr - SecContent),
                                   10);
-      W.startLine() << ' ';
+      W.getOStream() << ' ';
       for (i = 0; TmpSecPtr < SecEnd && i < 4; ++i) {
         for (k = 0; TmpSecPtr < SecEnd && k < 4; k++, TmpSecPtr++) {
           uint8_t Val = *(reinterpret_cast<const uint8_t *>(TmpSecPtr));
-          W.startLine() << format_hex_no_prefix(Val, 2);
+          W.getOStream() << format_hex_no_prefix(Val, 2);
         }
-        W.startLine() << ' ';
+        W.getOStream() << ' ';
       }
 
       // We need to print the correct amount of spaces to match the format.
@@ -211,17 +213,17 @@ void ObjDumper::printSectionsAsHex(const object::ObjectFile &Obj,
       // Least, if we cut in a middle of a row, we add the remaining characters,
       // which is (8 - (k * 2)).
       if (i < 4)
-        W.startLine() << format("%*c", (4 - i) * 8 + (4 - i), ' ');
+        W.getOStream() << format("%*c", (4 - i) * 8 + (4 - i), ' ');
       if (k < 4)
-        W.startLine() << format("%*c", 8 - k * 2, ' ');
+        W.getOStream() << format("%*c", 8 - k * 2, ' ');
 
       TmpSecPtr = SecPtr;
       for (i = 0; TmpSecPtr + i < SecEnd && i < 16; ++i)
-        W.startLine() << (isPrint(TmpSecPtr[i])
-                              ? static_cast<char>(TmpSecPtr[i])
-                              : '.');
+        W.getOStream() << (isPrint(TmpSecPtr[i])
+                               ? static_cast<char>(TmpSecPtr[i])
+                               : '.');
 
-      W.startLine() << '\n';
+      W.getOStream() << '\n';
     }
   }
 }

[mstorsjo]

LGTM, thanks!

[cjacek]

Merged, thanks.

[jh7370]

Behaviour like whitespace/indentation should have testing with --strict-whitespace and --match-full-lines enabled, so that we don't regress the formatting in the future. Does that currently exist for the options impacted here? If not, I think it would be worthwhile adding it.

[cjacek]

Yes, it's checked by file-headers.test tests (a few of them, for various formats) and hex-dump.test. I verified that they fail if I intentionally "break" the code to output an additional space. I will also make tests in #102245 more strict, to make sure that indentation works as expected.