Skip to content

Commit e826f32

Browse files
authored
[ASan/sanitizers] Make stack unwinding better on Windows. (llvm#180205)
I created an issue about this in llvm#179976. Clang's Address Sanitizer installs its own SEH filter which handles some types of uncaught exceptions. Along with register values and some other information, it also generates a stack trace. However, current logic is incomplete. It relies on DbgHelp's SymFunctionTableAccess64 and SymGetModuleBase64 which won't work with machine code that has its RUNTIME_FUNCTION entry registered with Rtl* (e.g. RtlAddFunctionTable) system calls. Most likely, this is because DbgHelp either relies on information in PDB files or considers PDATA and XDATA only from loaded EXE and DLL modules. Either way, consider the following example: ``` #include <windows.h> #include <iostream> #include <vector> typedef union _UNWIND_CODE { struct { BYTE CodeOffset; BYTE UnwindOp : 4; BYTE OpInfo : 4; }; USHORT FrameOffset; } UNWIND_CODE, * PUNWIND_CODE; typedef struct _UNWIND_INFO { BYTE Version : 3; BYTE Flags : 5; BYTE SizeOfProlog; BYTE CountOfCodes; BYTE FrameRegister : 4; BYTE FrameOffset : 4; UNWIND_CODE UnwindCode[1]; // Variable size } UNWIND_INFO, * PUNWIND_INFO; #define UWOP_PUSH_NONVOL 0 #define UWOP_ALLOC_LARGE 1 #define UWOP_ALLOC_SMALL 2 #define UWOP_SET_FPREG 3 #define UWOP_SAVE_NONVOL 4 #define UWOP_SAVE_NONVOL_FAR 5 #define UWOP_SAVE_XMM128 8 #define UWOP_SAVE_XMM128_FAR 9 #define UWOP_PUSH_MACHFRAME 10 int main() { // PUSH RBX (0x53) - Save non-volatile register // SUB RSP, 0x20 (0x48 0x83 0xEC 0x20) - Allocate 32 bytes (shadow space) // XOR RAX, RAX (0x48 0x31 0xC0) - Zero out RAX // MOV RAX, [RAX] (0x48 0x8B 0x00) - Dereference NULL std::vector<unsigned char> code = { 0x53, 0x48, 0x83, 0xEC, 0x20, 0x48, 0x31, 0xC0, 0x48, 0x8B, 0x00 }; size_t codeSize = code.size(); size_t totalSize = 100; LPVOID pMemory = VirtualAlloc(NULL, totalSize, MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE); BYTE* pCodeBase = (BYTE*)pMemory; PUNWIND_INFO pUnwindInfo = (PUNWIND_INFO)(pCodeBase + codeSize); size_t alignmentPadding = 0; if ((size_t)pUnwindInfo % 4 != 0) { alignmentPadding = 4 - ((size_t)pUnwindInfo % 4); pUnwindInfo = (PUNWIND_INFO)((BYTE*)pUnwindInfo + alignmentPadding); } memcpy(pCodeBase, code.data(), codeSize); pUnwindInfo->Version = 1; pUnwindInfo->Flags = UNW_FLAG_NHANDLER; pUnwindInfo->Flags = 0; pUnwindInfo->SizeOfProlog = 5; pUnwindInfo->CountOfCodes = 2; pUnwindInfo->FrameRegister = 0; pUnwindInfo->FrameOffset = 0; pUnwindInfo->UnwindCode[0].CodeOffset = 5; pUnwindInfo->UnwindCode[0].UnwindOp = UWOP_ALLOC_SMALL; pUnwindInfo->UnwindCode[0].OpInfo = 3; pUnwindInfo->UnwindCode[1].CodeOffset = 1; pUnwindInfo->UnwindCode[1].UnwindOp = UWOP_PUSH_NONVOL; pUnwindInfo->UnwindCode[1].OpInfo = 3; // RBX RUNTIME_FUNCTION tableEntry = {}; tableEntry.BeginAddress = 0; tableEntry.EndAddress = (DWORD)codeSize; tableEntry.UnwindData = (DWORD)((BYTE*)pUnwindInfo - (BYTE*)pMemory); DWORD64 baseAddress = (DWORD64)pMemory; RtlAddFunctionTable(&tableEntry, 1, baseAddress); typedef void(*FuncType)(); FuncType myFunc = (FuncType)pMemory; myFunc(); return 0; } ``` Windows' kernel can propagate hardware exception through that function, so clearly these entries are at least partially correct. Right now, ASan's stack walking produces this (compiled with latest release, clang++): ``` PS D:\Local Projects\cpp-playground> ./a.exe ================================================================= ==14216==ERROR: AddressSanitizer: access-violation on unknown address 0x000000000000 (pc 0x0199561c0008 bp 0x004cf0cffb30 sp 0x004cf0cff970 T0) ==14216==The signal is caused by a READ memory access. ==14216==Hint: address points to the zero page. #0 0x0199561c0007 (<unknown module>) #1 0x000000000000 (<unknown module>) #2 0x000000000000 (<unknown module>) ==14216==Register values: rax = 0 rbx = 4cf0cffaa0 rcx = 7ffcb97b4e28 rdx = 19955dc0000 rdi = 11bf564a0040 rsi = 0 rbp = 4cf0cffb30 rsp = 4cf0cff970 r8 = 7ffffffffffffffc r9 = 1 r10 = 0 r11 = 246 r12 = 0 r13 = 0 r14 = 0 r15 = 0 AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: access-violation (<unknown module>) ==14216==ABORTING ``` Frames one and two is just some stack space allocated by that dynamic function. While patched version produces this: ``` PS D:\Local Projects\cpp-playground> ./a.exe ================================================================= ==13660==ERROR: AddressSanitizer: access-violation on unknown address 0x000000000000 (pc 0x01ed5ad70008 bp 0x00d76492f650 sp 0x00d76492f490 T0) ==13660==The signal is caused by a READ memory access. ==13660==Hint: address points to the zero page. #0 0x01ed5ad70007 (<unknown module>) #1 0x7ff732e518a1 in main (D:\Local Projects\cpp-playground\a.exe+0x1400018a1) #2 0x7ff732e56a9b in invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78 llvm#3 0x7ff732e56a9b in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288 llvm#4 0x7ffcb878e8d6 (C:\WINDOWS\System32\KERNEL32.DLL+0x18002e8d6) llvm#5 0x7ffcb966c53b (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18008c53b) ==13660==Register values: rax = 0 rbx = d76492f5c0 rcx = 7ffcb97b4e28 rdx = 1ed5a870000 rdi = 12135afa0040 rsi = 0 rbp = d76492f650 rsp = d76492f490 r8 = 7ffffffffffffffc r9 = 1 r10 = 0 r11 = 246 r12 = 0 r13 = 0 r14 = 0 r15 = 0 AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: access-violation (<unknown module>) ==13660==ABORTING ``` Now we see that stack walking handled our dynamic function properly. Interestingly enough, it appears that other overloaded version of UnwindSlow procedure that works without CONTEXT structure already has some logic to handle this. Theoretically, symbolizer should also be able to provide some information about these functions, but I don't think that this is necessary. I added SANITIZER_WINDOWS64 check because I am pretty sure Microsoft only mentions these functions for 64 bit version of their OS. I also can't check how this works on ARM.
1 parent 7ad2a63 commit e826f32

File tree

1 file changed

+43
-6
lines changed

1 file changed

+43
-6
lines changed

compiler-rt/lib/sanitizer_common/sanitizer_unwind_win.cpp

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,47 @@ void BufferedStackTrace::UnwindSlow(uptr pc, u32 max_depth) {
4343
trace_buffer[0] = pc;
4444
}
4545

46-
#ifdef __clang__
47-
#pragma clang diagnostic push
48-
#pragma clang diagnostic ignored "-Wframe-larger-than="
49-
#endif
46+
PVOID CALLBACK FallbackFunctionTableAccess(HANDLE hProcess,
47+
DWORD64 dwAddrBase) {
48+
// First try DbgHelp's function.
49+
if (PVOID pResult =
50+
__sanitizer::SymFunctionTableAccess64(hProcess, dwAddrBase)) {
51+
return pResult;
52+
}
53+
54+
// Fall back to RtlLookupFunctionEntry for dynamic code.
55+
// Function registered with RtlAddFunctionTable is not necessarily registered
56+
// with DbgHelp, so this is required to cover some edge cases (e.g. JIT
57+
// compilers can use Rtl* functions).
58+
# if SANITIZER_WINDOWS64
59+
DWORD64 dw64ImageBase = 0;
60+
return RtlLookupFunctionEntry(dwAddrBase, &dw64ImageBase, nullptr);
61+
# else
62+
return nullptr;
63+
# endif
64+
}
65+
66+
DWORD64 CALLBACK FallbackGetModuleBase(HANDLE hProcess, DWORD64 dwAddr) {
67+
if (DWORD64 dwResult = __sanitizer::SymGetModuleBase64(hProcess, dwAddr)) {
68+
return dwResult;
69+
}
70+
71+
// Both GetModuleBase and FunctionTableAccess must provide this fallback,
72+
// otherwise dynamic functions won't be properly unwound.
73+
# if SANITIZER_WINDOWS64
74+
DWORD64 dw64ImageBase = 0;
75+
if (RtlLookupFunctionEntry(dwAddr, &dw64ImageBase, nullptr)) {
76+
return dw64ImageBase;
77+
}
78+
# endif
79+
80+
return 0;
81+
}
82+
83+
# ifdef __clang__
84+
# pragma clang diagnostic push
85+
# pragma clang diagnostic ignored "-Wframe-larger-than="
86+
# endif
5087
void BufferedStackTrace::UnwindSlow(uptr pc, void *context, u32 max_depth) {
5188
CHECK(context);
5289
CHECK_GE(max_depth, 2);
@@ -91,8 +128,8 @@ void BufferedStackTrace::UnwindSlow(uptr pc, void *context, u32 max_depth) {
91128
stack_frame.AddrFrame.Mode = AddrModeFlat;
92129
stack_frame.AddrStack.Mode = AddrModeFlat;
93130
while (StackWalk64(machine_type, GetCurrentProcess(), GetCurrentThread(),
94-
&stack_frame, &ctx, NULL, SymFunctionTableAccess64,
95-
SymGetModuleBase64, NULL) &&
131+
&stack_frame, &ctx, NULL, FallbackFunctionTableAccess,
132+
FallbackGetModuleBase, NULL) &&
96133
size < Min(max_depth, kStackTraceMax)) {
97134
trace_buffer[size++] = (uptr)stack_frame.AddrPC.Offset;
98135
}

0 commit comments

Comments
 (0)