[PATCH] D76684: [obj2yaml] - Refactor how we dump sections. NFCI.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:119-120
   StringRef Name = *NameOrErr;
+  if (Name.empty())
+    return "";
+
----------------
jhenderson wrote:
> grimar wrote:
> > jhenderson wrote:
> > > Could you explain this case, please, perhaps with a comment in the code?
> > It is for `obj2yaml\elf-null-section.yaml`. We have the following YAML description there:
> > 
> > ```
> > --- !ELF
> > FileHeader:
> >   Class:   ELFCLASS64
> >   Data:    ELFDATA2LSB
> >   Type:    ET_REL
> >   Machine: EM_X86_64
> > Sections:
> >   - Type: SHT_PROGBITS
> >     Name: .foo
> >   - Type:         SHT_NULL
> >     Name:         ''
> >     Flags:        [ SHF_ALLOC ]
> >     Link:         1
> >     Info:         2
> >     AddressAlign: 0x3
> >     Size:         0x4
> >     EntSize:      0x5
> >     Address:      0x6
> > ```
> > 
> > Note: SHT_NULL has empty name and it is the second `SHT_NULL` section in the object.
> > (first one is one at index 0, created implicitly).
> > 
> > We expect that obj2yaml produce the following;
> > 
> > ```
> > # SECOND-SEC-NEXT:   - Type:         SHT_NULL
> > # SECOND-SEC-NEXT:     Flags:        [ SHF_ALLOC ]
> > # SECOND-SEC-NEXT:     Address:      0x0000000000000006
> > # SECOND-SEC-NEXT:     Link:         .foo
> > # SECOND-SEC-NEXT:     AddressAlign: 0x0000000000000003
> > # SECOND-SEC-NEXT:     EntSize:      0x0000000000000005
> > # SECOND-SEC-NEXT:     Content:      '00000000'
> > # SECOND-SEC-NEXT:     Info:         0x0000000000000002
> > ```
> > 
> > But now since we started to dump `SHT_NULL` sections (we remove it later), obj2yaml produces:
> > ```
> > - Name: ' [1]'
> > ```
> > 
> > But it doesn't make sense I think. We should not uniquify empty names. I see no reasons for that.
> Seems reasonable for now. I doubt we'll have many (any?) cases where we want to reference multiple different sections with empty names.
After checking STT_SECTION, I think st_name=0 cases should be very rare.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76684