mirrors/llvm-project
0
0
Fork 0
mirror of https://github.com/llvm/llvm-project.git synced 2025-04-18 12:06:38 +00:00
Code Issues Projects Releases Wiki Activity

[libc++] Remove HIDE_FROM_ABI from virtual functions

Browse Source 
_LIBCPP_HIDE_FROM_ABI (which is what _LIBCPP_INLINE_VISIBILITY is) uses
ABI tags to avoid ODR violations when linking together object files
compiled against different versions of libc++. However, pointer
authentication uses the mangled name of the function to sign the
function pointer in the vtable, which means that the ABI tag effectively
changes how the pointers are signed.

This leads to PAC failures when passing an object that holds one of these
pointers in its vtable across an ABI boundary: one side will sign the
pointer using one function mangling (with one ABI tag), and the other
side will authenticate the pointer expecting it to have a different
mangled name, which won't work.

To make sure this does not regress in the future, this patch also adds
a clang-query test to detect incorrect applications of _LIBCPP_HIDE_FROM_ABI.

Differential Revision: https://reviews.llvm.org/D140453
This commit is contained in:
Louis Dionne 2022-12-21 10:08:54 -05:00
parent 2dcbd740ee
commit 5efc81166d
9 changed files with 71 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
libcxx/include/__config
View File

@ -610,6 +610,15 @@ typedef __char32_t char32_t;
// Note that we use _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION to ensure that we don't depend
// on _LIBCPP_HIDE_FROM_ABI methods of classes explicitly instantiated in the dynamic library.
//
// Also note that the _LIBCPP_HIDE_FROM_ABI_VIRTUAL macro should be used on virtual functions
// instead of _LIBCPP_HIDE_FROM_ABI. That macro does not use an ABI tag. Indeed, the mangled
// name of a virtual function is part of its ABI, since some architectures like arm64e can sign
// the virtual function pointer in the vtable based on the mangled name of the function. Since
// we use an ABI tag that changes with each released version, the mangled name of the virtual
// function would change, which is incorrect. Note that it doesn't make much sense to change
// the implementation of a virtual function in an ABI-incompatible way in the first place,
// since that would be an ABI break anyway. Hence, the lack of ABI tag should not be noticeable.
//
// TODO: We provide a escape hatch with _LIBCPP_NO_ABI_TAG for folks who want to avoid increasing
// the length of symbols with an ABI tag. In practice, we should remove the escape hatch and
// use compression mangling instead, see https://github.com/itanium-cxx-abi/cxx-abi/issues/70.
@ -620,6 +629,7 @@ typedef __char32_t char32_t;
# else
# define _LIBCPP_HIDE_FROM_ABI _LIBCPP_HIDDEN _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION
# endif
# define _LIBCPP_HIDE_FROM_ABI_VIRTUAL _LIBCPP_HIDDEN _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION
# ifdef _LIBCPP_BUILDING_LIBRARY
# if _LIBCPP_ABI_VERSION > 1

2
libcxx/include/__filesystem/filesystem_error.h
View File

@ -61,7 +61,7 @@ public:
filesystem_error(const filesystem_error&) = default;
~filesystem_error() override; // key function
_LIBCPP_INLINE_VISIBILITY
_LIBCPP_HIDE_FROM_ABI_VIRTUAL
const char* what() const noexcept override {
return __storage_->__what_.c_str();
}

2
libcxx/include/__functional/function.h
View File

@ -262,7 +262,7 @@ class __base<_Rp(_ArgTypes...)>
__base& operator=(const __base&);
public:
_LIBCPP_INLINE_VISIBILITY __base() {}
_LIBCPP_INLINE_VISIBILITY virtual ~__base() {}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL virtual ~__base() {}
virtual __base* __clone() const = 0;
virtual void __clone(__base*) const = 0;
virtual void destroy() _NOEXCEPT = 0;

4
libcxx/include/__memory/shared_ptr.h
View File

