[PATCH] D84234: [DWARFYAML] Implement the .debug_loclists section.

[Xing GUO via Phabricator via llvm-commits]

Higuoxing marked an inline comment as done.
Higuoxing added inline comments.


================
Comment at: llvm/lib/ObjectYAML/DWARFEmitter.cpp:446-448
+writeDWARFExpression(raw_ostream &OS,
+                     const DWARFYAML::DWARFOperation &Operation,
+                     uint8_t AddrSize, bool IsLittleEndian) {
----------------
jhenderson wrote:
> labath wrote:
> > jhenderson wrote:
> > > One of the things yaml2obj is useful for is for creating broken ELFs. I see you're adding further checks here to make sure the `Values` array is the right size for the type. But what if you want to have an incorrect number of values for a given operation?
> > > 
> > > There are similar issues elsewhere, I'm sure, so I'm not saying this specific patch needs changing at this point, but I do think you need to consider how to solve this case. For example, it might mean being more specific with the type of value by using distinct fields for different types. For example, instead of `Values: [0x1234, 0x4321]` you might want something like:
> > > ```
> > > Values:
> > >   - Type: SLEB128
> > >     Value: 0x1234
> > >   - Type: Address
> > >     Value: 0x4321
> > > ```
> > > or even:
> > > ```
> > > Values:
> > >   - SLEB128: 0x1234
> > >   - Address: 0x4321
> > > ```
> > > What do you think?
> > I've been wondering about that too. I personally don't find the ability to add more/less "values" than the appropriate DW_LLE kind expects useful, because the yaml structure will have no relation to how the result ends up being parsed. And the extra info needed to make this operation well-defined makes the yaml more verbose for the normal cases.
> > 
> > What I would find useful is if, at certain strategic levels, it would be possible to insert an arbitrary hex blob instead of fully formed yaml. One such place could be the dwarf expression -- instead of all the DW_OP thingies we could just add a hex blob. That would also give an escape hatch for the obj2yaml path so that it can serialize any DWARF expression with unknown/unsupported opcodes as hex.
> > 
> > Another such level could be the level of an individual location list -- if there's anything funny about the location list, one could put the entire list as hex, even though some parts of it could be valid and representable normally. I think that's fine because this is not likely to be needed often. Or maybe this could apply to an entire debug_loclists contribution. Or both...
> > 
> > At the last level, one could specify the entire section contents as hex, which is sort of what we have now.
> That makes sense to me. There are some cases in the ELF code, where we fall back to using hex section contents for known section types, when the content is unparseable for whatever reason.
> One of the things yaml2obj is useful for is for creating broken ELFs. I see you're adding further checks here to make sure the Values array is the right size for the type. But what if you want to have an incorrect number of values for a given operation?

Sounds good! What about having an additional field called `Types`. If the `Types` field is specified, yaml2obj emits values according to the `Types` array. If it doesn't exist, yaml2obj emits values according to the spec.

```
- Operator: DW_LLE_startx_endx
  Types:    [ ULEB128, Hex64 ]
  Values:   [ 0x1234, 0x4321 ]
```

The benefit is that we don't have to specify the `Value` field every time. The number of operands can also be adjusted via the `Types` array.


```
- Operator: DW_LLE_startx_endx
  Types:    [ ULEB128, Hex64, Hex16 ]
  Values:   [ 0x1234, 0x4321, 0x12 ]
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84234