[Bug c++/103490] New: Linkage type of typeinfo of polymorphic object with OOL functions

[Bug c++/103490] New: Linkage type of typeinfo of polymorphic object with OOL functions

[lizekun.zek at bytedance dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103490

            Bug ID: 103490
           Summary: Linkage type of typeinfo of polymorphic object with
                    OOL functions
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: lizekun.zek at bytedance dot com
  Target Milestone: ---

Hi guys,
  I found that the linkage of typeinfo of polymorphic object with OOL-functions
is weak, while that is strong for clang, and I want to attach a patch to make
the behavior of the two compilers consistent. However, I can't find the clear
definition of this in Itanium C++ ABI, so here are the questions:

1.It is easy to understand that we make the object with inlined-functions weak, 
  because it usually appears in a header file and may be defined several times, 
  so when functions are out-of-line, why do we make it weak?

2.In fact, I prefer the gcc way, are there any cases that show gcc is 
  better?

This is a case to show the issue.

---
test.h

class A{
public:
    virtual const int getA(){} ;
    virtual const int getB(){} ;
};

---
testA.cpp

#include<iostream>
#include "test.h"
using namespace std;

class Test : public A {
public:
    virtual const int getA() override;
};
const int Test ::getA() {
    return 1;
}


int main() {
    A * t=new Test();;
    cout << dynamic_cast<Test*>(t);
}
---

testB.cpp

#include "test.h"

class Test : A {
public:
    virtual const int getB() override;
};
const int Test :: getB() {
    return 2;
}

[Bug c++/103490] Linkage type of typeinfo of polymorphic object with OOL functions

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103490

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable
The virtual table for a class is emitted in the same object containing the
definition of its key function, i.e. the first non-pure virtual function that
is not inline at the point of class definition. If there is no key function, it
is emitted everywhere used. The emitted virtual table includes the full virtual
table group for the class, any new construction virtual tables required for
subobjects, and the VTT for the class. They are emitted in a COMDAT group, with
the virtual table mangled name as the identifying symbol. Note that if the key
function is not declared inline in the class definition, but its definition
later is always declared inline, it will be emitted in every object containing
the definition. A constexpr or consteval function is always declared constexpr
or consteval on its first declaration, and is implicitly inline, so is never a
key function.




I think GCC is correct here and LLVM is wrong due to: "They are emitted in a
COMDAT group, with the virtual table mangled name as the identifying symbol."

This was done because of things like:
struct Test1 {
    virtual int getB();
    virtual int getA();
};
inline int Test1 :: getB() {
    return 2;
}

and only the header have Test1 and such.

[Bug c++/103490] Linkage type of typeinfo of polymorphic object with OOL functions

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103490

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The bug is definitely on the clang side. GCC is following the ABI to letter of
having all vtables in comdat.



  if (CGM.supportsCOMDAT() && VTable->isWeakForLinker())
    VTable->setComdat(CGM.getModule().getOrInsertComdat(VTable->getName()));

Should not check isWeakForLinker. All vtable needs to be comdat:
They are emitted in a COMDAT group, with the virtual table mangled name as the
identifying symbol.


This is also true for RTTI even:
The RTTI std::type_info structures for complete class types and basic types are
emitted in COMDAT groups identified by their mangled names.

[Bug c++/103490] Linkage type of typeinfo of polymorphic object with OOL functions

[rjmccall at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103490

John McCall <rjmccall at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rjmccall at gmail dot com

--- Comment #3 from John McCall <rjmccall at gmail dot com> ---
I'm not sure I agree that that's the letter of the law, but even taken that as
given, I think I would argue that it's a defect rather than something that has
to be followed.  The purpose of the key-function optimization is to avoid the
overheads of having to redundantly emit and unique v-tables.  A lot of uniquing
can happen at link time, of course, but not all, and optimizing load times is
important.

Does GCC ever emit actually v-tables redundantly which would otherwise be
unique under the key-function optimization?  If not, it's a fixable defect. 
The sample code here does not count as an observation of the difference, as
it's not at all a well-formed program under C++ rules.

[Bug c++/103490] Linkage type of typeinfo of polymorphic object with OOL functions

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103490

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to John McCall from comment #3)
> I'm not sure I agree that that's the letter of the law, but even taken that
> as given, I think I would argue that it's a defect rather than something
> that has to be followed.  The purpose of the key-function optimization is to
> avoid the overheads of having to redundantly emit and unique v-tables.  A
> lot of uniquing can happen at link time, of course, but not all, and
> optimizing load times is important.

https://itanium-cxx-abi.github.io/cxx-abi/cxx-closed.html#B5

There was an open issue a long time ago which resolved this. If you think there
should be a new defect opened against the ABI be my guest but the resolution of
the original issue was resolved to say comdat in all cases.

"Resolution: Vtables will be emitted with the key function (first virtual
function that is not inline at the point of class definition), if any. If no
key function, emit everywhere used (i.e. referred to by name). Place in a
comdat group in all cases."

Given clang is the only compiler out there that is not complaint I think clang
should change.