Add initial support for creating executables that use symbol versioning#670
Add initial support for creating executables that use symbol versioning#670parth-07 wants to merge 1 commit intoqualcomm:mainfrom
Conversation
038f473 to
c25d32e
Compare
9f60a6c to
7a3f2ce
Compare
7a3f2ce to
baa1b94
Compare
9878f1a to
43e7eab
Compare
This commit adds primitive support for creating executables that use shared libraries with symbol versioning information. Signed-off-by: Parth Arora <partaror@qti.qualcomm.com>
43e7eab to
e6c2931
Compare
quic-seaswara
left a comment
There was a problem hiding this comment.
some initial comments
|
|
||
| #ifdef ELD_ENABLE_SYMBOL_VERSIONING | ||
| if (PInfo.isDyn() && PInfo.outSymbol() && PInfo.outSymbol()->shouldIgnore()) | ||
| return false; |
There was a problem hiding this comment.
When for a default versioned symbol such as foo@@V1, both the canonical foo@V1 and the non-canonical foo symbols are selected by the symbol resolution, then we need to discard the non-canonical symbol so that we do not have duplicates in the symbol table. The symbols that were resolving to the non-canonical foo are made to resolve to the canonical symbol foo@V1. All of this happens during the symbol normalization (IRBuilder::normalizeSymbols).
|
|
||
| void setDynStrTabSection(ELFSection *S) { DynStrTabSection = S; } | ||
|
|
||
| llvm::StringRef getDynStringTable() const; |
There was a problem hiding this comment.
Move it out of symbol versioning for DynStrTabSection and getDynStringTable
There was a problem hiding this comment.
Can we move it out of ELD_ENABLE_SYMBOL_VERSIONING block when it is required? Currently, we only need these functions for symbol versioning.
|
|
||
| template <class ELFT> | ||
| llvm::StringRef getSymbolVersionName(uint32_t SymIdx, | ||
| ResolveInfo::Desc SymDesc) const { |
There was a problem hiding this comment.
can we pass in ResolveInfo here ?
There was a problem hiding this comment.
No, this function is called before the ResolveInfo for the symbol is properly created. And also, here we are referring to the input symbol. With ResolveInfo, we refer to the output symbol.
| auto VerNameOffset = | ||
| reinterpret_cast<const typename ELFT::Verdef *>(VerDefs[SymVerID]) | ||
| ->getAux() | ||
| ->vda_name; |
There was a problem hiding this comment.
Is there a function that we can use instead ?
There was a problem hiding this comment.
This functionality is directly from the LLVM. vda_name member is as specified in the official Elf_Verdaux data structure. Why would a function be better here?
| return VerSyms[SymIdx] == getSymbolVersionID(SymIdx); | ||
| } | ||
|
|
||
| uint16_t getOutputVernAuxID(uint16_t InputVerID) { |
There was a problem hiding this comment.
It cannot be a const function, it may need to resize an internal member.
| uint16_t getOutputVernAuxID(uint16_t InputVerID) { | ||
| if (InputVerID >= OutputVernAuxIDMap.size()) | ||
| OutputVernAuxIDMap.resize(InputVerID + 1, 0); | ||
| return OutputVernAuxIDMap[InputVerID]; |
There was a problem hiding this comment.
Dont use [ as it will create an unnecessary bucket, find and return the value
There was a problem hiding this comment.
The line numbers 98-99 ensures that [ will not create a new bucket.
| OutputVernAuxIDMap[InputVerID] = OutputVerID; | ||
| } | ||
|
|
||
| const std::vector<uint16_t> &getOutputVernAuxIDMap() const { |
There was a problem hiding this comment.
Adding comments around functions would be helpful to see how it needs to be used
| return OutputVernAuxIDMap; | ||
| } | ||
|
|
||
| const void *getVerDef(uint16_t InputVerID) { |
This commit adds primitive support for creating executables that use
shared libraries with symbol versioning information.