Revert "[UBSan] Improve error message when a misalignment is due to t… #166197
+12
−86
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
…arget de…" This reverts commit 47c54d5.
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-compiler-rt-sanitizer Author: Matthew Nagy (gbMattN) Changes…arget de…" This reverts commit 47c54d5. Full diff: https://github.com/llvm/llvm-project/pull/166197.diff 6 Files Affected:
diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index f2dd22e9bed3b..14d8db32bafc6 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -18,9 +18,6 @@
#include "ConstantEmitter.h"
#include "TargetInfo.h"
#include "clang/Basic/CodeGenOptions.h"
-#include "clang/Basic/Sanitizers.h"
-#include "clang/Basic/SourceLocation.h"
-#include "clang/Basic/SourceManager.h"
#include "clang/CodeGen/CGFunctionInfo.h"
#include "llvm/IR/Intrinsics.h"
@@ -1752,17 +1749,6 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) {
allocator->isReservedGlobalPlacementOperator())
result = Builder.CreateLaunderInvariantGroup(result);
- // Check the default alignment of the type and why. Users may incorrectly
- // return misaligned memory from a replaced operator new without knowing
- // about default alignment.
- TypeCheckKind checkKind = CodeGenFunction::TCK_ConstructorCall;
- const TargetInfo &TI = getContext().getTargetInfo();
- unsigned DefaultTargetAlignment = TI.getNewAlign() / TI.getCharWidth();
- if (SanOpts.has(SanitizerKind::Alignment) &&
- (DefaultTargetAlignment >
- CGM.getContext().getTypeAlignInChars(allocType).getQuantity()))
- checkKind = CodeGenFunction::TCK_ConstructorCallMinimumAlign;
-
// Emit sanitizer checks for pointer value now, so that in the case of an
// array it was checked only once and not at each constructor call. We may
// have already checked that the pointer is non-null.
@@ -1770,9 +1756,10 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) {
// we'll null check the wrong pointer here.
SanitizerSet SkippedChecks;
SkippedChecks.set(SanitizerKind::Null, nullCheck);
- EmitTypeCheck(
- checkKind, E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
- result, allocType, result.getAlignment(), SkippedChecks, numElements);
+ EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall,
+ E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
+ result, allocType, result.getAlignment(), SkippedChecks,
+ numElements);
EmitNewInitializer(*this, E, allocType, elementTy, result, numElements,
allocSizeWithoutCookie);
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 047ca844c79de..8c4c1c8c2dc95 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -3296,10 +3296,7 @@ class CodeGenFunction : public CodeGenTypeCache {
TCK_NonnullAssign,
/// Checking the operand of a dynamic_cast or a typeid expression. Must be
/// null or an object within its lifetime.
- TCK_DynamicOperation,
- /// Checking the 'this' poiner for a constructor call, including that the
- /// alignment is greater or equal to the targets minimum alignment
- TCK_ConstructorCallMinimumAlign
+ TCK_DynamicOperation
};
/// Determine whether the pointer type check \p TCK permits null pointers.
diff --git a/compiler-rt/lib/ubsan/ubsan_checks.inc b/compiler-rt/lib/ubsan/ubsan_checks.inc
index f8757d781afb8..b1d09a9024e7e 100644
--- a/compiler-rt/lib/ubsan/ubsan_checks.inc
+++ b/compiler-rt/lib/ubsan/ubsan_checks.inc
@@ -28,7 +28,6 @@ UBSAN_CHECK(NullptrAfterNonZeroOffset, "nullptr-after-nonzero-offset",
UBSAN_CHECK(PointerOverflow, "pointer-overflow", "pointer-overflow")
UBSAN_CHECK(MisalignedPointerUse, "misaligned-pointer-use", "alignment")
UBSAN_CHECK(AlignmentAssumption, "alignment-assumption", "alignment")
-UBSAN_CHECK(MinumumAssumedAlignment, "minimum-assumed-alignment", "alignment")
UBSAN_CHECK(InsufficientObjectSize, "insufficient-object-size", "object-size")
UBSAN_CHECK(SignedIntegerOverflow, "signed-integer-overflow",
"signed-integer-overflow")
diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.cpp b/compiler-rt/lib/ubsan/ubsan_handlers.cpp
index fc6063af4562b..63319f46734a4 100644
--- a/compiler-rt/lib/ubsan/ubsan_handlers.cpp
+++ b/compiler-rt/lib/ubsan/ubsan_handlers.cpp
@@ -73,26 +73,14 @@ enum TypeCheckKind {
TCK_NonnullAssign,
/// Checking the operand of a dynamic_cast or a typeid expression. Must be
/// null or an object within its lifetime.
- TCK_DynamicOperation,
- /// Checking the 'this' poiner for a constructor call, including that the
- /// alignment is greater or equal to the targets minimum alignment
- TCK_ConstructorCallMinimumAlign
+ TCK_DynamicOperation
};
extern const char *const TypeCheckKinds[] = {
- "load of",
- "store to",
- "reference binding to",
- "member access within",
- "member call on",
- "constructor call on",
- "downcast of",
- "downcast of",
- "upcast of",
- "cast to virtual base of",
- "_Nonnull binding to",
- "dynamic operation on",
- "constructor call with pointer from operator new on"};
+ "load of", "store to", "reference binding to", "member access within",
+ "member call on", "constructor call on", "downcast of", "downcast of",
+ "upcast of", "cast to virtual base of", "_Nonnull binding to",
+ "dynamic operation on"};
}
static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer,
@@ -106,9 +94,7 @@ static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer,
? ErrorType::NullPointerUseWithNullability
: ErrorType::NullPointerUse;
else if (Pointer & (Alignment - 1))
- ET = (Data->TypeCheckKind == TCK_ConstructorCallMinimumAlign)
- ? ErrorType::MinumumAssumedAlignment
- : ErrorType::MisalignedPointerUse;
+ ET = ErrorType::MisalignedPointerUse;
else
ET = ErrorType::InsufficientObjectSize;
@@ -131,13 +117,6 @@ static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer,
Diag(Loc, DL_Error, ET, "%0 null pointer of type %1")
<< TypeCheckKinds[Data->TypeCheckKind] << Data->Type;
break;
- case ErrorType::MinumumAssumedAlignment:
- Diag(Loc, DL_Error, ET,
- "%0 misaligned address %1 for type %2, "
- "which requires target minimum assumed %3 byte alignment")
- << TypeCheckKinds[Data->TypeCheckKind] << (void *)Pointer << Data->Type
- << Alignment;
- break;
case ErrorType::MisalignedPointerUse:
Diag(Loc, DL_Error, ET, "%0 misaligned address %1 for type %3, "
"which requires %2 byte alignment")
diff --git a/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp b/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp
deleted file mode 100644
index 4642126ab74c4..0000000000000
--- a/compiler-rt/test/ubsan/TestCases/TypeCheck/minimum-alignment.cpp
+++ /dev/null
@@ -1,36 +0,0 @@
-// RUN: %clangxx %gmlt -fsanitize=alignment %s -o %t
-// RUN: %run %t 2>&1 | FileCheck %s
-
-// UNSUPPORTED: i386
-// UNSUPPORTED: armv7l
-
-// These sanitizers already overload the new operator so won't compile this test
-// UNSUPPORTED: ubsan-msan
-// UNSUPPORTED: ubsan-tsan
-
-#include <cassert>
-#include <cstdlib>
-
-void *operator new(std::size_t count) {
- constexpr const size_t offset = 8;
-
- // allocate a bit more so we can safely offset it
- void *ptr = std::malloc(count + offset);
-
- // verify malloc returned 16 bytes aligned mem
- static_assert(__STDCPP_DEFAULT_NEW_ALIGNMENT__ == 16);
- assert(((std::ptrdiff_t)ptr & (__STDCPP_DEFAULT_NEW_ALIGNMENT__ - 1)) == 0);
-
- return (char *)ptr + offset;
-}
-
-struct Foo {
- void *_cookie1, *_cookie2;
-};
-
-static_assert(alignof(Foo) == 8);
-int main() {
- // CHECK: runtime error: constructor call with pointer from operator new on misaligned address 0x{{.*}} for type 'Foo', which requires target minimum assumed 16 byte alignment
- Foo *f = new Foo;
- return 0;
-}
diff --git a/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp b/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp
index 4b0b2b5923c6f..e39a0ab4e6589 100644
--- a/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp
+++ b/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp
@@ -101,7 +101,7 @@ int main(int, char **argv) {
return s->f() && 0;
case 'n':
- // CHECK-NEW: misaligned.cpp:[[@LINE+4]]{{(:21)?}}: runtime error: constructor call with pointer from operator new on misaligned address [[PTR:0x[0-9a-f]*]] for type 'S', which requires target minimum assumed 4 byte alignment
+ // CHECK-NEW: misaligned.cpp:[[@LINE+4]]{{(:21)?}}: runtime error: constructor call on misaligned address [[PTR:0x[0-9a-f]*]] for type 'S', which requires 4 byte alignment
// CHECK-NEW-NEXT: [[PTR]]: note: pointer points here
// CHECK-NEW-NEXT: {{^ 00 00 00 01 02 03 04 05}}
// CHECK-NEW-NEXT: {{^ \^}}
|
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions h,inc,cpp -- clang/lib/CodeGen/CGExprCXX.cpp clang/lib/CodeGen/CodeGenFunction.h compiler-rt/lib/ubsan/ubsan_checks.inc compiler-rt/lib/ubsan/ubsan_handlers.cpp compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp --diff_from_common_commit
View the diff from clang-format here.diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.cpp b/compiler-rt/lib/ubsan/ubsan_handlers.cpp
index 63319f467..f688f6e46 100644
--- a/compiler-rt/lib/ubsan/ubsan_handlers.cpp
+++ b/compiler-rt/lib/ubsan/ubsan_handlers.cpp
@@ -76,11 +76,18 @@ enum TypeCheckKind {
TCK_DynamicOperation
};
-extern const char *const TypeCheckKinds[] = {
- "load of", "store to", "reference binding to", "member access within",
- "member call on", "constructor call on", "downcast of", "downcast of",
- "upcast of", "cast to virtual base of", "_Nonnull binding to",
- "dynamic operation on"};
+extern const char *const TypeCheckKinds[] = {"load of",
+ "store to",
+ "reference binding to",
+ "member access within",
+ "member call on",
+ "constructor call on",
+ "downcast of",
+ "downcast of",
+ "upcast of",
+ "cast to virtual base of",
+ "_Nonnull binding to",
+ "dynamic operation on"};
}
static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer,
|
gbMattN
deleted the
revert-152532-users/nagym/ubsan-incorrect-alignment
branch
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.
…arget de…"
This reverts commit 47c54d5.