[cfe-dev] Integer overflow checking
David Chisnall
csdavec at swansea.ac.uk
Sat May 16 09:45:57 PDT 2009
An earlier version added a flag which supported unsigned overflow
checking, but this was rejected by Chris because signed overflow is
well-defined behaviour in C.
Please, please, please, don't remove the ability to replace the
overflowed value with another one. I am using this feature, and
replacing it with an implementation without this feature would be a
very irritating regression.
David
On 16 May 2009, at 15:19, Martin Doucha wrote:
> Hi,
> I've looked at the open projects page and noticed that you're
> interested in run-time integer overflow check generator in CodeGen.
> Here's my CodeGen hack that does just that (even for ++/--, it's not
> compatible with the original -ftrapv implementation though) from my
> bachelor thesis. rtl.c is a sample implementation of abort helper
> function which prints pretty error messages. Other nice checks can
> be found in my thesis project at http://repo.or.cz/w/clang/acc.git
>
> Regards,
> Martin Doucha
> diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/
> CGExprScalar.cpp
> index 5e0159e..37b0948 100644
> --- a/lib/CodeGen/CGExprScalar.cpp
> +++ b/lib/CodeGen/CGExprScalar.cpp
> @@ -18,6 +18,7 @@
> #include "clang/AST/RecordLayout.h"
> #include "clang/AST/StmtVisitor.h"
> #include "clang/Basic/TargetInfo.h"
> +#include "clang/Basic/SourceManager.h"
> #include "llvm/Constants.h"
> #include "llvm/Function.h"
> #include "llvm/GlobalVariable.h"
> @@ -272,13 +273,14 @@ public:
> // Binary Operators.
> Value *EmitMul(const BinOpInfo &Ops) {
> if (CGF.getContext().getLangOptions().OverflowChecking
> - && Ops.Ty->isSignedIntegerType())
> + && Ops.Ty->isIntegerType())
> return EmitOverflowCheckedBinOp(Ops);
> return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul");
> }
> /// Create a binary op that checks for overflow.
> /// Currently only supports +, - and *.
> Value *EmitOverflowCheckedBinOp(const BinOpInfo &Ops);
> + Value *EmitOverflowInst(unsigned IID, std::string &err,
> PresumedLoc loc, const BinOpInfo &Info);
> Value *EmitDiv(const BinOpInfo &Ops);
> Value *EmitRem(const BinOpInfo &Ops);
> Value *EmitAdd(const BinOpInfo &Ops);
> @@ -679,7 +681,20 @@ Value
> *ScalarExprEmitter::VisitPrePostIncDec(const UnaryOperator *E,
> &ignored);
> NextVal = llvm::ConstantFP::get(F);
> }
> - NextVal = Builder.CreateAdd(InVal, NextVal, isInc ? "inc" :
> "dec");
> +
> + if (CGF.getContext().getLangOptions().OverflowChecking && ValTy-
> >isIntegerType()) {
> + std::string e = isInc ? "Integer increment overflow" :
> "Integer decrement overflow";
> + unsigned IID = ValTy->isSignedIntegerType() ?
> + llvm::Intrinsic::sadd_with_overflow :
> llvm::Intrinsic::uadd_with_overflow;
> + PresumedLoc loc =
> CGF.getContext().getSourceManager().getPresumedLoc(E-
> >getOperatorLoc());
> + BinOpInfo Ops;
> + Ops.Ty = ValTy;
> + Ops.LHS = InVal;
> + Ops.RHS = NextVal;
> + NextVal = EmitOverflowInst(IID, e, loc, Ops);
> + } else {
> + NextVal = Builder.CreateAdd(InVal, NextVal, isInc ? "inc" :
> "dec");
> + }
> }
>
> // Store the updated result through the lvalue.
> @@ -847,41 +862,48 @@ Value *ScalarExprEmitter::EmitRem(const
> BinOpInfo &Ops) {
>
> Value *ScalarExprEmitter::EmitOverflowCheckedBinOp(const BinOpInfo
> &Ops) {
> unsigned IID;
> - unsigned OpID = 0;
> + std::string e;
>
> switch (Ops.E->getOpcode()) {
> case BinaryOperator::Add:
> case BinaryOperator::AddAssign:
> - OpID = 1;
> - IID = llvm::Intrinsic::sadd_with_overflow;
> + e = "Integer addition overflow";
> + IID = Ops.Ty->isSignedIntegerType() ?
> llvm::Intrinsic::sadd_with_overflow
> + : llvm::Intrinsic::uadd_with_overflow;
> break;
> case BinaryOperator::Sub:
> case BinaryOperator::SubAssign:
> - OpID = 2;
> - IID = llvm::Intrinsic::ssub_with_overflow;
> + e = "Integer subtraction overflow";
> + IID = Ops.Ty->isSignedIntegerType() ?
> llvm::Intrinsic::ssub_with_overflow
> + : llvm::Intrinsic::usub_with_overflow;
> break;
> case BinaryOperator::Mul:
> case BinaryOperator::MulAssign:
> - OpID = 3;
> - IID = llvm::Intrinsic::smul_with_overflow;
> + e = "Integer multiplication overflow";
> + IID = Ops.Ty->isSignedIntegerType() ?
> llvm::Intrinsic::smul_with_overflow
> + : llvm::Intrinsic::umul_with_overflow;
> break;
> default:
> assert(false && "Unsupported operation for overflow detection");
> IID = 0;
> }
> - OpID <<= 1;
> - OpID |= 1;
>
> - const llvm::Type *opTy = CGF.CGM.getTypes().ConvertType(Ops.Ty);
> + PresumedLoc loc =
> CGF.getContext().getSourceManager().getPresumedLoc(Ops.E-
> >getOperatorLoc());
> +
> + return EmitOverflowInst(IID, e, loc, Ops);
> +}
> +
> +// Info.E is not used
> +Value *ScalarExprEmitter::EmitOverflowInst(unsigned IID,
> std::string &err, PresumedLoc loc, const BinOpInfo &Info) {
> + const llvm::Type *opTy = CGF.CGM.getTypes().ConvertType(Info.Ty);
>
> llvm::Function *intrinsic = CGF.CGM.getIntrinsic(IID, &opTy, 1);
>
> - Value *resultAndOverflow = Builder.CreateCall2(intrinsic,
> Ops.LHS, Ops.RHS);
> + Value *resultAndOverflow = Builder.CreateCall2(intrinsic,
> Info.LHS, Info.RHS);
> Value *result = Builder.CreateExtractValue(resultAndOverflow, 0);
> Value *overflow = Builder.CreateExtractValue(resultAndOverflow, 1);
>
> // Branch in case of overflow.
> - llvm::BasicBlock *initialBB = Builder.GetInsertBlock();
> llvm::BasicBlock *overflowBB =
> CGF.createBasicBlock("overflow", CGF.CurFn);
> llvm::BasicBlock *continueBB =
> @@ -893,47 +915,36 @@ Value
> *ScalarExprEmitter::EmitOverflowCheckedBinOp(const BinOpInfo &Ops) {
>
> Builder.SetInsertPoint(overflowBB);
>
> + QualType cCharPtr =
> CGF.getContext().getPointerType(CGF.getContext().CharTy);
> + cCharPtr.setCVRQualifiers(QualType::Const);
> + QualType integer = CGF.getContext().IntTy;
> +
> + llvm::Value *error =
> Builder.CreateStructGEP(CGF.CGM.GetAddrOfConstantCString(err), 0,
> "arraydecay");
> + llvm::Value *func =
> Builder.CreateStructGEP(CGF.CGM.GetAddrOfConstantCString(CGF.CurFn-
> >getName()), 0, "arraydecay");
> + llvm::Value *file =
> Builder
> .CreateStructGEP
> (CGF.CGM.GetAddrOfConstantCString(std::string(loc.getFilename())),
> 0, "arraydecay");
> +
> // Handler is:
> - // long long *__overflow_handler)(long long a, long long b, char
> op,
> - // char width)
> - std::vector<const llvm::Type*> handerArgTypes;
> - handerArgTypes.push_back(llvm::Type::Int64Ty);
> - handerArgTypes.push_back(llvm::Type::Int64Ty);
> - handerArgTypes.push_back(llvm::Type::Int8Ty);
> - handerArgTypes.push_back(llvm::Type::Int8Ty);
> - llvm::FunctionType *handlerTy =
> llvm::FunctionType::get(llvm::Type::Int64Ty,
> - handerArgTypes, false);
> - llvm::Value *handlerFunction =
> - CGF.CGM.getModule().getOrInsertGlobal("__overflow_handler",
> - llvm::PointerType::getUnqual(handlerTy));
> - handlerFunction = Builder.CreateLoad(handlerFunction);
> -
> - llvm::Value *handlerResult = Builder.CreateCall4(handlerFunction,
> - Builder.CreateSExt(Ops.LHS, llvm::Type::Int64Ty),
> - Builder.CreateSExt(Ops.RHS, llvm::Type::Int64Ty),
> - llvm::ConstantInt::get(llvm::Type::Int8Ty, OpID),
> - llvm::ConstantInt::get(llvm::Type::Int8Ty,
> - cast<llvm::IntegerType>(opTy)->getBitWidth()));
> -
> - handlerResult = Builder.CreateTrunc(handlerResult, opTy);
> -
> - Builder.CreateBr(continueBB);
> + // void __acc_runtime_failure_line(const char *error, const char
> *func,
> + // const char *file, int line)
> + llvm::Function *handlerFunction = llvm::dyn_cast<llvm::Function>(
> +
> CGF
> .CGM
> .getModule
> ().getOrInsertFunction(std::string("__acc_runtime_failure_line"),
> llvm::Type::VoidTy, error->getType(), func->getType(), file-
> >getType(), ConvertType(integer), NULL));
> + assert(handlerFunction && "getOrInsertFunction didn't produce
> llvm::Function");
> + handlerFunction->addFnAttr(llvm::Attribute::NoReturn |
> llvm::Attribute::NoUnwind);
> +
> + Builder.CreateCall4(handlerFunction, error, func, file,
> + llvm::ConstantInt::get(ConvertType(integer), loc.getLine()));
> + Builder.CreateUnreachable();
> +
>
> // Set up the continuation
> Builder.SetInsertPoint(continueBB);
> - // Get the correct result
> - llvm::PHINode *phi = Builder.CreatePHI(opTy);
> - phi->reserveOperandSpace(2);
> - phi->addIncoming(result, initialBB);
> - phi->addIncoming(handlerResult, overflowBB);
> -
> - return phi;
> + return result;
> }
>
> Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &Ops) {
> if (!Ops.Ty->isPointerType()) {
> if (CGF.getContext().getLangOptions().OverflowChecking
> - && Ops.Ty->isSignedIntegerType())
> + && Ops.Ty->isIntegerType())
> return EmitOverflowCheckedBinOp(Ops);
> return Builder.CreateAdd(Ops.LHS, Ops.RHS, "add");
> }
> @@ -998,7 +1009,7 @@ Value *ScalarExprEmitter::EmitAdd(const
> BinOpInfo &Ops) {
> Value *ScalarExprEmitter::EmitSub(const BinOpInfo &Ops) {
> if (!isa<llvm::PointerType>(Ops.LHS->getType())) {
> if (CGF.getContext().getLangOptions().OverflowChecking
> - && Ops.Ty->isSignedIntegerType())
> + && Ops.Ty->isIntegerType())
> return EmitOverflowCheckedBinOp(Ops);
> return Builder.CreateSub(Ops.LHS, Ops.RHS, "sub");
> }
> #include <stdio.h>
> #include <stdlib.h>
>
> void __acc_runtime_failure(const char *error, const char *func) {
> fprintf(stderr, "Runtime check failure in function %s: %s\n", func,
> error);
> abort();
> }
>
> void __acc_runtime_failure_line(const char *error, const char *func,
> const char *file, int line) {
>
> fprintf(stderr, "Runtime check failure in function %s (%s:%d): %s\n",
> func, file, line, error);
> abort();
> }
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
More information about the cfe-dev
mailing list