llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libunwind
Author: safocl
<details>
<summary>Changes</summary>
1. Performing type _reinterpret-casts_ between pointers to objects that are not
_pointers-interconvertible_ and then using the results is incorrect usage.
2. Only the “array of N unsigned char” or of type “array of N std::byte” types
can used as storage (in general case). To convert a pointer to a structure
object containing a byte array into a pointer to an object explicitly created
in that storage, std::launder must be called on that "newly typed" pointer.
```cpp
struct S {
unsigned char data[_LIBUNWIND_CURSOR_SIZE * sizeof(uint64_t)];
} s;
struct S2 { int i; };
new(s.data) S2(42);
auto s2_ptr1 = reinterpret_cast<S2*>(s.data);
auto s2_ptr2 = reinterpret_cast<S2*>(s); // this pointer is not changed
([expr.static.cast#<!-- -->12]) and points to s (struct S)
auto s2_ptr3 = std::launder(reinterpret_cast<S2*>(s));
int i1 = s2_ptr1->i; // OK
int i3 = s2_ptr3->i; // OK (?)
int i2 = s2_ptr2->i; // this is undefined behavior
```
Also a pointer to object of type uint16[2] cannot be reinterpreted as uint32 --
this is undefined behavior (https://godbolt.org/z/vY1zKEvo6).
Documentation links:
https://eel.is/c++draft/intro.object#<!-- -->3
https://eel.is/c++draft/basic.compound#<!-- -->5
https://eel.is/c++draft/basic.compound#<!-- -->6
https://eel.is/c++draft/basic.lval#<!-- -->11
https://eel.is/c++draft/expr.static.cast#<!-- -->12
https://eel.is/c++draft/expr.reinterpret.cast#<!-- -->7
https://en.cppreference.com/w/cpp/utility/launder.html
https://en.cppreference.com/w/cpp/language/reinterpret_cast.html
related:
https://gitlab.winehq.org/wine/wine/-/merge_requests/10471
https://github.com/cplusplus/CWG/issues/874
https://github.com/llvm/llvm-project/issues/189040
---
Full diff: https://github.com/llvm/llvm-project/pull/189209.diff
4 Files Affected:
- (modified) libunwind/include/libunwind.h (+7-1)
- (modified) libunwind/src/Unwind-seh.cpp (+9-12)
- (modified) libunwind/src/UnwindCursor.hpp (+17-6)
- (modified) libunwind/src/libunwind.cpp (+15-16)
``````````diff
diff --git a/libunwind/include/libunwind.h b/libunwind/include/libunwind.h
index 6c2dfe58e53bf..5a3e0c4e2f7bd 100644
--- a/libunwind/include/libunwind.h
+++ b/libunwind/include/libunwind.h
@@ -176,7 +176,13 @@ struct unw_context_t {
typedef struct unw_context_t unw_context_t;
struct unw_cursor_t {
- uint64_t data[_LIBUNWIND_CURSOR_SIZE];
+ /* Only the “array of N unsigned char” or of type “array of N std::byte”
types can used as
+ storage (https://eel.is/c++draft/intro.object#3) */
+ union {
+ unsigned char data[_LIBUNWIND_CURSOR_SIZE * sizeof(uint64_t)];
+ /* `aligner` exists only for alignment */
+ uint64_t aligner__[_LIBUNWIND_CURSOR_SIZE];
+ } u;
} LIBUNWIND_CURSOR_ALIGNMENT_ATTR;
typedef struct unw_cursor_t unw_cursor_t;
diff --git a/libunwind/src/Unwind-seh.cpp b/libunwind/src/Unwind-seh.cpp
index 0b1930b44d1c6..3435e8b81396e 100644
--- a/libunwind/src/Unwind-seh.cpp
+++ b/libunwind/src/Unwind-seh.cpp
@@ -498,24 +498,21 @@ _Unwind_GetRegionStart(struct _Unwind_Context *context) {
static int __unw_init_seh(unw_cursor_t *cursor, CONTEXT *context) {
#ifdef _LIBUNWIND_TARGET_X86_64
- new (reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_x86_64>
*>(cursor))
+ auto *co = new (reinterpret_cast<UnwindCursor<LocalAddressSpace,
Registers_x86_64> *>(cursor->u.data))
UnwindCursor<LocalAddressSpace, Registers_x86_64>(
context, LocalAddressSpace::sThisAddressSpace);
- auto *co = reinterpret_cast<AbstractUnwindCursor *>(cursor);
co->setInfoBasedOnIPRegister();
return UNW_ESUCCESS;
#elif defined(_LIBUNWIND_TARGET_ARM)
- new (reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_arm>
*>(cursor))
+ auto *co = new (reinterpret_cast<UnwindCursor<LocalAddressSpace,
Registers_arm> *>(cursor->u.data))
UnwindCursor<LocalAddressSpace, Registers_arm>(
context, LocalAddressSpace::sThisAddressSpace);
- auto *co = reinterpret_cast<AbstractUnwindCursor *>(cursor);
co->setInfoBasedOnIPRegister();
return UNW_ESUCCESS;
#elif defined(_LIBUNWIND_TARGET_AARCH64)
- new (reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_arm64>
*>(cursor))
+ auto *co = new (reinterpret_cast<UnwindCursor<LocalAddressSpace,
Registers_arm64> *>(cursor->u.data))
UnwindCursor<LocalAddressSpace, Registers_arm64>(
context, LocalAddressSpace::sThisAddressSpace);
- auto *co = reinterpret_cast<AbstractUnwindCursor *>(cursor);
co->setInfoBasedOnIPRegister();
return UNW_ESUCCESS;
#else
@@ -525,11 +522,11 @@ static int __unw_init_seh(unw_cursor_t *cursor, CONTEXT
*context) {
static DISPATCHER_CONTEXT *__unw_seh_get_disp_ctx(unw_cursor_t *cursor) {
#ifdef _LIBUNWIND_TARGET_X86_64
- return reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_x86_64>
*>(cursor)->getDispatcherContext();
+ return reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_x86_64>
*>(cursor->u.data)->getDispatcherContext();
#elif defined(_LIBUNWIND_TARGET_ARM)
- return reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_arm>
*>(cursor)->getDispatcherContext();
+ return reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_arm>
*>(cursor->u.data)->getDispatcherContext();
#elif defined(_LIBUNWIND_TARGET_AARCH64)
- return reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_arm64>
*>(cursor)->getDispatcherContext();
+ return reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_arm64>
*>(cursor->u.data)->getDispatcherContext();
#else
return nullptr;
#endif
@@ -538,11 +535,11 @@ static DISPATCHER_CONTEXT
*__unw_seh_get_disp_ctx(unw_cursor_t *cursor) {
static void __unw_seh_set_disp_ctx(unw_cursor_t *cursor,
DISPATCHER_CONTEXT *disp) {
#ifdef _LIBUNWIND_TARGET_X86_64
- reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_x86_64>
*>(cursor)->setDispatcherContext(disp);
+ reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_x86_64>
*>(cursor->u.data)->setDispatcherContext(disp);
#elif defined(_LIBUNWIND_TARGET_ARM)
- reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_arm>
*>(cursor)->setDispatcherContext(disp);
+ reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_arm>
*>(cursor->u.data)->setDispatcherContext(disp);
#elif defined(_LIBUNWIND_TARGET_AARCH64)
- reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_arm64>
*>(cursor)->setDispatcherContext(disp);
+ reinterpret_cast<UnwindCursor<LocalAddressSpace, Registers_arm64>
*>(cursor->u.data)->setDispatcherContext(disp);
#endif
}
diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index ff92c9e0a844a..2e207d692f184 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -2114,13 +2114,24 @@ bool UnwindCursor<A, R>::getInfoFromSEH(pint_t pc) {
// follows the UNWIND_INFO. (This follows how both Clang and MSVC emit
// these structures.)
// N.B. UNWIND_INFO structs are DWORD-aligned.
- uint32_t lastcode = (xdata->CountOfCodes + 1) & ~1;
- const uint32_t *handler = reinterpret_cast<uint32_t
*>(&xdata->UnwindCodes[lastcode]);
- _info.lsda = reinterpret_cast<unw_word_t>(handler+1);
+
+ // uint32_t lastcode = (xdata->CountOfCodes + 1) & ~1;
+ // NOTE: lastcode can be equal to or greater than 2, then accessing
UnwindCodes[lastcode] is
+ // out of bound (i.e. undefined behavior). The external memory outside
the class object
+ // cannot be reached from a pointer to a subobject. The only valid case
is when CountOfCodes
+ // is 0, and then lastcode will be equal 0.
+ // However, `reinterpret_cast` from a pointer to uint16[2] to a pointer
to uint32 is not
+ // allowed in any case (https://eel.is/c++draft/basic.compound#5,
+ // https://eel.is/c++draft/basic.lval#11).
(https://godbolt.org/z/vY1zKEvo6)
+ uint32_t handler;
+ memcpy(&handler, xdata->UnwindCodes, sizeof(uint32_t));
+ // FIXME: Does `xdata` actually point to an array of UNWIND_INFO?
+ // https://github.com/cplusplus/CWG/issues/874
+ _info.lsda = reinterpret_cast<unw_word_t>(xdata+1);
_dispContext.HandlerData = reinterpret_cast<void *>(_info.lsda);
_dispContext.LanguageHandler =
- reinterpret_cast<EXCEPTION_ROUTINE *>(base + *handler);
- if (*handler) {
+ reinterpret_cast<EXCEPTION_ROUTINE *>(base + handler);
+ if (handler) {
_info.handler =
reinterpret_cast<unw_word_t>(__libunwind_seh_personality);
} else
_info.handler = 0;
@@ -3351,7 +3362,7 @@ bool UnwindCursor<A, R>::isReadableAddr(const pint_t
addr) const {
#if defined(_LIBUNWIND_USE_CET) || defined(_LIBUNWIND_USE_GCS)
extern "C" void *__libunwind_shstk_get_registers(unw_cursor_t *cursor) {
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
return co->get_registers();
}
#endif
diff --git a/libunwind/src/libunwind.cpp b/libunwind/src/libunwind.cpp
index 0b9d56ec805bc..fff4cc8006c0d 100644
--- a/libunwind/src/libunwind.cpp
+++ b/libunwind/src/libunwind.cpp
@@ -83,11 +83,10 @@ _LIBUNWIND_HIDDEN int __unw_init_local(unw_cursor_t *cursor,
# error Architecture not supported
#endif
// Use "placement new" to allocate UnwindCursor in the cursor buffer.
- new (reinterpret_cast<UnwindCursor<LocalAddressSpace, REGISTER_KIND>
*>(cursor))
+ auto *co = new (reinterpret_cast<UnwindCursor<LocalAddressSpace,
REGISTER_KIND> *>(cursor->u.data))
UnwindCursor<LocalAddressSpace, REGISTER_KIND>(
context, LocalAddressSpace::sThisAddressSpace);
#undef REGISTER_KIND
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
co->setInfoBasedOnIPRegister();
return UNW_ESUCCESS;
@@ -100,7 +99,7 @@ _LIBUNWIND_HIDDEN int __unw_get_reg(unw_cursor_t *cursor,
unw_regnum_t regNum,
_LIBUNWIND_TRACE_API("__unw_get_reg(cursor=%p, regNum=%d, &value=%p)",
static_cast<void *>(cursor), regNum,
static_cast<void *>(value));
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
if (co->validReg(regNum)) {
*value = co->getReg(regNum);
return UNW_ESUCCESS;
@@ -116,7 +115,7 @@ _LIBUNWIND_HIDDEN int __unw_set_reg(unw_cursor_t *cursor,
unw_regnum_t regNum,
")",
static_cast<void *>(cursor), regNum, value);
typedef LocalAddressSpace::pint_t pint_t;
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
if (co->validReg(regNum)) {
// special case altering IP to re-find info (being called by personality
// function)
@@ -188,7 +187,7 @@ _LIBUNWIND_HIDDEN int __unw_get_fpreg(unw_cursor_t *cursor,
unw_regnum_t regNum,
_LIBUNWIND_TRACE_API("__unw_get_fpreg(cursor=%p, regNum=%d, &value=%p)",
static_cast<void *>(cursor), regNum,
static_cast<void *>(value));
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
if (co->validFloatReg(regNum)) {
*value = co->getFloatReg(regNum);
return UNW_ESUCCESS;
@@ -207,7 +206,7 @@ _LIBUNWIND_HIDDEN int __unw_set_fpreg(unw_cursor_t *cursor,
unw_regnum_t regNum,
_LIBUNWIND_TRACE_API("__unw_set_fpreg(cursor=%p, regNum=%d, value=%g)",
static_cast<void *>(cursor), regNum, value);
#endif
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
if (co->validFloatReg(regNum)) {
co->setFloatReg(regNum, value);
return UNW_ESUCCESS;
@@ -219,7 +218,7 @@ _LIBUNWIND_WEAK_ALIAS(__unw_set_fpreg, unw_set_fpreg)
/// Move cursor to next frame.
_LIBUNWIND_HIDDEN int __unw_step(unw_cursor_t *cursor) {
_LIBUNWIND_TRACE_API("__unw_step(cursor=%p)", static_cast<void *>(cursor));
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
return co->step();
}
_LIBUNWIND_WEAK_ALIAS(__unw_step, unw_step)
@@ -229,7 +228,7 @@ _LIBUNWIND_WEAK_ALIAS(__unw_step, unw_step)
extern "C" _LIBUNWIND_HIDDEN int __unw_step_stage2(unw_cursor_t *cursor) {
_LIBUNWIND_TRACE_API("__unw_step_stage2(cursor=%p)",
static_cast<void *>(cursor));
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
return co->step(true);
}
@@ -238,7 +237,7 @@ _LIBUNWIND_HIDDEN int __unw_get_proc_info(unw_cursor_t
*cursor,
unw_proc_info_t *info) {
_LIBUNWIND_TRACE_API("__unw_get_proc_info(cursor=%p, &info=%p)",
static_cast<void *>(cursor), static_cast<void *>(info));
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
co->getInfo(info);
if (info->end_ip == 0)
return UNW_ENOINFO;
@@ -273,7 +272,7 @@ _LIBUNWIND_HIDDEN int __unw_resume(unw_cursor_t *cursor) {
// Inform the ASan runtime that now might be a good time to clean stuff up.
__asan_handle_no_return();
#endif
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
co->jumpto();
return UNW_EUNSPEC;
}
@@ -285,7 +284,7 @@ _LIBUNWIND_HIDDEN int __unw_get_proc_name(unw_cursor_t
*cursor, char *buf,
_LIBUNWIND_TRACE_API("__unw_get_proc_name(cursor=%p, &buf=%p, bufLen=%lu)",
static_cast<void *>(cursor), static_cast<void *>(buf),
static_cast<unsigned long>(bufLen));
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
if (co->getFunctionName(buf, bufLen, offset))
return UNW_ESUCCESS;
return UNW_EUNSPEC;
@@ -297,7 +296,7 @@ _LIBUNWIND_HIDDEN int __unw_is_fpreg(unw_cursor_t *cursor,
unw_regnum_t regNum) {
_LIBUNWIND_TRACE_API("__unw_is_fpreg(cursor=%p, regNum=%d)",
static_cast<void *>(cursor), regNum);
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
return co->validFloatReg(regNum);
}
_LIBUNWIND_WEAK_ALIAS(__unw_is_fpreg, unw_is_fpreg)
@@ -307,7 +306,7 @@ _LIBUNWIND_HIDDEN const char *__unw_regname(unw_cursor_t
*cursor,
unw_regnum_t regNum) {
_LIBUNWIND_TRACE_API("__unw_regname(cursor=%p, regNum=%d)",
static_cast<void *>(cursor), regNum);
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
return co->getRegisterName(regNum);
}
_LIBUNWIND_WEAK_ALIAS(__unw_regname, unw_regname)
@@ -316,7 +315,7 @@ _LIBUNWIND_WEAK_ALIAS(__unw_regname, unw_regname)
_LIBUNWIND_HIDDEN int __unw_is_signal_frame(unw_cursor_t *cursor) {
_LIBUNWIND_TRACE_API("__unw_is_signal_frame(cursor=%p)",
static_cast<void *>(cursor));
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
return co->isSignalFrame();
}
_LIBUNWIND_WEAK_ALIAS(__unw_is_signal_frame, unw_is_signal_frame)
@@ -325,7 +324,7 @@ _LIBUNWIND_WEAK_ALIAS(__unw_is_signal_frame,
unw_is_signal_frame)
_LIBUNWIND_EXPORT uintptr_t __unw_get_data_rel_base(unw_cursor_t *cursor) {
_LIBUNWIND_TRACE_API("unw_get_data_rel_base(cursor=%p)",
static_cast<void *>(cursor));
- AbstractUnwindCursor *co = reinterpret_cast<AbstractUnwindCursor *>(cursor);
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
return co->getDataRelBase();
}
_LIBUNWIND_WEAK_ALIAS(__unw_get_data_rel_base, unw_get_data_rel_base)
@@ -336,7 +335,7 @@ _LIBUNWIND_WEAK_ALIAS(__unw_get_data_rel_base,
unw_get_data_rel_base)
_LIBUNWIND_HIDDEN void __unw_save_vfp_as_X(unw_cursor_t *cursor) {
_LIBUNWIND_TRACE_API("__unw_get_fpreg_save_vfp_as_X(cursor=%p)",
static_cast<void *>(cursor));
- AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
+ AbstractUnwindCursor *co = dynamic_cast<AbstractUnwindCursor
*>(reinterpret_cast<AbstractUnwindCursor *>(cursor->u.data));
return co->saveVFPAsX();
}
_LIBUNWIND_WEAK_ALIAS(__unw_save_vfp_as_X, unw_save_vfp_as_X)
``````````
</details>
https://github.com/llvm/llvm-project/pull/189209
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
