Skip to content

Commit 52de49e

Browse files
[lldb][debugserver][MacOSX] Work around sanitizer misaligned address errors when reading exception data (#132193)
We've been dealing with UBSAN issues around this code for some time now (see `9c36859b33b386fbfa9599646de1e2ae01158180` and `1a2122e9e9d1d495fdf337a4a9445b61ca56df6f`). On recent macOS versions, a UBSAN-enabled debugserver will crash when performing a `memcpy` of the input `mach_exception_data_t`. The pointer to the beginning of the exception data may not be aligned on a doubleword boundary, leading to UBSAN failures such as: ``` $ ./bin/debugserver 0.0.0.0:5555 /Volumes/SSD/llvm-builds/llvm-worktrees/clang-work/build-sanitized-release/tools/lldb/test/Shell/Recognizer/Output/verbose_trap.test.tmp.out /Volumes/SSD/llvm-builds/llvm-worktrees/clang-work/lldb/tools/debugserver/source/MacOSX/MachException.cpp:35:12: runtime error: store to misaligned address 0x00016ddfa634 for type 'mach_exception_data_type_t *' (aka 'long long *'), which requires 8 byte alignment 0x00016ddfa634: note: pointer points here 02 00 00 00 03 00 01 00 00 00 00 00 11 00 00 00 00 00 00 00 00 00 00 00 08 00 00 00 00 00 00 00 ^ SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Volumes/SSD/llvm-builds/llvm-worktrees/clang-work/lldb/tools/debugserver/source/MacOSX/MachException.cpp:35:12 ``` Work around these failures by pretending the input data is a `char*` buffer. Drive-by changes: * I factored out some duplicated code into a static `AppendExceptionData` and made the types consistent --------- Co-authored-by: Jonas Devlieghere <[email protected]>
1 parent 4b41984 commit 52de49e

File tree

2 files changed

+25
-22
lines changed

2 files changed

+25
-22
lines changed

lldb/tools/debugserver/source/MacOSX/MachException.cpp

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,25 @@
1818
#include "PThreadMutex.h"
1919
#include "SysSignal.h"
2020
#include <cerrno>
21+
#include <inttypes.h>
2122
#include <sys/ptrace.h>
2223
#include <sys/types.h>
2324

25+
static void AppendExceptionData(std::vector<mach_exception_data_type_t> &out,
26+
mach_exception_data_t data,
27+
mach_msg_type_number_t count) {
28+
mach_exception_data_type_t buf;
29+
for (mach_msg_type_number_t i = 0; i < count; ++i) {
30+
// The input Data we receive need not be aligned correctly.
31+
// Perform an unaligned copy by pretending we're dealing with
32+
// a char* buffer. This is required to work around UBSAN/ASAN
33+
// "misaligned address" errors.
34+
auto *src = reinterpret_cast<char *>(data + i);
35+
memcpy(&buf, src, sizeof(mach_exception_data_type_t));
36+
out.push_back(buf);
37+
}
38+
}
39+
2440
// Routine mach_exception_raise
2541
extern "C" kern_return_t
2642
catch_mach_exception_raise(mach_port_t exception_port, mach_port_t thread,
@@ -95,20 +111,16 @@ catch_mach_exception_raise(mach_port_t exc_port, mach_port_t thread_port,
95111
mach_exception_data_t exc_data,
96112
mach_msg_type_number_t exc_data_count) {
97113
if (DNBLogCheckLogBit(LOG_EXCEPTIONS)) {
98-
std::vector<uint64_t> exc_datas;
99-
uint64_t tmp;
100-
for (unsigned i = 0; i < exc_data_count; ++i) {
101-
// Perform an unaligned copy.
102-
memcpy(&tmp, &exc_data[i], sizeof(uint64_t));
103-
exc_datas.push_back(tmp);
104-
}
114+
std::vector<mach_exception_data_type_t> exc_datas;
115+
AppendExceptionData(exc_datas, exc_data, exc_data_count);
105116
DNBLogThreaded("::%s ( exc_port = 0x%4.4x, thd_port = 0x%4.4x, tsk_port = "
106-
"0x%4.4x, exc_type = %d ( %s ), exc_data[%d] = { 0x%llx, "
107-
"0x%llx })",
117+
"0x%4.4x, exc_type = %d ( %s ), exc_data[%d] = { 0x%" PRIx64
118+
", "
119+
"0x%" PRIx64 " })",
108120
__FUNCTION__, exc_port, thread_port, task_port, exc_type,
109121
MachException::Name(exc_type), exc_data_count,
110-
(uint64_t)(exc_data_count > 0 ? exc_datas[0] : 0xBADDBADD),
111-
(uint64_t)(exc_data_count > 1 ? exc_datas[1] : 0xBADDBADD));
122+
(exc_data_count > 0 ? exc_datas[0] : 0xBADDBADD),
123+
(exc_data_count > 1 ? exc_datas[1] : 0xBADDBADD));
112124
}
113125
g_message->exc_type = 0;
114126
g_message->exc_data.clear();
@@ -117,7 +129,7 @@ catch_mach_exception_raise(mach_port_t exc_port, mach_port_t thread_port,
117129
g_message->task_port = task_port;
118130
g_message->thread_port = thread_port;
119131
g_message->exc_type = exc_type;
120-
g_message->AppendExceptionData(exc_data, exc_data_count);
132+
AppendExceptionData(g_message->exc_data, exc_data, exc_data_count);
121133
return KERN_SUCCESS;
122134
} else if (!MachTask::IsValid(g_message->task_port)) {
123135
// Our original exception port isn't valid anymore check for a SIGTRAP
@@ -129,7 +141,7 @@ catch_mach_exception_raise(mach_port_t exc_port, mach_port_t thread_port,
129141
g_message->task_port = task_port;
130142
g_message->thread_port = thread_port;
131143
g_message->exc_type = exc_type;
132-
g_message->AppendExceptionData(exc_data, exc_data_count);
144+
AppendExceptionData(g_message->exc_data, exc_data, exc_data_count);
133145
return KERN_SUCCESS;
134146
}
135147
}

lldb/tools/debugserver/source/MacOSX/MachException.h

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,6 @@ class MachException {
7070
return (exc_type == EXC_BREAKPOINT ||
7171
((exc_type == EXC_SOFTWARE) && exc_data[0] == 1));
7272
}
73-
void AppendExceptionData(mach_exception_data_t Data,
74-
mach_msg_type_number_t Count) {
75-
mach_exception_data_type_t Buf;
76-
for (mach_msg_type_number_t i = 0; i < Count; ++i) {
77-
// Perform an unaligned copy.
78-
memcpy(&Buf, Data + i, sizeof(mach_exception_data_type_t));
79-
exc_data.push_back(Buf);
80-
}
81-
}
8273
void Dump() const;
8374
void DumpStopReason() const;
8475
bool GetStopInfo(struct DNBThreadStopInfo *stop_info) const;

0 commit comments

Comments
 (0)