Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

D4/D5 destructor and other manglings not specified #73

Open
jengelh opened this issue Mar 19, 2019 · 5 comments
Open

D4/D5 destructor and other manglings not specified #73

jengelh opened this issue Mar 19, 2019 · 5 comments

Comments

@jengelh
Copy link

jengelh commented Mar 19, 2019

Judging from the GCC and clang source codes, gcc/cp/mangle.c and lib/AST/ItaniumMangle.c, these compilers can produce additional manglings currently missing from the cxx-abi documents.

D4: “old-style "[unified]" destructor” / maybe-in-charge destructor [gcc]
D5: “D5 is a comdat name with D1, D2 and, if virtual, D0 in it.” [clang], also https://stackoverflow.com/questions/19485012/what-is-a-destructor-group-symbol-in-gcc-name-mangling
CI: something to do with inheriting constructors [gcc]
C4: “old-style "[unified]" constructor” / maybe-in-charge constructor [gcc]
C5: “C5 is a comdat name with C1 and C2 in it.” [clang]

Does their absence from cxx-abi indicate that these constitute vendor-specific extensions which just failed to make use of the v/U mangling character? (That is to say, _ZN3FooD4Ev should have been something like _ZN3Foov0Ev.) Should C4/C5/D4/D5/CI be marked in cxx-abi as reserved nonetheless, so as to not cause future problems for compilers?

@rjmccall
Copy link
Collaborator

Sorry for letting this sit for a long time. Technically, everything not explicitly set aside for vendor extensions is reserved for future use by the ABI. This is the first I've heard of these manglings, but I would appreciate if vendors asked for these manglings to be dedicated to these purposes when needed.

@zygoloid @jicama

@rjmccall
Copy link
Collaborator

Note that the ABI does already reserve a name for a concept, the "complete object allocating constructor", that it does not actually specify rules for and is definitely not a mandatory part of the ABI. (It is presumably meant to have the effect of return new T(args...) and might be a useful code-size optimization despite not being mandatory in the ABI.)

@zygoloid
Copy link
Contributor

The CI mangling is covered by issue #48.

Do we need to document C4/D4? (Are they still in use?)

C5/D5 were proposed to the ABI list here: https://www.mail-archive.com/cxx-abi-dev@codesourcery.com/msg00168.html (complete with a patch).

@rjmccall
Copy link
Collaborator

Ah, I knew I should've searched the active issues. Opened an issue for C5/D5 here: #92

@jengelh
Copy link
Author

jengelh commented Aug 5, 2022

Judging from today's gcc code, C/D4 are still used, unlike C/D5.
58a644cfdee53275e3d07eff0c2126dc88b87aef: libcc1/libcp1plugin.cc: plugin_define_cdtor_clone
Even if vendor-specific, just mentioning that such existed at some point in time in a HISTORIC section or so would be quite useful - if only to avoid reusing C/D4.

jrguzman-ms pushed a commit to msft-mirror-aosp/platform.external.libabigail that referenced this issue Jan 24, 2023
While looking at something else, I noticed this spurious change
report:

    1 member function deletion:
      'method virtual tbb::internal::concurrent_queue_base_v3::~concurrent_queue_base_v3(int)' at concurrent_queue.cpp:361:1
    1 member function insertion:
		'method virtual tbb::internal::concurrent_queue_base_v3::~concurrent_queue_base_v3(int)' at concurrent_queue.cpp:370:1

This is when running tests/runtestdiffpkg and the result is in
tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt.

To understand what is going on, one need to refer to the definitions
of the C++ ABI at
https://itanium-cxx-abi.github.io/cxx-abi/abi.html#definitions.

When a class has a virtual destructor, historically, there might be 3
different and co-existing actual implementations of the destructors:
the D0, D1 and D2 destructors.  There might even be a D3 destructor,
actually.

In itanium-cxx-abi/cxx-abi#73, one can see
that there might be new D4 and D5 destructors.  Each one of them might
replace the set of the D0,D1,D2 destructors.

So, in a new version of a binary, the virtual D4 destructor might
replace the previous ones, without it being an ABI issue.

The switch to the D4 virtual destructor is what libabigail is naively
reporting in the change report above.

This patch detects this kind of changes in the pipeline when the edit
script from the diff2 algorithm is interpreted for virtual member
functions and drops it on the floor.

	* src/abg-comparison.cc (find_virtual_dtor_in_map): Define new
	static function.
	(class_diff::ensure_lookup_tables_populated): If a virtual
	destructor is removed from the old binary version but is added to
	the new one (but through a different name), let's assume the
	virtual destructor is still there so there is no ABI issue
	from that point of view.
	* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt: Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants