-
Notifications
You must be signed in to change notification settings - Fork 15k
[UBSAN] add null and alignment checks for aggregates #164548
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
856ac3b
a42a794
4b7ff1e
85ee368
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2249,6 +2249,24 @@ void CodeGenFunction::EmitAggregateCopy(LValue Dest, LValue Src, QualType Ty, | |
| bool isVolatile) { | ||
| assert(!Ty->isAnyComplexType() && "Shouldn't happen for complex"); | ||
|
|
||
| // Sanitizer checks to verify source and destination pointers are | ||
| // non-null and properly aligned before copying. | ||
| // Without these checks, undefined behavior from invalid pointers goes undetected. | ||
| if (SanOpts.hasOneOf(SanitizerKind::Null | SanitizerKind::Alignment)) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recommend omitting this check and leaving |
||
| Address SrcAddr = Src.getAddress(); | ||
| Address DestAddr = Dest.getAddress(); | ||
|
|
||
| // Check source pointer for null and alignment violations | ||
| EmitTypeCheck(TCK_Load, SourceLocation(), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the scope should be expanded to other cases covered by |
||
| SrcAddr.emitRawPointer(*this), Ty, SrcAddr.getAlignment(), | ||
| SanitizerSet()); | ||
|
|
||
| // Check destination pointer for null and alignment violations | ||
| EmitTypeCheck(TCK_Store, SourceLocation(), | ||
| DestAddr.emitRawPointer(*this), Ty, DestAddr.getAlignment(), | ||
| SanitizerSet()); | ||
| } | ||
|
|
||
| Address DestPtr = Dest.getAddress(); | ||
| Address SrcPtr = Src.getAddress(); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| // RUN: %clangxx -fsanitize=alignment,null -O0 %s -o %t && %run %t | ||
| // RUN: %clangxx -fsanitize=alignment,null -O0 -DTEST_NULL_SRC %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-NULL-SRC | ||
| // RUN: %clangxx -fsanitize=alignment,null -O0 -DTEST_NULL_DEST %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-NULL-DEST | ||
| // RUN: %clangxx -fsanitize=alignment,null -O0 -DTEST_MISALIGN_SRC %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-ALIGN-SRC | ||
| // RUN: %clangxx -fsanitize=alignment,null -O0 -DTEST_MISALIGN_DEST %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-ALIGN-DEST | ||
|
|
||
| // Tests for null pointer and alignment checks in aggregate copy operations. | ||
| // This validates the sanitizer checks added to EmitAggregateCopy for both | ||
| // source and destination pointers with null and alignment violations. | ||
|
|
||
| #include <stdlib.h> | ||
| #include <string.h> | ||
|
|
||
| struct alignas(16) AlignedStruct { | ||
| int a; | ||
| int b; | ||
| int c; | ||
| int d; | ||
| }; | ||
|
|
||
| struct NormalStruct { | ||
| int x; | ||
| int y; | ||
| int z; | ||
| }; | ||
|
|
||
| void test_null_src() { | ||
| AlignedStruct dest; | ||
| AlignedStruct *src = nullptr; | ||
| // CHECK-NULL-SRC: runtime error: load of null pointer of type 'AlignedStruct' | ||
| dest = *src; | ||
| } | ||
|
|
||
| void test_null_dest() { | ||
| AlignedStruct src = {1, 2, 3, 4}; | ||
| AlignedStruct *dest = nullptr; | ||
| // CHECK-NULL-DEST: runtime error: store to null pointer of type 'AlignedStruct' | ||
| *dest = src; | ||
| } | ||
|
|
||
| void test_misaligned_src() { | ||
| char buffer[sizeof(AlignedStruct) + 16]; | ||
| // Create a misaligned pointer (not 16-byte aligned) | ||
| AlignedStruct *src = (AlignedStruct *)(buffer + 1); | ||
| AlignedStruct dest; | ||
| // CHECK-ALIGN-SRC: runtime error: load of misaligned address {{0x[0-9a-f]+}} for type 'AlignedStruct', which requires 16 byte alignment | ||
| dest = *src; | ||
| } | ||
|
|
||
| void test_misaligned_dest() { | ||
| AlignedStruct src = {1, 2, 3, 4}; | ||
| char buffer[sizeof(AlignedStruct) + 16]; | ||
| // Create a misaligned pointer (not 16-byte aligned) | ||
| AlignedStruct *dest = (AlignedStruct *)(buffer + 1); | ||
| // CHECK-ALIGN-DEST: runtime error: store to misaligned address {{0x[0-9a-f]+}} for type 'AlignedStruct', which requires 16 byte alignment | ||
| *dest = src; | ||
| } | ||
|
|
||
| void test_normal_copy() { | ||
| // This should work fine - properly aligned, non-null pointers | ||
| AlignedStruct src = {1, 2, 3, 4}; | ||
| AlignedStruct dest; | ||
| dest = src; | ||
| } | ||
|
|
||
| int main() { | ||
| #ifdef TEST_NULL_SRC | ||
| test_null_src(); | ||
| #elif defined(TEST_NULL_DEST) | ||
| test_null_dest(); | ||
| #elif defined(TEST_MISALIGN_SRC) | ||
| test_misaligned_src(); | ||
| #elif defined(TEST_MISALIGN_DEST) | ||
| test_misaligned_dest(); | ||
| #else | ||
| test_normal_copy(); | ||
| #endif | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add comment block here to explain
if block