[PATCH] D21890: [ELF] - Do not error out when version declaration not found when building executable.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/SymbolTable.cpp:194
@@ +193,3 @@
+  // In this case we just create new version.
+  if (!Config->Shared && !Config->VersionScript) {
+    Config->SymbolVersions.push_back(elf::Version(Version));
----------------
ruiu wrote:
> grimar wrote:
> > ruiu wrote:
> > > Is `!Config->VersionScript` the same as `Config->SymbolVersions.empty()`?
> > Not anymore. Because of next line:
> > 
> > ```
> >  Config->SymbolVersions.push_back(elf::Version(Version));
> > ```
> > 
> > So since we can create a new version now during SymbolTable<ELFT>::insert(), it is not the same.
> I'm not still convinced why this condition is not `if (!Config->HasVersionScript)`. I do understand that you are just implementing the PR, but I think you need to justify the PR.
>I'm not still convinced why this condition is not if (!Config->HasVersionScript).

Because if we are linking DSO and have a symbol which contains version in name and do not have HasVersionScript,
that most probably an error, since it looks that version script was not specified by mistake.
Because linking DSO with versioned symbols without script is uncommon. It is so much uncommon that gnu linker restricts that,
so in real world we should not met that.
In opposite, it is common case for executables, that is what PR shows I believe. That is why I believe additional check of
!Config->Shared is a must here. Since we do not want and do not need to be less strict here than we should.


http://reviews.llvm.org/D21890