[PATCH] D86867: [obj2yaml] Add support for dumping the .debug_str section.

[Xing GUO via Phabricator via llvm-commits]

Higuoxing added inline comments.


================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:209
+      if (SecName == "debug_str")
+        return !(RawSec->EntSize && (uint64_t)*RawSec->EntSize == 1) ||
+               RawSec->Flags.getValueOr(ELFYAML::ELF_SHF(0)) !=
----------------
grimar wrote:
> jhenderson wrote:
> > grimar wrote:
> > > Higuoxing wrote:
> > > > jhenderson wrote:
> > > > > I'm struggling with this code. Under what circumstances can `RawSec->EntSize` by None here? Similar question for the Flags below.
> > > > I check it because `EntSize` and `Flags` are `Optional` values. It seems that you are right, I cannot create a test case that make them `None`. Could you please tell me why they cannot be `None` here?
> > > Well. I haven't debugged this, but I *think* they can be None.
> > > 
> > > Looking on the code, we have:
> > > 
> > > ```
> > > Error ELFDumper<ELFT>::dumpCommonSection(const Elf_Shdr *Shdr,
> > >                                          ELFYAML::Section &S) {
> > > ...
> > >   if (Shdr->sh_entsize != getDefaultShEntSize<ELFT>(S.Type))
> > >     S.EntSize = static_cast<llvm::yaml::Hex64>(Shdr->sh_entsize);
> > > ```
> > > 
> > > So i'd expect to have `None` value when `sh_entsize == 0`. Isn't it true?
> > @grimar is more familiar with this code than I am, so perhaps he can explain. I unfortunately have higher priority work that needs doing today, or I'd go digging myself!
> So yes, I've debugged this and it is possible to get `None` for `EntSize` when `sh_entsize == 0`.
> (I've used the YAML below).
> 
> ```
> --- !ELF
> FileHeader:
>   Class: ELFCLASS64
>   Data:  ELFDATA2LSB
>   Type:  ET_EXEC
> Sections:
>   - Name:   .debug_str
>     Type:    SHT_PROGBITS
>     EntSize: 0
>     Size:    10
>     ShFlags: 0
> ```
> 
> When there are no flags, we have 'None` for `Flags`.
Thank you @grimar . I reproduce it using the YAML provided by you with a slight modification.

```
--- !ELF
FileHeader:
  Class: ELFCLASS64
  Data:  ELFDATA2LSB
  Type:  ET_EXEC
Sections:
  - Name:         .debug_str
    Type:         SHT_PROGBITS
    AddressAlign: 1
    Flags:        []
```

Because the default value of `AddressAlign` is 0. If we don't specify it, this function will return in the first `if` branch and it will not reach the second `if` branch where the optional value is dereferenced.

```
if (RawSec->Type != ELF::SHT_PROGBITS || !RawSec->Link.empty() ||
    RawSec->Info || RawSec->AddressAlign != 1 || RawSec->Address)
                    ^~~~~~~~~~~~~~~~~~~~~~~~~
  return true;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86867