[clang][bytecode] Check source pointer for bitcast validity #166907
Merged
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
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesUnfortunately this is more dynamic than anticipated. Fixes #165006 Full diff: https://github.com/llvm/llvm-project/pull/166907.diff 6 Files Affected:
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 4e634000adc3b..7e8b162517cff 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -208,19 +208,6 @@ template <class Emitter> class LocOverrideScope final {
} // namespace interp
} // namespace clang
-template <class Emitter>
-bool Compiler<Emitter>::isValidBitCast(const CastExpr *E) {
- QualType FromTy = E->getSubExpr()->getType()->getPointeeType();
- QualType ToTy = E->getType()->getPointeeType();
-
- if (classify(FromTy) == classify(ToTy))
- return true;
-
- if (FromTy->isVoidType() || ToTy->isVoidType())
- return true;
- return false;
-}
-
template <class Emitter>
bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
const Expr *SubExpr = CE->getSubExpr();
@@ -506,12 +493,9 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
if (!FromT || !ToT)
return false;
- if (!this->isValidBitCast(CE) &&
- !this->emitInvalidCast(CastKind::ReinterpretLike, /*Fatal=*/false, CE))
- return false;
-
assert(isPtrType(*FromT));
assert(isPtrType(*ToT));
+ bool SrcIsVoidPtr = SubExprTy->isVoidPointerType();
if (FromT == ToT) {
if (CE->getType()->isVoidPointerType() &&
!SubExprTy->isFunctionPointerType()) {
@@ -520,6 +504,10 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
if (!this->visit(SubExpr))
return false;
+ if (!this->emitCheckBitCast(CETy->getPointeeType().getTypePtr(),
+ SrcIsVoidPtr, CE))
+ return false;
+
if (CE->getType()->isFunctionPointerType() ||
SubExprTy->isFunctionPointerType()) {
return this->emitFnPtrCast(CE);
diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h
index fac0a7f4e1886..5c46f75af4da3 100644
--- a/clang/lib/AST/ByteCode/Compiler.h
+++ b/clang/lib/AST/ByteCode/Compiler.h
@@ -425,8 +425,6 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
bool refersToUnion(const Expr *E);
- bool isValidBitCast(const CastExpr *E);
-
protected:
/// Variable to storage mapping.
llvm::DenseMap<const ValueDecl *, Scope::Local> Locals;
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 6877b03f5916b..7afaf1195525b 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -3289,6 +3289,52 @@ inline bool SideEffect(InterpState &S, CodePtr OpPC) {
return S.noteSideEffect();
}
+inline bool CheckBitCast(InterpState &S, CodePtr OpPC, const Type *TargetType,
+ bool SrcIsVoidPtr) {
+ const auto &Ptr = S.Stk.peek<Pointer>();
+ if (Ptr.isZero())
+ return true;
+ if (!Ptr.isBlockPointer())
+ return true;
+
+ if (TargetType->isIntegerType())
+ return true;
+
+ if (SrcIsVoidPtr && S.getLangOpts().CPlusPlus) {
+ bool HasValidResult = !Ptr.isZero();
+
+ if (HasValidResult) {
+ if (S.getStdAllocatorCaller("allocate"))
+ return true;
+
+ const auto &E = cast<CastExpr>(S.Current->getExpr(OpPC));
+ if (S.getLangOpts().CPlusPlus26 &&
+ S.getASTContext().hasSimilarType(Ptr.getType(),
+ QualType(TargetType, 0)))
+ return true;
+
+ S.CCEDiag(E, diag::note_constexpr_invalid_void_star_cast)
+ << E->getSubExpr()->getType() << S.getLangOpts().CPlusPlus26
+ << Ptr.getType().getCanonicalType() << E->getType()->getPointeeType();
+ } else if (!S.getLangOpts().CPlusPlus26) {
+ const SourceInfo &E = S.Current->getSource(OpPC);
+ S.CCEDiag(E, diag::note_constexpr_invalid_cast)
+ << diag::ConstexprInvalidCastKind::CastFrom << "'void *'"
+ << S.Current->getRange(OpPC);
+ }
+ }
+
+ QualType PtrType = Ptr.getType();
+ if (PtrType->isRecordType() &&
+ PtrType->getAsRecordDecl() != TargetType->getAsRecordDecl()) {
+ S.CCEDiag(S.Current->getSource(OpPC), diag::note_constexpr_invalid_cast)
+ << diag::ConstexprInvalidCastKind::ThisConversionOrReinterpret
+ << S.getLangOpts().CPlusPlus << S.Current->getRange(OpPC);
+ return false;
+ }
+ return true;
+}
+
/// Same here, but only for casts.
inline bool InvalidCast(InterpState &S, CodePtr OpPC, CastKind Kind,
bool Fatal) {
diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td
index 1c17ad9e95d05..976d66664f8d3 100644
--- a/clang/lib/AST/ByteCode/Opcodes.td
+++ b/clang/lib/AST/ByteCode/Opcodes.td
@@ -422,6 +422,8 @@ def CheckLiteralType : Opcode {
def CheckArraySize : Opcode { let Args = [ArgUint64]; }
+def CheckBitCast : Opcode { let Args = [ArgTypePtr, ArgBool]; }
+
// [] -> [Value]
def GetGlobal : AccessOpcode;
def GetGlobalUnchecked : AccessOpcode;
diff --git a/clang/test/AST/ByteCode/cxx11.cpp b/clang/test/AST/ByteCode/cxx11.cpp
index e283a7b42e554..8449ee0408f99 100644
--- a/clang/test/AST/ByteCode/cxx11.cpp
+++ b/clang/test/AST/ByteCode/cxx11.cpp
@@ -379,3 +379,13 @@ namespace DiscardedAddrLabel {
}
}
+namespace PointerCast {
+ /// The two interpreters disagree here.
+ struct S { int x, y; } s;
+ constexpr S* sptr = &s;
+ struct U {};
+ struct Str {
+ int e : (Str*)(sptr) == (Str*)(sptr); // expected-error {{not an integral constant expression}} \
+ // expected-note {{cast that performs the conversion of a reinterpret_cast}}
+ };
+}
diff --git a/clang/test/AST/ByteCode/invalid.cpp b/clang/test/AST/ByteCode/invalid.cpp
index 1f2d6bc1d48eb..115c8663079a1 100644
--- a/clang/test/AST/ByteCode/invalid.cpp
+++ b/clang/test/AST/ByteCode/invalid.cpp
@@ -88,4 +88,22 @@ namespace InvalidBitCast {
// both-note {{in call to}}
+ struct sockaddr
+ {
+ char sa_data[8];
+ };
+ struct in_addr
+ {
+ unsigned int s_addr;
+ };
+ struct sockaddr_in
+ {
+ unsigned short int sin_port;
+ struct in_addr sin_addr;
+ };
+ /// Bitcast from sockaddr to sockaddr_in. Used to crash.
+ unsigned int get_addr(sockaddr addr) {
+ return ((sockaddr_in *)&addr)->sin_addr.s_addr;
+ }
+
}
|
Unfortunately this is more dynamic than anticipated. Fixes llvm#165006
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/42076 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/16934 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Unfortunately this is more dynamic than anticipated.
Fixes #165006