[PATCH] D80203: [ObjectYAML][DWARF] Add DWARF entry in ELFYAML.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/lib/ObjectYAML/ELFYAML.cpp:1254
+    // For sections identified by their names, we have to set the section type
+    // for them earlier.
+    if (ELFYAML::StackSizesSection::nameMatches(Name))
----------------
grimar wrote:
> jhenderson wrote:
> > I'd add the following sentence to this comment: "This means they can have other SHT_* type values, but that the special parsing of those types is not supported."
> > 
> > This seems like a reasonable solution to me, but I'm interested to here @grimar's thoughts.
> If I understand it right, that change means:
> 1) We are unable to create, for example, `SHT_DYNAMIC` section named  `.debug_` anymore.
> 2) It was done to allow creating `.debug_` sections that has `sh_type` overriden to something, like to `SHT_DYNAMIC`.
> 
> Currently, yaml2obj uses section type to make a specific section mapping. It looks generally more right than checking sections by name (in the ideal world). Unfortunately we have to check section names still. But it is just because they have no dedicated SHT_* types first of all I believe.
> 
> And my concern that (2) might be confusing, because the current way we have to override things is to use Sh* fields. E.g:
> 
> ```
>   // This can be used to override the offset stored in the sh_name field.
>   // It does not affect the name stored in the string table.
>   Optional<llvm::yaml::Hex64> ShName;
> 
>   // This can be used to override the sh_offset field. It does not place the
>   // section data at the offset specified.
>   Optional<llvm::yaml::Hex64> ShOffset;
> 
>   // This can be used to override the sh_size field. It does not affect the
>   // content written.
>   Optional<llvm::yaml::Hex64> ShSize;
> 
>   // This can be used to override the sh_flags field.
>   Optional<llvm::yaml::Hex64> ShFlags;
> ```
> 
> Also, what if we want to create a `SHT_RELR` section with the type `SHT_DYNAMIC`?
> 
> I think the better way would be to introduce `ShType` field that would allow to override the `sh_type` to anything.
> (And I believe it can be done in a follow-up patch, no need to support such specific things from the begining).
> 
> How does it sound?
> 
To clarify.

This will create a dynamic section with the name `.debug_str`.
```
- Name: .debug_str
  Type: SHT_DYNAMIC
```

And this then can be used to create a debug section with the type `SHT_DYNAMIC`:

```
- Name: .debug_str
  Type: SHT_PROGBITS
  ShType: SHT_DYNAMIC
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80203