@ -991,7 +991,7 @@ struct __unbounded_array_control_block<_Tp[], _Alloc> : __shared_weak_count
return (__bytes + __align - 1) & ~(__align - 1);
}
_LIBCPP_HIDE_FROM_ABI
_LIBCPP_HIDE_FROM_ABI_VIRTUAL
~__unbounded_array_control_block() override { } // can't be `= default` because of the sometimes-non-trivial union member __data_
private:
@ -1058,7 +1058,7 @@ struct __bounded_array_control_block<_Tp[_Count], _Alloc>
std::__uninitialized_allocator_value_construct_n(__alloc_, std::addressof(__data_[0]), _Count);
}
_LIBCPP_HIDE_FROM_ABI
_LIBCPP_HIDE_FROM_ABI_VIRTUAL
~__bounded_array_control_block() override { } // can't be `= default` because of the sometimes-non-trivial union member __data_
private:

2
libcxx/include/future
View File

@ -1626,7 +1626,7 @@ class _LIBCPP_AVAILABILITY_FUTURE __packaged_task_base<_Rp(_ArgTypes...)>
public:
_LIBCPP_INLINE_VISIBILITY
__packaged_task_base() {}
_LIBCPP_INLINE_VISIBILITY
_LIBCPP_HIDE_FROM_ABI_VIRTUAL
virtual ~__packaged_task_base() {}
virtual void __move_to(__packaged_task_base*) _NOEXCEPT = 0;
virtual void destroy() = 0;

48
libcxx/include/locale
View File

@ -680,7 +680,7 @@ public:
static locale::id id;
protected:
_LIBCPP_HIDE_FROM_ABI ~num_get() override {}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL ~num_get() override {}
template <class _Fp>
_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
@ -1350,7 +1350,7 @@ public:
static locale::id id;
protected:
_LIBCPP_HIDE_FROM_ABI ~num_put() override {}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL ~num_put() override {}
virtual iter_type do_put(iter_type __s, ios_base& __iob, char_type __fl,
bool __v) const;
@ -1795,7 +1795,7 @@ public:
static locale::id id;
protected:
_LIBCPP_HIDE_FROM_ABI ~time_get() override {}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL ~time_get() override {}
virtual dateorder do_date_order() const;
virtual iter_type do_get_time(iter_type __b, iter_type __e, ios_base& __iob,
@ -2414,25 +2414,17 @@ public:
__time_get_storage<_CharT>(__nm) {}
protected:
_LIBCPP_HIDE_FROM_ABI ~time_get_byname() override {}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL ~time_get_byname() override {}
_LIBCPP_INLINE_VISIBILITY
dateorder do_date_order() const override {return this->__do_date_order();}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL dateorder do_date_order() const override {return this->__do_date_order();}
private:
_LIBCPP_INLINE_VISIBILITY
const string_type* __weeks() const override {return this->__weeks_;}
_LIBCPP_INLINE_VISIBILITY
const string_type* __months() const override {return this->__months_;}
_LIBCPP_INLINE_VISIBILITY
const string_type* __am_pm() const override {return this->__am_pm_;}
_LIBCPP_INLINE_VISIBILITY
const string_type& __c() const override {return this->__c_;}
_LIBCPP_INLINE_VISIBILITY
const string_type& __r() const override {return this->__r_;}
_LIBCPP_INLINE_VISIBILITY
const string_type& __x() const override {return this->__x_;}
_LIBCPP_INLINE_VISIBILITY
const string_type& __X() const override {return this->__X_;}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL const string_type* __weeks() const override {return this->__weeks_;}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL const string_type* __months() const override {return this->__months_;}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL const string_type* __am_pm() const override {return this->__am_pm_;}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL const string_type& __c() const override {return this->__c_;}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL const string_type& __r() const override {return this->__r_;}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL const string_type& __x() const override {return this->__x_;}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL const string_type& __X() const override {return this->__X_;}
};
extern template class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS time_get_byname<char>;
@ -2482,7 +2474,7 @@ public:
static locale::id id;
protected:
_LIBCPP_HIDE_FROM_ABI ~time_put() override {}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL ~time_put() override {}
virtual iter_type do_put(iter_type __s, ios_base&, char_type, const tm* __tm,
char __fmt, char __mod) const;
@ -2570,7 +2562,7 @@ public:
: time_put<_CharT, _OutputIterator>(__nm, __refs) {}
protected:
_LIBCPP_HIDE_FROM_ABI ~time_put_byname() override {}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL ~time_put_byname() override {}
};
extern template class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS time_put_byname<char>;
@ -2618,7 +2610,7 @@ public:
static const bool intl = _International;
protected:
_LIBCPP_HIDE_FROM_ABI ~moneypunct() override {}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL ~moneypunct() override {}
virtual char_type do_decimal_point() const {return numeric_limits<char_type>::max();}
virtual char_type do_thousands_sep() const {return numeric_limits<char_type>::max();}
@ -2668,7 +2660,7 @@ public:
: moneypunct<_CharT, _International>(__refs) {init(__nm.c_str());}
protected:
_LIBCPP_HIDE_FROM_ABI ~moneypunct_byname() override {}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL ~moneypunct_byname() override {}
char_type do_decimal_point() const override {return __decimal_point_;}
char_type do_thousands_sep() const override {return __thousands_sep_;}
@ -2796,7 +2788,7 @@ public:
static locale::id id;
protected:
_LIBCPP_HIDE_FROM_ABI ~money_get() override {}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL ~money_get() override {}
virtual iter_type do_get(iter_type __b, iter_type __e, bool __intl,
ios_base& __iob, ios_base::iostate& __err,
@ -3340,7 +3332,7 @@ public:
static locale::id id;
protected:
_LIBCPP_HIDE_FROM_ABI ~money_put() override {}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL ~money_put() override {}
virtual iter_type do_put(iter_type __s, bool __intl, ios_base& __iob,
char_type __fl, long double __units) const;
@ -3508,7 +3500,7 @@ public:
static locale::id id;
protected:
_LIBCPP_HIDE_FROM_ABI ~messages() override {}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL ~messages() override {}
virtual catalog do_open(const basic_string<char>&, const locale&) const;
virtual string_type do_get(catalog, int __set, int __msgid,
@ -3597,7 +3589,7 @@ public:
: messages<_CharT>(__refs) {}
protected:
_LIBCPP_HIDE_FROM_ABI ~messages_byname() override {}
_LIBCPP_HIDE_FROM_ABI_VIRTUAL ~messages_byname() override {}
};
extern template class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS messages_byname<char>;

6
libcxx/include/regex
View File

@ -1446,12 +1446,12 @@ public:
_LIBCPP_INLINE_VISIBILITY
__node() {}
_LIBCPP_INLINE_VISIBILITY
_LIBCPP_HIDE_FROM_ABI_VIRTUAL
virtual ~__node() {}
_LIBCPP_INLINE_VISIBILITY
_LIBCPP_HIDE_FROM_ABI_VIRTUAL
virtual void __exec(__state&) const {}
_LIBCPP_INLINE_VISIBILITY
_LIBCPP_HIDE_FROM_ABI_VIRTUAL
virtual void __exec_split(bool, __state&) const {}
};

