[clang] 0130b6c - Don't assume a reference refers to at least sizeof(T) bytes.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 31 19:08:53 PST 2020
Author: Richard Smith
Date: 2020-01-31T19:08:17-08:00
New Revision: 0130b6cb5a8d94511e2bb09ac2f5a613a59f70b4
URL: https://github.com/llvm/llvm-project/commit/0130b6cb5a8d94511e2bb09ac2f5a613a59f70b4
DIFF: https://github.com/llvm/llvm-project/commit/0130b6cb5a8d94511e2bb09ac2f5a613a59f70b4.diff
LOG: Don't assume a reference refers to at least sizeof(T) bytes.
When T is a class type, only nvsize(T) bytes need be accessible through
the reference. We had matching bugs in the application of the
dereferenceable attribute and in -fsanitize=undefined.
Added:
Modified:
clang/include/clang/AST/DeclCXX.h
clang/lib/AST/DeclCXX.cpp
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGClass.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/test/CodeGenCXX/catch-undef-behavior.cpp
clang/test/CodeGenCXX/thunks.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 2e8e31dbf4c7..6d3a833b5037 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1696,6 +1696,10 @@ class CXXRecordDecl : public RecordDecl {
/// actually abstract.
bool mayBeAbstract() const;
+ /// Determine whether it's impossible for a class to be derived from this
+ /// class. This is best-effort, and may conservatively return false.
+ bool isEffectivelyFinal() const;
+
/// If this is the closure type of a lambda expression, retrieve the
/// number to be used for name mangling in the Itanium C++ ABI.
///
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 227fe80ccab4..931a1141b1b4 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -1923,6 +1923,18 @@ bool CXXRecordDecl::mayBeAbstract() const {
return false;
}
+bool CXXRecordDecl::isEffectivelyFinal() const {
+ auto *Def = getDefinition();
+ if (!Def)
+ return false;
+ if (Def->hasAttr<FinalAttr>())
+ return true;
+ if (const auto *Dtor = Def->getDestructor())
+ if (Dtor->hasAttr<FinalAttr>())
+ return true;
+ return false;
+}
+
void CXXDeductionGuideDecl::anchor() {}
bool ExplicitSpecifier::isEquivalent(const ExplicitSpecifier Other) const {
@@ -2142,12 +2154,8 @@ CXXMethodDecl *CXXMethodDecl::getDevirtualizedMethod(const Expr *Base,
// Similarly, if the class itself or its destructor is marked 'final',
// the class can't be derived from and we can therefore devirtualize the
// member function call.
- if (BestDynamicDecl->hasAttr<FinalAttr>())
+ if (BestDynamicDecl->isEffectivelyFinal())
return DevirtualizedMethod;
- if (const auto *dtor = BestDynamicDecl->getDestructor()) {
- if (dtor->hasAttr<FinalAttr>())
- return DevirtualizedMethod;
- }
if (const auto *DRE = dyn_cast<DeclRefExpr>(Base)) {
if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl()))
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 3f132a0a62aa..9ed2ccd54487 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -2054,8 +2054,8 @@ void CodeGenModule::ConstructAttributeList(
if (const auto *RefTy = RetTy->getAs<ReferenceType>()) {
QualType PTy = RefTy->getPointeeType();
if (!PTy->isIncompleteType() && PTy->isConstantSizeType())
- RetAttrs.addDereferenceableAttr(getContext().getTypeSizeInChars(PTy)
- .getQuantity());
+ RetAttrs.addDereferenceableAttr(
+ getMinimumObjectSize(PTy).getQuantity());
else if (getContext().getTargetAddressSpace(PTy) == 0 &&
!CodeGenOpts.NullPointerIsValid)
RetAttrs.addAttribute(llvm::Attribute::NonNull);
@@ -2164,8 +2164,8 @@ void CodeGenModule::ConstructAttributeList(
if (const auto *RefTy = ParamType->getAs<ReferenceType>()) {
QualType PTy = RefTy->getPointeeType();
if (!PTy->isIncompleteType() && PTy->isConstantSizeType())
- Attrs.addDereferenceableAttr(getContext().getTypeSizeInChars(PTy)
- .getQuantity());
+ Attrs.addDereferenceableAttr(
+ getMinimumObjectSize(PTy).getQuantity());
else if (getContext().getTargetAddressSpace(PTy) == 0 &&
!CodeGenOpts.NullPointerIsValid)
Attrs.addAttribute(llvm::Attribute::NonNull);
diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 7389207bc8ad..acc9a9ec4f4a 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -35,20 +35,37 @@ using namespace CodeGen;
/// Return the best known alignment for an unknown pointer to a
/// particular class.
CharUnits CodeGenModule::getClassPointerAlignment(const CXXRecordDecl *RD) {
- if (!RD->isCompleteDefinition())
+ if (!RD->hasDefinition())
return CharUnits::One(); // Hopefully won't be used anywhere.
auto &layout = getContext().getASTRecordLayout(RD);
// If the class is final, then we know that the pointer points to an
// object of that type and can use the full alignment.
- if (RD->hasAttr<FinalAttr>()) {
+ if (RD->isEffectivelyFinal())
return layout.getAlignment();
// Otherwise, we have to assume it could be a subclass.
- } else {
- return layout.getNonVirtualAlignment();
- }
+ return layout.getNonVirtualAlignment();
+}
+
+/// Return the smallest possible amount of storage that might be allocated
+/// starting from the beginning of an object of a particular class.
+///
+/// This may be smaller than sizeof(RD) if RD has virtual base classes.
+CharUnits CodeGenModule::getMinimumClassObjectSize(const CXXRecordDecl *RD) {
+ if (!RD->hasDefinition())
+ return CharUnits::One();
+
+ auto &layout = getContext().getASTRecordLayout(RD);
+
+ // If the class is final, then we know that the pointer points to an
+ // object of that type and can use the full alignment.
+ if (RD->isEffectivelyFinal())
+ return layout.getSize();
+
+ // Otherwise, we have to assume it could be a subclass.
+ return std::max(layout.getNonVirtualSize(), CharUnits::One());
}
/// Return the best known alignment for a pointer to a virtual base,
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index f1a5e3dcb33c..4b8a31c180c8 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -711,7 +711,7 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
if (SanOpts.has(SanitizerKind::ObjectSize) &&
!SkippedChecks.has(SanitizerKind::ObjectSize) &&
!Ty->isIncompleteType()) {
- uint64_t TySize = getContext().getTypeSizeInChars(Ty).getQuantity();
+ uint64_t TySize = CGM.getMinimumObjectSize(Ty).getQuantity();
llvm::Value *Size = llvm::ConstantInt::get(IntPtrTy, TySize);
if (ArraySize)
Size = Builder.CreateMul(Size, ArraySize);
@@ -742,7 +742,9 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
!SkippedChecks.has(SanitizerKind::Alignment)) {
AlignVal = Alignment.getQuantity();
if (!Ty->isIncompleteType() && !AlignVal)
- AlignVal = getContext().getTypeAlignInChars(Ty).getQuantity();
+ AlignVal =
+ getNaturalTypeAlignment(Ty, nullptr, nullptr, /*ForPointeeType=*/true)
+ .getQuantity();
// The glvalue must be suitably aligned.
if (AlignVal > 1 &&
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index a711d5ccba0e..e8162fcfc0cd 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -868,6 +868,17 @@ class CodeGenModule : public CodeGenTypeCache {
/// Returns the assumed alignment of an opaque pointer to the given class.
CharUnits getClassPointerAlignment(const CXXRecordDecl *CD);
+ /// Returns the minimum object size for an object of the given class type
+ /// (or a class derived from it).
+ CharUnits getMinimumClassObjectSize(const CXXRecordDecl *CD);
+
+ /// Returns the minimum object size for an object of the given type.
+ CharUnits getMinimumObjectSize(QualType Ty) {
+ if (CXXRecordDecl *RD = Ty->getAsCXXRecordDecl())
+ return getMinimumClassObjectSize(RD);
+ return getContext().getTypeSizeInChars(Ty);
+ }
+
/// Returns the assumed alignment of a virtual base of a class.
CharUnits getVBaseAlignment(CharUnits DerivedAlign,
const CXXRecordDecl *Derived,
diff --git a/clang/test/CodeGenCXX/catch-undef-behavior.cpp b/clang/test/CodeGenCXX/catch-undef-behavior.cpp
index ba72af038aba..c5aec53ad724 100644
--- a/clang/test/CodeGenCXX/catch-undef-behavior.cpp
+++ b/clang/test/CodeGenCXX/catch-undef-behavior.cpp
@@ -426,6 +426,25 @@ void indirect_function_call(void (*p)(int)) {
p(42);
}
+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 {};
+ // CHECK-LABEL: define {{.*}} @_ZN15VBaseObjectSize1fERNS_1BE(
+ B &f(B &b) {
+ // Size check: check for nvsize(B) == 16 (do not require size(B) == 32)
+ // CHECK: [[SIZE:%.+]] = call i{{32|64}} @llvm.objectsize.i64.p0i8(
+ // CHECK: icmp uge i{{32|64}} [[SIZE]], 16,
+
+ // Alignment check: check for nvalign(B) == 8 (do not require align(B) == 16)
+ // CHECK: [[PTRTOINT:%.+]] = ptrtoint {{.*}} to i64,
+ // CHECK: and i64 [[PTRTOINT]], 7,
+ return b;
+ }
+}
+
namespace FunctionSanitizerVirtualCalls {
struct A {
virtual void f() {}
diff --git a/clang/test/CodeGenCXX/thunks.cpp b/clang/test/CodeGenCXX/thunks.cpp
index 4a0610ed488d..fe7d656eb7e5 100644
--- a/clang/test/CodeGenCXX/thunks.cpp
+++ b/clang/test/CodeGenCXX/thunks.cpp
@@ -412,7 +412,7 @@ namespace Test13 {
// CHECK: getelementptr inbounds i8, i8* {{.*}}, i64 8
// CHECK: ret %"struct.Test13::D"*
- // WIN64-LABEL: define weak_odr dso_local dereferenceable(32) %"struct.Test13::D"* @"?foo1 at D@Test13@@$4PPPPPPPE at A@EAAAEAUB1 at 2@XZ"(
+ // WIN64-LABEL: define weak_odr dso_local dereferenceable(8) %"struct.Test13::D"* @"?foo1 at D@Test13@@$4PPPPPPPE at A@EAAAEAUB1 at 2@XZ"(
// This adjustment.
// WIN64: getelementptr inbounds i8, i8* {{.*}}, i64 -12
// Call implementation.
More information about the cfe-commits
mailing list