Skip to content

Commit

Permalink
Fix PR35902: incorrect alignment used for ubsan check.
Browse files Browse the repository at this point in the history
UBSan was using the complete-object align rather than nv alignment
when checking the "this" pointer of a method.

Furthermore, CGF.CXXABIThisAlignment was also being set incorrectly,
due to an incorrectly negated test. The latter doesn't appear to have
had any impact, due to it not really being used anywhere.

Differential Revision: https://reviews.llvm.org/D93072
  • Loading branch information
jyknight committed Dec 28, 2020
1 parent 85af1d6 commit 4ddf140
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 9 deletions.
4 changes: 2 additions & 2 deletions clang/lib/CodeGen/CGCXXABI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ void CGCXXABI::buildThisParam(CodeGenFunction &CGF, FunctionArgList &params) {
// down to whether we know it's a complete object or not.
auto &Layout = CGF.getContext().getASTRecordLayout(MD->getParent());
if (MD->getParent()->getNumVBases() == 0 || // avoid vcall in common case
MD->getParent()->hasAttr<FinalAttr>() ||
!isThisCompleteObject(CGF.CurGD)) {
MD->getParent()->isEffectivelyFinal() ||
isThisCompleteObject(CGF.CurGD)) {
CGF.CXXABIThisAlignment = Layout.getAlignment();
} else {
CGF.CXXABIThisAlignment = Layout.getNonVirtualAlignment();
Expand Down
8 changes: 3 additions & 5 deletions clang/lib/CodeGen/CodeGenFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1137,11 +1137,9 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
MD->getParent()->getLambdaCaptureDefault() == LCD_None)
SkippedChecks.set(SanitizerKind::Null, true);

EmitTypeCheck(isa<CXXConstructorDecl>(MD) ? TCK_ConstructorCall
: TCK_MemberCall,
Loc, CXXABIThisValue, ThisTy,
getContext().getTypeAlignInChars(ThisTy->getPointeeType()),
SkippedChecks);
EmitTypeCheck(
isa<CXXConstructorDecl>(MD) ? TCK_ConstructorCall : TCK_MemberCall,
Loc, CXXABIThisValue, ThisTy, CXXABIThisAlignment, SkippedChecks);
}
}

Expand Down
13 changes: 11 additions & 2 deletions clang/test/CodeGenCXX/catch-undef-behavior.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -430,8 +430,8 @@ namespace VBaseObjectSize {
// Note: C is laid out such that offsetof(C, B) + sizeof(B) extends outside
// the C object.
struct alignas(16) A { void *a1, *a2; };
struct B : virtual A { void *b; };
struct C : virtual A, virtual B {};
struct B : virtual A { void *b; void* g(); };
struct C : virtual A, virtual B { };
// CHECK-LABEL: define {{.*}} @_ZN15VBaseObjectSize1fERNS_1BE(
B &f(B &b) {
// Size check: check for nvsize(B) == 16 (do not require size(B) == 32)
Expand All @@ -443,6 +443,15 @@ namespace VBaseObjectSize {
// CHECK: and i64 [[PTRTOINT]], 7,
return b;
}

// CHECK-LABEL: define {{.*}} @_ZN15VBaseObjectSize1B1gEv(
void *B::g() {
// Ensure that the check on the "this" pointer also uses the proper
// alignment. We should be using nvalign(B) == 8, not 16.
// CHECK: [[PTRTOINT:%.+]] = ptrtoint {{.*}} to i64,
// CHECK: and i64 [[PTRTOINT]], 7
return nullptr;
}
}

namespace FunctionSanitizerVirtualCalls {
Expand Down

0 comments on commit 4ddf140

Please sign in to comment.