4
libcxx/test/libcxx/clang_query.sh.cpp
View File

@ -17,6 +17,10 @@
// RUN: cat %t.output
// RUN: cat %t.output | wc -l | grep -Fxq 1
// RUN: clang-query -f %S/clang_query/abi_tag_on_virtual.query %s --use-color -- -Wno-unknown-warning-option %{compile_flags} -fno-modules > %t.output
// RUN: cat %t.output
// RUN: cat %t.output | wc -l | grep -Fxq 1
// Prevent <ext/hash_map> from generating deprecated warnings for this test.
#if defined(__DEPRECATED)
# undef __DEPRECATED

29
libcxx/test/libcxx/clang_query/abi_tag_on_virtual.query Normal file
View File

@ -0,0 +1,29 @@
#===------------------------------------------------------------------------===#
#
# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
# See https://llvm.org/LICENSE.txt for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
#
#===------------------------------------------------------------------------===#
# This clang-query test ensures that we don't place an abi_tag attribute on
# virtual functions. This can happen by mistakenly applying a macro like
# _LIBCPP_HIDE_FROM_ABI on a virtual function.
#
# The problem is that arm64e pointer authentication extensions use the mangled
# name of the function to sign the function pointer in the vtable, which means
# that the ABI tag effectively influences how the pointers are signed.
#
# This can lead to PAC failures when passing an object that holds one of these
# pointers in its vtable across an ABI boundary if the two sides have been compiled
# with different versions of libc++: one side will sign the pointer using one function
# mangling (with one ABI tag), and the other side will authenticate the pointer expecting
# it to have a different mangled name due to the ABI tag being different, which will crash.
#
# This test ensures that we don't re-introduce this issue in the code base.
match
cxxMethodDecl(isVirtual(),
hasAttr("attr::AbiTag"),
unless(isExpansionInSystemHeader())
)