[DWARFYAML] Make the ExtLen field of extended opcodes optional.

[Higuoxing]

Summary

This patch makes the 'ExtLen' field of extended opcodes optional. We
don't need to manually calculate it in the future.

[jhenderson]

llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml

627

Something's not quite right at the end of this number sequence, the dots don't line up with the words beneath. Do you actually need the dots?

628–636

Where you're not interested in the header, maybe we could simplify this to just say something like ^------------------------ header?

[Higuoxing]

Address review comments.

Thanks!

llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml

627

0x20 is <SPC>. Hence, the number of dots on the right side is 15 (16 - 1). I think we don't actually need them.

628–636

Good idea!

[jhenderson]

llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml

638

Thinking about it, I guess we need something on this line to show that we've reached the end of the content (i.e. to show we haven't written any bogus extra data). Maybe something as simple as {{ }} (with more than one space). Other ideas also welcome.

[Higuoxing]

Thanks!

[jhenderson]

llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml

638

I think you need more than one space in the pattern. Otherwise this will still match:
00b42401 11223344 which would be the output with data appearing after the thing you want to check.

Thinking about it, I don't think {{ }} (two spaces) will work without FileCheck's --strict-whitespace. You might just be better off adding the extra bit at the end of the line for this line only.

[Higuoxing]

Append --strict-whitespace option.
Use {{ }} to match more (2) spaces.

[jhenderson]

LGTM.

llvm/include/llvm/ObjectYAML/DWARFYAML.h

struct File {
   StringRef Name;
   uint64_t DirIdx;
   uint64_t ModTime;
   uint64_t Length;
 };
 
 struct LineTableOpcode {
   dwarf::LineNumberOps Opcode;
-  uint64_t ExtLen;
+  Optional<uint64_t> ExtLen;
   dwarf::LineNumberExtendedOps SubOpcode;
   uint64_t Data;
   int64_t SData;
   File FileEntry;
   std::vector<llvm::yaml::Hex8> UnknownOpcodeData;
   std::vector<llvm::yaml::Hex64> StandardOpcodeData;
 };
 

llvm/lib/ObjectYAML/DWARFEmitter.cpp

 static void emitFileEntry(raw_ostream &OS, const DWARFYAML::File &File) {
   OS.write(File.Name.data(), File.Name.size());
   OS.write('\0');
   encodeULEB128(File.DirIdx, OS);
   encodeULEB128(File.ModTime, OS);
   encodeULEB128(File.Length, OS);
 }
 
