[PATCH] D85717: [DWARFYAML] Teach yaml2obj emit the correct line table program.

Tue Aug 11 03:30:19 PDT 2020

Higuoxing added inline comments.

================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml:526-531
+## j) Test that yaml2obj emits an error message if the line table's associated compilation unit's address
+## size is not 8 or 4.
+
+# RUN: not yaml2obj --docnum=10 %s 2>&1 | FileCheck %s --check-prefix=INVALID-ADDRSIZE
+
+# INVALID-ADDRSIZE: yaml2obj: error: unable to write address for operator DW_LNE_set_address: invalid integer write size: 3
----------------
jhenderson wrote:
> Higuoxing wrote:
> > jhenderson wrote:
> > > It's good to have a test for this, but I actually think the behaviour is incorrect for yaml2obj. It's possible to have line tables without .debug_info, and there's no reference from a line table to .debug_info anyway. The address should be inferred from the target machine, in my opinion (in this case it would be 8).
> > > 
> > > From the DWARF v4 spec:
> > > 
> > > > 2.   DW_LNE_set_address   The DW_LNE_set_address opcode takes a single relocatable address as an operand. The size of the operand is the size of an address on the target machine.
> > > 
> > Oh, I see. What do you think of having an optional field `AddressSize` for the line table. If it's specified, yaml2obj will emit addresses according to the value.
> We should probably only have an explicit `AddressSize` field for v5 line tables, I think, but we could use the same internal logic that that uses to identify it's default value when missing (which should match that of other sections), to determine the address size.
I got it. We should make the address size inferred from the target machine when the version is less than or equal to 4. When the version is 5, there should be an optional field `AddressSize`. In a follow-up patch, the reference of address size of debug_info should be removed and this test case should be modified, right?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85717/new/

https://reviews.llvm.org/D85717