[PATCH] D94771: [Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 18 00:49:27 PST 2021
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
More information about the llvm-commits
mailing list