This is an archive of the discontinued LLVM Phabricator instance.

Differential D130715

[libc++][AIX] Use non-unique implementation for typeinfo comparison
ClosedPublic

Authored by xingxue on Jul 28 2022, 9:26 AM.

Details

Reviewers
ldionne
Mordante
cebowleratibm
hubert.reinterpretcast
daltenty
philnik
Group Reviewers
Restricted Project
Commits
rGaeb1c98f4c10: [libc++][AIX] Use non-unique implementation for typeinfo comparison
Summary

The AIX linker does not merge typeinfos when shared libraries are involved, which causes address comparison to fail although the types are the same. This patch changes to use the non-unique implementation for typeinfo comparison for AIX.

Diff Detail

Event Timeline

xingxue created this revision.Jul 28 2022, 9:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 28 2022, 9:26 AM
Herald added a subscriber: smeenai. · View Herald Transcript
xingxue requested review of this revision.Jul 28 2022, 9:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 28 2022, 9:26 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
hubert.reinterpretcast accepted this revision.Jul 28 2022, 9:44 AM

LGTM.

philnik accepted this revision.Jul 28 2022, 9:46 AM
philnik added a subscriber: philnik.

LGTM % nit.

libcxx/include/typeinfo
177
This revision is now accepted and ready to land.Jul 28 2022, 9:46 AM
This revision was landed with ongoing or failed builds.Jul 28 2022, 10:18 AM
Closed by commit rGaeb1c98f4c10: [libc++][AIX] Use non-unique implementation for typeinfo comparison (authored by xingxue). · Explain Why
This revision was automatically updated to reflect the committed changes.
xingxue added a commit: rGaeb1c98f4c10: [libc++][AIX] Use non-unique implementation for typeinfo comparison.
xingxue marked an inline comment as done.Jul 28 2022, 10:20 AM
xingxue added inline comments.
libcxx/include/typeinfo
177

Changed the comment as suggested. Thanks, @philnik!

Harbormaster completed remote builds in B178093: Diff 448358.Jul 28 2022, 1:02 PM

Revision Contents

PathSize
libcxx/
include/
4 lines

Diff 448358

libcxx/include/typeinfo

Show First 20 Lines • Show All 168 Lines • ▼ Show 20 Lines
// a unique RTTI: the RTTI is emitted with linkonce_odr linkage and is assumed // a unique RTTI: the RTTI is emitted with linkonce_odr linkage and is assumed
// to be deduplicated by the linker within the linked image. Across linked image // to be deduplicated by the linker within the linked image. Across linked image
// boundaries, such types are thus considered different types. // boundaries, such types are thus considered different types.
// This value can be overriden in the __config_site. When it's not overriden, // This value can be overriden in the __config_site. When it's not overriden,
// we pick a default implementation based on the platform here. // we pick a default implementation based on the platform here.
#ifndef _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION #ifndef _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION
// Windows binaries can't merge typeinfos, so use the NonUnique implementation. // Windows or AIX binaries can't merge typeinfos, so use the NonUnique implementation.
philnikUnsubmitted
Not Done
ReplyInline Actions
#ifndef _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION
- // Windows or AIX binaries can't merge typeinfos, so use the NonUnique implementation.
+ // Windows and AIX binaries can't merge typeinfos, so use the NonUnique implementation.
# if defined(_LIBCPP_OBJECT_FORMAT_COFF) || defined(_LIBCPP_OBJECT_FORMAT_XCOFF)
philnik:
xingxueAuthorUnsubmitted
Done
ReplyInline Actions

Changed the comment as suggested. Thanks, @philnik!

xingxue: Changed the comment as suggested. Thanks, @philnik!
# ifdef _LIBCPP_OBJECT_FORMAT_COFF # if defined(_LIBCPP_OBJECT_FORMAT_COFF) || defined(_LIBCPP_OBJECT_FORMAT_XCOFF)
# define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 2 # define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 2
// On arm64 on Apple platforms, use the special NonUniqueARMRTTIBit implementation. // On arm64 on Apple platforms, use the special NonUniqueARMRTTIBit implementation.
# elif defined(__APPLE__) && defined(__LP64__) && !defined(__x86_64__) # elif defined(__APPLE__) && defined(__LP64__) && !defined(__x86_64__)
# define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 3 # define _LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION 3
// On all other platforms, assume the Itanium C++ ABI and use the Unique implementation. // On all other platforms, assume the Itanium C++ ABI and use the Unique implementation.
# else # else
▲ Show 20 Lines • Show All 195 Lines • Show Last 20 Lines