[obj2yaml] - Fix indentations in the ELF output.

[grimar]

Summary

This is as about the problem D73045 tried to solve,
but I suggest a completely different approach.

What if we just do a post processing for the output?
I.e. first output it to a string and then using our knowledge
about YAML grammar will reformat indentations.

It separates the formatting pass from the original pass,
what makes the whole approach isolated and simple.

[jhenderson]

Overall, the process seems sound and simple. That probably makes it the right thing to do here. Some comments explaining each step of the post-processing would be useful though. Also, should this really be restricted to ELF?

llvm/test/tools/obj2yaml/elf-output-indentation.yaml

2

an output -> the output

40

Use an arbitrary YAML document
or
Use arbitrary YAML

would be better English. However, I think what you really want to say is something like "Use a YAML document with arbitrary fields and values to show how the indentation is affected."

[grimar]

In D74611#1880193, @jhenderson wrote:

Also, should this really be restricted to ELF?

I see no obvious reasons for such restrictions. Though I supposed that we might want to start from ELF and then
enable it for other targets. Do you have a preference here? I.e. do you want to see it for all targets from the begining?

[jhenderson]

In D74611#1880357, @grimar wrote:

In D74611#1880193, @jhenderson wrote:

Also, should this really be restricted to ELF?

I see no obvious reasons for such restrictions. Though I supposed that we might want to start from ELF and then
enable it for other targets. Do you have a preference here? I.e. do you want to see it for all targets from the begining?

I don't have a strong preference, and am happy for you to go with whatever is easier.

[MaskRay]

Does this patch handle sequence types with has_FlowTraits (llvm/Support/YAMLTraits.h) set? It is used for auto line wrapping.

Key: [value0, value1, value2,
      value3, value4,
      value5]

I don't know whether obj2yaml has such objects to emit, though.

Maybe you can summarize the problems and ask on llvm-dev?

(I think the direction of D73045 may be fine.)

[grimar]

In D74611#1881673, @MaskRay wrote:

Does this patch handle sequence types with has_FlowTraits (llvm/Support/YAMLTraits.h) set? It is used for auto line wrapping.

Key: [value0, value1, value2,
      value3, value4,
      value5]

Not yet. Yesterday I've found this and other cases where the current code doesn't produce an expected result.
Though it looks like they can be fixed without adding much complexity to the algorithm, but I had no chance to check yet.

I don't know whether obj2yaml has such objects to emit, though.

Maybe you can summarize the problems and ask on llvm-dev?

(I think the direction of D73045 may be fine.)

I do not like the direction of D73045. It looks to me too intrusive for a "add or remove few spaces" task.
(Note that the version posted in D73045 does not solve the problem, it should be extended and it seems
to me that it should be noticably larger to solve the task).

[grimar]

• Updated the algorithm used.
• Added a much better test cases.
• Added comments.

llvm/test/tools/obj2yaml/elf-output-indentation.yaml

+## obj2yaml tries to optimize indentations between keys and
+## values to make an output nicer.
+## In this test we demonstrate the output produced.
+
+# RUN: yaml2obj %s -o %t
+# RUN: obj2yaml %t | FileCheck %s --strict-whitespace --match-full-lines
+
+#      CHECK:--- !ELF
+# CHECK-NEXT:FileHeader:
+# CHECK-NEXT:  Class:   ELFCLASS64
+# CHECK-NEXT:  Data:    ELFDATA2LSB
+# CHECK-NEXT:  Type:    ET_REL
+# CHECK-NEXT:  Machine: EM_X86_64
+# CHECK-NEXT:Sections:
+# CHECK-NEXT:  - Name: .text
+# CHECK-NEXT:    Type: SHT_PROGBITS
+# CHECK-NEXT:  - Name:         .data
+# CHECK-NEXT:    Type:         SHT_PROGBITS
+# CHECK-NEXT:    AddressAlign: 0x0000000000000001
+# CHECK-NEXT:  - Name:    .rela.text
+# CHECK-NEXT:    Type:    SHT_RELA
+# CHECK-NEXT:    Link:    .symtab
+# CHECK-NEXT:    EntSize: 0x0000000000000018
+# CHECK-NEXT:    Info:    .text
+# CHECK-NEXT:    Relocations:
+# CHECK-NEXT:      - Offset: 0x0000000000000000
+# CHECK-NEXT:        Type:   R_X86_64_PC32
+# CHECK-NEXT:  - Name:    .rela.text2
+# CHECK-NEXT:    Type:    SHT_RELA
+# CHECK-NEXT:    Link:    .symtab
+# CHECK-NEXT:    EntSize: 0x0000000000000018
+# CHECK-NEXT:Symbols: []
+# CHECK-NEXT:DynamicSymbols:
+# CHECK-NEXT:  - Name:    dynamic
+# CHECK-NEXT:    Binding: STB_GLOBAL
+# CHECK-NEXT:    Value:   0x0000000012345678
+# CHECK-NEXT:  - Name: both
+# CHECK-NEXT:...
+
+## Use an arbitrary YAML with different fields.
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name: .text
+    Type: SHT_PROGBITS
+  - Name: .data
+    Type: SHT_PROGBITS
+    AddressAlign: 0x1
+  - Name:    .rela.text
+    Type:    SHT_RELA
+    Link:    .symtab
+    EntSize: 0x0000000000000018
+    Info:    .text
+    Relocations:
+      - Offset: 0x0000000000000000
+        Type:   R_X86_64_PC32
+  - Name:    .rela.text2
+    Type:    SHT_RELA
+    Link:    .symtab
+    EntSize: 0x0000000000000018
+    Relocations: []
+ProgramHeaders:
+  - Type:  PT_LOAD
+    VAddr: 0x100
+    Sections:
+      - Section: .text
+Symbols: []
+DynamicSymbols:
+  - Name:    dynamic
+    Binding: STB_GLOBAL
+    Value:   0x0000000012345678
+  - Name:  both

llvm/tools/obj2yaml/obj2yaml.cpp

 #include "llvm/Object/COFF.h"
 #include "llvm/Object/Minidump.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/InitLLVM.h"
 
 using namespace llvm;
 using namespace llvm::object;
 
+std::string postProcess(StringRef In) {
+  struct LineInfo {
+    size_t KeyLen;
+    size_t Colon;
+    StringRef Line;
+  };
+
+  struct ScanState {
+    size_t MaxKey = 0;
+    std::vector<LineInfo> Lines;
+  } State;
+
+  std::string Out;
+  auto FlushState = [&]() {
+    for (const LineInfo &LI : State.Lines) {
+      Out += LI.Line.take_front(LI.Colon + 1).str();
+      Out += std::string(State.MaxKey - LI.KeyLen + 1, ' ');
+      Out += LI.Line.drop_front(LI.Colon + 1).ltrim(' ');
+      Out += '\n';
+    }
+    State = {};
+  };
+
+  SmallVector<StringRef, 32> Lines;
+  In.split(Lines, '\n');
+
+  for (StringRef Line : Lines) {
+    size_t Colon = Line.find(':');
+    if (Colon == StringRef::npos || Colon == Line.size() - 1) {
+      FlushState();
+      Out += Line.str() + '\n';
+      continue;
+    }
+
+    if (Line.find('-') != StringRef::npos)
+      FlushState();
+
+    StringRef Key = Line.take_front(Colon).ltrim("- ");
+    if (Key.size() > State.MaxKey)
+      State.MaxKey = Key.size();
+    State.Lines.push_back({Key.size(), Colon, Line});
+  }
+  return Out;
+}
+
 static Error dumpObject(const ObjectFile &Obj) {
   if (Obj.isCOFF())
     return errorCodeToError(coff2yaml(outs(), cast<COFFObjectFile>(Obj)));
 
   if (Obj.isXCOFF())
     return errorCodeToError(xcoff2yaml(outs(), cast<XCOFFObjectFile>(Obj)));
 
-  if (Obj.isELF())
-    return elf2yaml(outs(), Obj);
+  if (Obj.isELF()) {
+    SmallString<0> Storage;
+    raw_svector_ostream OS(Storage);
+    if (Error Err = elf2yaml(OS, Obj))
+      return Err;
+    std::string Formatted = postProcess(Storage);
+    outs().write(Formatted.data(), Formatted.size());
+    return Error::success();
+  }
 
   if (Obj.isWasm())
     return errorCodeToError(wasm2yaml(outs(), cast<WasmObjectFile>(Obj)));
 
   return errorCodeToError(obj2yaml_error::unsupported_obj_file_format);
 }
 
 static Error dumpInput(StringRef File) {