[lldb][debugserver][MacOSX] Work around sanitizer misaligned address errors when reading exception data #132193
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Michael137
changed the title
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesWe've been dealing with UBSAN issues around this code for some time now (see Work around these failures by pretending the input data is a Drive-by changes:
Full diff: https://github.com/llvm/llvm-project/pull/132193.diff 2 Files Affected:
diff --git a/lldb/tools/debugserver/source/MacOSX/MachException.cpp b/lldb/tools/debugserver/source/MacOSX/MachException.cpp
index 659fb2ff8186d..d05b47cd9b590 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachException.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/MachException.cpp
@@ -20,6 +20,23 @@
#include <cerrno>
#include <sys/ptrace.h>
#include <sys/types.h>
+#include <inttypes.h>
+
+static void AppendExceptionData(
+ std::vector<mach_exception_data_type_t> &out,
+ mach_exception_data_t Data, mach_msg_type_number_t Count) {
+ mach_exception_data_type_t Buf;
+ for (mach_msg_type_number_t i = 0; i < Count; ++i) {
+ // The input Data we receive need not be aligned correctly.
+ // Perform an unaligned copy by pretending we're dealing with
+ // a char* buffer. This is required to work around UBSAN/ASAN
+ // "misaligned address" errors.
+ auto * src = reinterpret_cast<char*>(&Buf);
+ auto * dst = reinterpret_cast<char*>(Data + i);
+ memcpy(dst, src, sizeof(mach_exception_data_type_t));
+ out.push_back(Buf);
+ }
+}
// Routine mach_exception_raise
extern "C" kern_return_t
@@ -95,20 +112,15 @@ catch_mach_exception_raise(mach_port_t exc_port, mach_port_t thread_port,
mach_exception_data_t exc_data,
mach_msg_type_number_t exc_data_count) {
if (DNBLogCheckLogBit(LOG_EXCEPTIONS)) {
- std::vector<uint64_t> exc_datas;
- uint64_t tmp;
- for (unsigned i = 0; i < exc_data_count; ++i) {
- // Perform an unaligned copy.
- memcpy(&tmp, &exc_data[i], sizeof(uint64_t));
- exc_datas.push_back(tmp);
- }
+ std::vector<mach_exception_data_type_t> exc_datas;
+ AppendExceptionData(exc_datas, exc_data, exc_data_count);
DNBLogThreaded("::%s ( exc_port = 0x%4.4x, thd_port = 0x%4.4x, tsk_port = "
- "0x%4.4x, exc_type = %d ( %s ), exc_data[%d] = { 0x%llx, "
- "0x%llx })",
+ "0x%4.4x, exc_type = %d ( %s ), exc_data[%d] = { 0x%" PRIx64 ", "
+ "0x%" PRIx64 " })",
__FUNCTION__, exc_port, thread_port, task_port, exc_type,
MachException::Name(exc_type), exc_data_count,
- (uint64_t)(exc_data_count > 0 ? exc_datas[0] : 0xBADDBADD),
- (uint64_t)(exc_data_count > 1 ? exc_datas[1] : 0xBADDBADD));
+ (exc_data_count > 0 ? exc_datas[0] : 0xBADDBADD),
+ (exc_data_count > 1 ? exc_datas[1] : 0xBADDBADD));
}
g_message->exc_type = 0;
g_message->exc_data.clear();
@@ -117,7 +129,7 @@ catch_mach_exception_raise(mach_port_t exc_port, mach_port_t thread_port,
g_message->task_port = task_port;
g_message->thread_port = thread_port;
g_message->exc_type = exc_type;
- g_message->AppendExceptionData(exc_data, exc_data_count);
+ AppendExceptionData(g_message->exc_data, exc_data, exc_data_count);
return KERN_SUCCESS;
} else if (!MachTask::IsValid(g_message->task_port)) {
// 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,
g_message->task_port = task_port;
g_message->thread_port = thread_port;
g_message->exc_type = exc_type;
- g_message->AppendExceptionData(exc_data, exc_data_count);
+ AppendExceptionData(g_message->exc_data, exc_data, exc_data_count);
return KERN_SUCCESS;
}
}
diff --git a/lldb/tools/debugserver/source/MacOSX/MachException.h b/lldb/tools/debugserver/source/MacOSX/MachException.h
index bf9771ef0bf93..573e84e991608 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachException.h
+++ b/lldb/tools/debugserver/source/MacOSX/MachException.h
@@ -70,15 +70,6 @@ class MachException {
return (exc_type == EXC_BREAKPOINT ||
((exc_type == EXC_SOFTWARE) && exc_data[0] == 1));
}
- void AppendExceptionData(mach_exception_data_t Data,
- mach_msg_type_number_t Count) {
- mach_exception_data_type_t Buf;
- for (mach_msg_type_number_t i = 0; i < Count; ++i) {
- // Perform an unaligned copy.
- memcpy(&Buf, Data + i, sizeof(mach_exception_data_type_t));
- exc_data.push_back(Buf);
- }
- }
void Dump() const;
void DumpStopReason() const;
bool GetStopInfo(struct DNBThreadStopInfo *stop_info) const;
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
…errors when reading exception data
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
Michael137
force-pushed
the
lldb/debugserver-ubsan
branch
from
03eb418 to
64d8c3e
Compare
Co-authored-by: Jonas Devlieghere <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We've been dealing with UBSAN issues around this code for some time now (see
9c36859b33b386fbfa9599646de1e2ae01158180and1a2122e9e9d1d495fdf337a4a9445b61ca56df6f). On recent macOS versions, a UBSAN-enabled debugserver will crash when performing amemcpyof the inputmach_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:Work around these failures by pretending the input data is a
char*buffer.Drive-by changes:
AppendExceptionDataand made the types consistent