[PATCH] D94771: [Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/verdef-invalid.test:277
 # INVALID-VERDEF-GNU-NEXT:  Addr: 0000000000000000  Offset: 0x000040  Link: 3 (.dynsym)
-# INVALID-VERDEF-GNU-NEXT: warning: '[[FILE]]': unable to get a version for entry 1 of SHT_GNU_versym section with index 1: invalid SHT_GNU_verdef section with index 2: version definition 1 goes past the end of the section
+# INVALID-VERDEF-GNU-NEXT: warning: '[[FILE]]': invalid SHT_GNU_verdef section with index 2: version definition 1 goes past the end of the section
 # INVALID-VERDEF-GNU-NEXT:   000: 0 (*local*) 2 (<corrupt>)
----------------
jhenderson wrote:
> What's happened to these error messages?
It is because of changes in `GNUELFDumper<ELFT>::printVersionSymbolSection`.

Previoulsy, `ELFDumper<ELFT>::getSymbolVersionByIndex` loaded the version mapping internally
(`ELFDumper<ELFT>::LoadVersionMap()`), because it stored it in `VersionMap` member.

The new code moves `getSymbolVersionByIndex` to `ELFFile<ELFT>`. And now it hase `VersionMap` argument:

```
  Expected<StringRef> getSymbolVersionByIndex(
      uint32_t SymbolVersionIndex, bool &IsDefault,
      SmallVector<Optional<VersionEntry>, 16> &VersionMap) const;
```

So now its up to caller to decide if `VersionMap` is stored and where. Doesn't seem like we want to add a
`VersionMap` member to `ELFFile<ELFT>`.

With that currently we might have an error when we call 

```
Expected<SmallVector<Optional<VersionEntry>, 16> *> MapOrErr =
      getVersionMap();
```

which might occur a bit earlier in the code, i.e. before we start iterating over version entries.

Note it doesn't seem to degrade the diagnostics reported. The reason ("invalid SHT_GNU_verdef...") is still printed and remains the same.
It is probably even better as it general and reported once for all entries now.




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

https://reviews.llvm.org/D94771