[PATCH] D79984: [yaml2obj] - Add a technical prefix for each unnamed chunk.

[George Rimar via Phabricator via llvm-commits]

grimar marked 2 inline comments as done.
grimar added inline comments.


================
Comment at: llvm/test/tools/yaml2obj/ELF/section-link.yaml:32
+# ERR:      error: unknown section referenced: '.unknown1' by YAML section '.foo'
+# ERR-NEXT: error: unknown section referenced: '.unknown2' by YAML section ' [unnamed, index #2]'
+# ERR-NEXT: error: unknown section referenced: '.unknown3' by YAML section '.bar'
----------------
MaskRay wrote:
> grimar wrote:
> > MaskRay wrote:
> > > Can we omit `unnamed` and use `'' (index 2)`?
> > We can omit/add whatever we want into `[ ... ]` part.
> > 
> > This patch works because it automatically adds unique suffixes. What we use for
> > describing sections/symbols with the same name already:
> > 
> > ```
> >   - Name: '.foo [1]'
> >     Type: SHT_PROGBITS
> >   - Name: '.foo [2]'
> >     Type: SHT_PROGBITS
> > ```
> > 
> > I.e. we can add any `[ any text here ]` as suffix to a section name and the existent logic will
> > handle it properly. But we can't switch `[]` to `()` without doing additional changes.
> > 
> > It could be `' [index #2]'`, but not `'' [index 2]` nor `'' (index 2)`.
> > 
> > It can be possible to change (add an additional name parsing of a section name) in 
> > `toSectionIndex` to improve the message reported, but I am not sure it worth that additional complexity:
> > 
> > ```
> > template <class ELFT>
> > unsigned ELFState<ELFT>::toSectionIndex(StringRef S, StringRef LocSec,
> >                                         StringRef LocSym) {
> > ....
> > //// Add a code to parse LocSec/LocSym here.
> > 
> >   if (!LocSym.empty())
> >     reportError("unknown section referenced: '" + S + "' by YAML symbol '" +
> >                 LocSym + "'");
> >   else
> >     reportError("unknown section referenced: '" + S + "' by YAML section '" +
> >                 LocSec + "'");
> >   return 0;
> > }
> > ```
> OK, this name should be fine. But why doesn't `'' [index 2]"` work? (I am actually ok with either one but `#2` not working seems strange to me.)
See.

1) We have the following code:

```
StringRef llvm::ELFYAML::dropUniqueSuffix(StringRef S) {
  size_t SuffixPos = S.rfind(" [");
  if (SuffixPos == StringRef::npos)
    return S;
  return S.substr(0, SuffixPos);
}
```

I.e. we can safely add ` [` suffix and everything starting from it will be ignored.
The empty section name with a suffix looks like " [...", where `...` is any data or nothing.
e.g " [ index 2 ]"/" [ foo"/" [" are valid empty section names.

2)  The code that reports an error is:
```
reportError("unknown section referenced: '" + S + "' by YAML section '" +
                LocSec + "'");
```

I.e. if the section name is `X`, it prints:
```
unknown section referenced: '.foo' by YAML section 'X' 
```

How can it print `'' [index 2]`? For section name " [ index 2 ]" it would print:

```
unknown section referenced: '.foo' by YAML section ' [index 2]`
```




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

https://reviews.llvm.org/D79984