[PATCH] D36573: [ELF] - Treat .gnu.linkonce sections as COMDAT

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/InputFiles.cpp:496-502
   // The linkonce feature is a sort of proto-comdat. Some glibc i386 object
   // files contain definitions of symbol "__x86.get_pc_thunk.bx" in linkonce
-  // sections. Drop those sections to avoid duplicate symbol errors.
+  // sections. Section usually named as '.gnu.linkonce.TYPE.NAME', where NAME
+  // is a symbol name this section contains and also a name of comdat group.
+  // Usually NAME does not contain dots ('.'), but case above is special.
+  // Since this is temporary hack, we handle only ".gnu.linkonce.t.*" sections
+  // as minimal case for simplicity.
----------------
ruiu wrote:
> ".gnu.linkonce.t" is not the only ".gnu.linkonce" section. There are other sections such as ".gnu.linkonce.r". Why can you ignore them but only handle ".gnu.linkonce.t"? It seems something is missing.
In all descriptions I found it was said that we should expect
`.gnu.linkonce.NAME` or `.gnu.linkonce.TYPE.NAME`, for example:

(http://www.airs.com/blog/archives/52)
//"Any section whose name starts with “.gnu.linkonce.” is a COMDAT section. The associated string is simply the section name itself. Thus the inline function f1 would be put into the section “.gnu.linkonce.f1”"//

In that case all we would need here is to take the string `NAME` which follows last `.`.
But in real life situation is different and I did not find exact documentation about naming.

gold linker has comment saying that for `.gnu.linkonce.t.__i686.get_pc_thunk.bx`
`__i686.get_pc_thunk.bx` should be used as function name. That corresponds
to documentation probably.  But at the same time it says that for case of `.gnu.linkonce.d.rel.ro.local`
the name should **not** be `rel.ro.local`, but `local`. So we generally can not simply take whole
string following the last `.` for taking comdat group name.

And because this patch was a hack to fix exact user request, I decided to support case
that is simple, clear for me and enough for fixing request.

But anyways, this patch was abandoned, do you want to resurrect it ?




https://reviews.llvm.org/D36573