Skip to content

Commit a8a0ffb

Browse files
authored
[clang][bytecode] Check source pointer for bitcast validity (llvm#166907)
Unfortunately this is more dynamic than anticipated. Fixes llvm#165006
1 parent 996639d commit a8a0ffb

File tree

6 files changed

+81
-20
lines changed

6 files changed

+81
-20
lines changed

clang/lib/AST/ByteCode/Compiler.cpp

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -208,19 +208,6 @@ template <class Emitter> class LocOverrideScope final {
208208
} // namespace interp
209209
} // namespace clang
210210

211-
template <class Emitter>
212-
bool Compiler<Emitter>::isValidBitCast(const CastExpr *E) {
213-
QualType FromTy = E->getSubExpr()->getType()->getPointeeType();
214-
QualType ToTy = E->getType()->getPointeeType();
215-
216-
if (classify(FromTy) == classify(ToTy))
217-
return true;
218-
219-
if (FromTy->isVoidType() || ToTy->isVoidType())
220-
return true;
221-
return false;
222-
}
223-
224211
template <class Emitter>
225212
bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
226213
const Expr *SubExpr = CE->getSubExpr();
@@ -506,12 +493,9 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
506493
if (!FromT || !ToT)
507494
return false;
508495

509-
if (!this->isValidBitCast(CE) &&
510-
!this->emitInvalidCast(CastKind::ReinterpretLike, /*Fatal=*/false, CE))
511-
return false;
512-
513496
assert(isPtrType(*FromT));
514497
assert(isPtrType(*ToT));
498+
bool SrcIsVoidPtr = SubExprTy->isVoidPointerType();
515499
if (FromT == ToT) {
516500
if (CE->getType()->isVoidPointerType() &&
517501
!SubExprTy->isFunctionPointerType()) {
@@ -520,6 +504,10 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
520504

521505
if (!this->visit(SubExpr))
522506
return false;
507+
if (!this->emitCheckBitCast(CETy->getPointeeType().getTypePtr(),
508+
SrcIsVoidPtr, CE))
509+
return false;
510+
523511
if (CE->getType()->isFunctionPointerType() ||
524512
SubExprTy->isFunctionPointerType()) {
525513
return this->emitFnPtrCast(CE);

clang/lib/AST/ByteCode/Compiler.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -425,8 +425,6 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
425425

426426
bool refersToUnion(const Expr *E);
427427

428-
bool isValidBitCast(const CastExpr *E);
429-
430428
protected:
431429
/// Variable to storage mapping.
432430
llvm::DenseMap<const ValueDecl *, Scope::Local> Locals;

clang/lib/AST/ByteCode/Interp.h

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3290,6 +3290,52 @@ inline bool SideEffect(InterpState &S, CodePtr OpPC) {
32903290
return S.noteSideEffect();
32913291
}
32923292

3293+
inline bool CheckBitCast(InterpState &S, CodePtr OpPC, const Type *TargetType,
3294+
bool SrcIsVoidPtr) {
3295+
const auto &Ptr = S.Stk.peek<Pointer>();
3296+
if (Ptr.isZero())
3297+
return true;
3298+
if (!Ptr.isBlockPointer())
3299+
return true;
3300+
3301+
if (TargetType->isIntegerType())
3302+
return true;
3303+
3304+
if (SrcIsVoidPtr && S.getLangOpts().CPlusPlus) {
3305+
bool HasValidResult = !Ptr.isZero();
3306+
3307+
if (HasValidResult) {
3308+
if (S.getStdAllocatorCaller("allocate"))
3309+
return true;
3310+
3311+
const auto &E = cast<CastExpr>(S.Current->getExpr(OpPC));
3312+
if (S.getLangOpts().CPlusPlus26 &&
3313+
S.getASTContext().hasSimilarType(Ptr.getType(),
3314+
QualType(TargetType, 0)))
3315+
return true;
3316+
3317+
S.CCEDiag(E, diag::note_constexpr_invalid_void_star_cast)
3318+
<< E->getSubExpr()->getType() << S.getLangOpts().CPlusPlus26
3319+
<< Ptr.getType().getCanonicalType() << E->getType()->getPointeeType();
3320+
} else if (!S.getLangOpts().CPlusPlus26) {
3321+
const SourceInfo &E = S.Current->getSource(OpPC);
3322+
S.CCEDiag(E, diag::note_constexpr_invalid_cast)
3323+
<< diag::ConstexprInvalidCastKind::CastFrom << "'void *'"
3324+
<< S.Current->getRange(OpPC);
3325+
}
3326+
}
3327+
3328+
QualType PtrType = Ptr.getType();
3329+
if (PtrType->isRecordType() &&
3330+
PtrType->getAsRecordDecl() != TargetType->getAsRecordDecl()) {
3331+
S.CCEDiag(S.Current->getSource(OpPC), diag::note_constexpr_invalid_cast)
3332+
<< diag::ConstexprInvalidCastKind::ThisConversionOrReinterpret
3333+
<< S.getLangOpts().CPlusPlus << S.Current->getRange(OpPC);
3334+
return false;
3335+
}
3336+
return true;
3337+
}
3338+
32933339
/// Same here, but only for casts.
32943340
inline bool InvalidCast(InterpState &S, CodePtr OpPC, CastKind Kind,
32953341
bool Fatal) {

clang/lib/AST/ByteCode/Opcodes.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,8 +422,8 @@ def CheckLiteralType : Opcode {
422422
}
423423

424424
def CheckArraySize : Opcode { let Args = [ArgUint64]; }
425-
426425
def CheckFunctionDecl : Opcode { let Args = [ArgFunctionDecl]; }
426+
def CheckBitCast : Opcode { let Args = [ArgTypePtr, ArgBool]; }
427427

428428
// [] -> [Value]
429429
def GetGlobal : AccessOpcode;

clang/test/AST/ByteCode/cxx11.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,3 +387,14 @@ struct Counter {
387387
// Passing an lvalue by value makes a non-elidable copy.
388388
constexpr int PassByValue(Counter c) { return c.copies; }
389389
static_assert(PassByValue(Counter(0)) == 0, "expect no copies");
390+
391+
namespace PointerCast {
392+
/// The two interpreters disagree here.
393+
struct S { int x, y; } s;
394+
constexpr S* sptr = &s;
395+
struct U {};
396+
struct Str {
397+
int e : (Str*)(sptr) == (Str*)(sptr); // expected-error {{not an integral constant expression}} \
398+
// expected-note {{cast that performs the conversions of a reinterpret_cast}}
399+
};
400+
}

clang/test/AST/ByteCode/invalid.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,4 +88,22 @@ namespace InvalidBitCast {
8888
// both-note {{in call to}}
8989

9090

91+
struct sockaddr
92+
{
93+
char sa_data[8];
94+
};
95+
struct in_addr
96+
{
97+
unsigned int s_addr;
98+
};
99+
struct sockaddr_in
100+
{
101+
unsigned short int sin_port;
102+
struct in_addr sin_addr;
103+
};
104+
/// Bitcast from sockaddr to sockaddr_in. Used to crash.
105+
unsigned int get_addr(sockaddr addr) {
106+
return ((sockaddr_in *)&addr)->sin_addr.s_addr;
107+
}
108+
91109
}

0 commit comments

Comments
 (0)