-static void writeLineTableOpcode(const DWARFYAML::LineTableOpcode &Op,
-                                 uint8_t OpcodeBase, uint8_t AddrSize,
-                                 raw_ostream &OS, bool IsLittleEndian) {
-  writeInteger((uint8_t)Op.Opcode, OS, IsLittleEndian);
-  if (Op.Opcode == 0) {
-    encodeULEB128(Op.ExtLen, OS);
-    writeInteger((uint8_t)Op.SubOpcode, OS, IsLittleEndian);
-    switch (Op.SubOpcode) {
-    case dwarf::DW_LNE_set_address:
-      cantFail(
-          writeVariableSizedInteger(Op.Data, AddrSize, OS, IsLittleEndian));
-      break;
-    case dwarf::DW_LNE_define_file:
-      emitFileEntry(OS, Op.FileEntry);
-      break;
-    case dwarf::DW_LNE_set_discriminator:
-      encodeULEB128(Op.Data, OS);
-      break;
-    case dwarf::DW_LNE_end_sequence:
-      break;
-    default:
-      for (auto OpByte : Op.UnknownOpcodeData)
-        writeInteger((uint8_t)OpByte, OS, IsLittleEndian);
-    }
+static void writeExtendedOpcode(const DWARFYAML::LineTableOpcode &Op,
+                                uint8_t AddrSize, bool IsLittleEndian,
+                                raw_ostream &OS) {
+  // The first byte of extended opcodes is a zero byte. The next bytes are an
+  // ULEB128 integer giving the number of bytes in the instruction itself (does
+  // not include the first zero byte or the size). We serialize the instruction
+  // itself into the OpBuffer and then write the size of the buffer and the
+  // buffer to the real output stream.
+  std::string OpBuffer;
+  raw_string_ostream OpBufferOS(OpBuffer);
+  writeInteger((uint8_t)Op.SubOpcode, OpBufferOS, IsLittleEndian);
+    cantFail(writeVariableSizedInteger(Op.Data, AddrSize, OpBufferOS,
+                                       IsLittleEndian));
+    emitFileEntry(OpBufferOS, Op.FileEntry);
+    encodeULEB128(Op.Data, OpBufferOS);
+      writeInteger((uint8_t)OpByte, OpBufferOS, IsLittleEndian);
+  }
+  uint64_t ExtLen = Op.ExtLen.getValueOr(OpBuffer.size());
+  encodeULEB128(ExtLen, OS);
+  OS.write(OpBuffer.data(), OpBuffer.size());
+
+static void writeLineTableOpcode(const DWARFYAML::LineTableOpcode &Op,
+                                 uint8_t OpcodeBase, uint8_t AddrSize,
+                                 raw_ostream &OS, bool IsLittleEndian) {
+  writeInteger((uint8_t)Op.Opcode, OS, IsLittleEndian);
+  if (Op.Opcode == 0) {
+    writeExtendedOpcode(Op, AddrSize, IsLittleEndian, OS);
   } else if (Op.Opcode < OpcodeBase) {
     switch (Op.Opcode) {
     case dwarf::DW_LNS_copy:
     case dwarf::DW_LNS_negate_stmt:
     case dwarf::DW_LNS_set_basic_block:
     case dwarf::DW_LNS_const_add_pc:
     case dwarf::DW_LNS_set_prologue_end:
     case dwarf::DW_LNS_set_epilogue_begin:

llvm/lib/ObjectYAML/DWARFYAML.cpp

void MappingTraits<DWARFYAML::File>::mapping(IO &IO, DWARFYAML::File &File) {
   IO.mapRequired("ModTime", File.ModTime);
   IO.mapRequired("Length", File.Length);
 }
 
 void MappingTraits<DWARFYAML::LineTableOpcode>::mapping(
     IO &IO, DWARFYAML::LineTableOpcode &LineTableOpcode) {
   IO.mapRequired("Opcode", LineTableOpcode.Opcode);
   if (LineTableOpcode.Opcode == dwarf::DW_LNS_extended_op) {
-    IO.mapRequired("ExtLen", LineTableOpcode.ExtLen);
+    IO.mapOptional("ExtLen", LineTableOpcode.ExtLen);
     IO.mapRequired("SubOpcode", LineTableOpcode.SubOpcode);
   }
 
   if (!LineTableOpcode.UnknownOpcodeData.empty() || !IO.outputting())
     IO.mapOptional("UnknownOpcodeData", LineTableOpcode.UnknownOpcodeData);
   if (!LineTableOpcode.UnknownOpcodeData.empty() || !IO.outputting())
     IO.mapOptional("StandardOpcodeData", LineTableOpcode.StandardOpcodeData);
   if (!LineTableOpcode.FileEntry.Name.empty() || !IO.outputting())

llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml

debug_line:
     - Version:               4
       MinInstLength:         1
       MaxOpsPerInst:         1
       DefaultIsStmt:         1
       LineBase:              251
       LineRange:             14
       OpcodeBase:            4
       StandardOpcodeLengths: [ 0, 1, 1 ]
+
+## l) Test that we can specify or omit the ExtLen field of extended opcodes.
+
+# RUN: yaml2obj --docnum=12 %s -o %t12.o
+# RUN: llvm-readelf --hex-dump=.debug_line %t12.o | \
+# RUN:   FileCheck %s --check-prefix=EXTLEN
+
+#      EXTLEN: Hex dump of section '.debug_line':
+# EXTLEN-NEXT: 0x00000000 20000000 04000800 00000101 01fb0e01
+##                        ^---------------------------------- line number program header
+# EXTLEN-NEXT: 0x00000010 0000000c 03616263 6400b424 b424b424
+##                        ----
+##                            ^-                              DW_LNS_extended_op
+##                              ^-                            extended opcode length (ULEB128) 12
+##                                 ^-                         DW_LNE_define_file
+##                                   ^----------              "abcd\0"
+##                                              ^---          directory index (ULEB128) 0x1234
+##                                                   ^---     modification time (ULEB128) 0x1234
+##                                                       ^--- file length (ULEB128) 0x1234
+# EXTLEN-NEXT: 0x00000020 00b42401{{ }}
+##                        ^-                                  DW_LNS_extended_op
+##                          ^---                              extended opcode length (ULEB128) 0x1234
+##                              ^-                            DW_LNE_end_sequence
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+DWARF:
+  debug_line:
+    - Version:               4
+      MinInstLength:         1
+      MaxOpsPerInst:         1
+      DefaultIsStmt:         1
+      LineBase:              251
+      LineRange:             14
+      OpcodeBase:            1
+      StandardOpcodeLengths: []
+      Opcodes:
+        - Opcode:    DW_LNS_extended_op
+          ## Omit the ExtLen field.
+          SubOpcode: DW_LNE_define_file
+          FileEntry:
+            Name:    abcd
+            DirIdx:  0x1234
+            ModTime: 0x1234
+            Length:  0x1234
+        - Opcode:    DW_LNS_extended_op
+          ## Specify the ExtLen field.
+          ExtLen:    0x1234
+          SubOpcode: DW_LNE_end_sequence

llvm/tools/obj2yaml/dwarf2yaml.cpp

if (auto StmtOffset =
             NewOp.Data = LineData.getAddress(&Offset);
             break;
           case dwarf::DW_LNE_define_file:
             dumpFileEntry(LineData, Offset, NewOp.FileEntry);
             break;
           case dwarf::DW_LNE_end_sequence:
             break;
           default:
-            while (Offset < StartExt + NewOp.ExtLen)
+            while (Offset < StartExt + *NewOp.ExtLen)
               NewOp.UnknownOpcodeData.push_back(LineData.getU8(&Offset));
           }
         } else if (NewOp.Opcode < DebugLines.OpcodeBase) {
           switch (NewOp.Opcode) {
           case dwarf::DW_LNS_copy:
           case dwarf::DW_LNS_negate_stmt:
           case dwarf::DW_LNS_set_basic_block:
           case dwarf::DW_LNS_const_add_pc: