Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a dynamic check for pointer arithmetic of null pointers #663

Merged
merged 4 commits into from
Aug 20, 2019
Merged

Add a dynamic check for pointer arithmetic of null pointers #663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2eb2b30
Add a dynamic check for pointer arithmetic of null pointers
Aug 14, 2019
f284c26
Fix Typo
Aug 16, 2019
d039f06
Rename the flag and fix behavior of -fno-checkedc-null-ptr-arith
Aug 19, 2019
cb201aa
Remove debugging code
Aug 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions include/clang/Basic/CodeGenOptions.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,9 @@ CODEGENOPT(BranchTargetEnforcement, 1, 0)
/// Whether to emit unused static constants.
CODEGENOPT(KeepStaticConsts, 1, 0)

/// Whether to add dynamic checks for null pointer arithmetic.
CODEGENOPT(CheckedCNullPtrArith, 1, 1)

#undef CODEGENOPT
#undef ENUM_CODEGENOPT
#undef VALUE_CODEGENOPT
Expand Down
8 changes: 8 additions & 0 deletions include/clang/Driver/Options.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2621,6 +2621,14 @@ def fintegrated_as : Flag<["-"], "fintegrated-as">, Flags<[DriverOption]>,
def fno_integrated_as : Flag<["-"], "fno-integrated-as">,
Flags<[CC1Option, DriverOption]>, Group<f_Group>,
HelpText<"Disable the integrated assembler">;

def fcheckedc_null_ptr_arith : Flag<["-"], "fcheckedc-null-ptr-arith">,
Group<f_Group>, Flags<[CC1Option]>,
HelpText<"Enable runtime null pointer arithmetic checks">;
def fno_checkedc_null_ptr_arith : Flag<["-"], "fno-checkedc-null-ptr-arith">,
Group<f_Group>, Flags<[CC1Option]>,
HelpText<"Disable runtime null pointer arithmetic checks">;

def : Flag<["-"], "integrated-as">, Alias<fintegrated_as>, Flags<[DriverOption]>;
def : Flag<["-"], "no-integrated-as">, Alias<fno_integrated_as>,
Flags<[CC1Option, DriverOption]>;
Expand Down
28 changes: 24 additions & 4 deletions lib/CodeGen/CGDynamicCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,38 @@ void CodeGenFunction::EmitExplicitDynamicCheck(const Expr *Condition) {
// General Functions for inserting dynamic checks
//

static bool shouldEmitNonNullCheck(const CodeGenModule &CGM,
const QualType BaseTy) {
if (!CGM.getLangOpts().CheckedC)
return false;

if (!(BaseTy->isCheckedPointerType() || BaseTy->isCheckedArrayType()))
return false;

return true;
}

void CodeGenFunction::EmitDynamicNonNullCheck(const Address BaseAddr,
const QualType BaseTy) {
if (!getLangOpts().CheckedC)
if (!shouldEmitNonNullCheck(CGM, BaseTy))
return;

if (!(BaseTy->isCheckedPointerType() || BaseTy->isCheckedArrayType()))
++NumDynamicChecksNonNull;

Value *ConditionVal = Builder.CreateIsNotNull(BaseAddr.getPointer(),
"_Dynamic_check.non_null");
EmitDynamicCheckBlocks(ConditionVal);
}

void CodeGenFunction::EmitDynamicNonNullCheck(Value *Val,
const QualType BaseTy) {
if (!shouldEmitNonNullCheck(CGM, BaseTy))
return;

++NumDynamicChecksNonNull;

Value *ConditionVal =
Builder.CreateIsNotNull(BaseAddr.getPointer(), "_Dynamic_check.non_null");
Value *ConditionVal = Builder.CreateIsNotNull(Val,
"_Dynamic_check.non_null");
EmitDynamicCheckBlocks(ConditionVal);
}

Expand Down
14 changes: 14 additions & 0 deletions lib/CodeGen/CGExprScalar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2319,6 +2319,14 @@ static BinOpInfo createBinOpInfoFromIncDec(const UnaryOperator *E,
return BinOp;
}

static void emitDynamicNonNullCheck(CodeGenFunction &CGF,
Value *Val, QualType Ty) {
if (!CGF.CGM.getCodeGenOpts().CheckedCNullPtrArith)
return;

CGF.EmitDynamicNonNullCheck(Val, Ty);
}

llvm::Value *ScalarExprEmitter::EmitIncDecConsiderOverflowBehavior(
const UnaryOperator *E, llvm::Value *InVal, bool IsInc) {
llvm::Value *Amount =
Expand Down Expand Up @@ -2427,6 +2435,9 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,

// Next most common: pointer increment.
} else if (const PointerType *ptr = type->getAs<PointerType>()) {
// Insert a dynamic check for arithmetic on null checked pointers.
emitDynamicNonNullCheck(CGF, value, type);

QualType type = ptr->getPointeeType();

// VLA types don't have constant size.
Expand Down Expand Up @@ -3162,6 +3173,9 @@ static Value *emitPointerArithmetic(CodeGenFunction &CGF,
std::swap(pointerOperand, indexOperand);
}

// Insert a dynamic check for arithmetic on null checked pointers.
emitDynamicNonNullCheck(CGF, pointer, pointerOperand->getType());

bool isSigned = indexOperand->getType()->isSignedIntegerOrEnumerationType();

unsigned width = cast<llvm::IntegerType>(index->getType())->getBitWidth();
Expand Down
1 change: 1 addition & 0 deletions lib/CodeGen/CodeGenFunction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2923,6 +2923,7 @@ class CodeGenFunction : public CodeGenTypeCache {

void EmitExplicitDynamicCheck(const Expr *Condition);
void EmitDynamicNonNullCheck(const Address BaseAddr, const QualType BaseTy);
void EmitDynamicNonNullCheck(llvm::Value *Val, const QualType BaseTy);
void EmitDynamicOverflowCheck(const Address BaseAddr, const QualType BaseTy,
const Address PtrAddr);
/// \brief Emit a dynamic bounds check.
Expand Down
3 changes: 3 additions & 0 deletions lib/Driver/ToolChains/Clang.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4721,6 +4721,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
Args.AddLastArg(CmdArgs, options::OPT_fno_checkedc_extension);
Args.AddLastArg(CmdArgs, options::OPT_fdump_inferred_bounds);

Args.AddLastArg(CmdArgs, options::OPT_fcheckedc_null_ptr_arith,
options::OPT_fno_checkedc_null_ptr_arith);

// -fno-declspec is default, except for PS4.
if (Args.hasFlag(options::OPT_fdeclspec, options::OPT_fno_declspec,
RawTriple.isPS4()))
Expand Down
2 changes: 2 additions & 0 deletions lib/Frontend/CompilerInvocation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1322,6 +1322,8 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,

Opts.DefaultFunctionAttrs = Args.getAllArgValues(OPT_default_function_attr);

Opts.CheckedCNullPtrArith = !Args.hasArg(OPT_fno_checkedc_null_ptr_arith);

return Success;
}

Expand Down
2 changes: 1 addition & 1 deletion test/CheckedC/dynamic-checks/array-compound-assign-code-gen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

// RUN: %clang_cc1 -fcheckedc-extension -verify -verify-ignore-unexpected=note %s -ast-dump | FileCheck %s --check-prefix=CHECK-AST
// RUN: %clang_cc1 -fcheckedc-extension -verify -verify-ignore-unexpected=note %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR
// RUN: %clang_cc1 -fcheckedc-extension -fno-checkedc-null-ptr-arith -verify -verify-ignore-unexpected=note %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default, null ptr arithmetic checks are enabled. This breaks a few test cases which perform ptr arithmetic but which do not expect the checks. So I had to add the flag -fno-checkedc-null-ptr-arith to keep these tests from breaking.


// In the following generated IR, we do not verify the alignment of any loads/stores
// ie, the IR checked by line 37 might read "%1 = load i32*, i32** @gp1, align 4"
Expand Down
2 changes: 1 addition & 1 deletion test/CheckedC/dynamic-checks/array-increment-code-gen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

// RUN: %clang_cc1 -fcheckedc-extension -verify -verify-ignore-unexpected=note %s -ast-dump | FileCheck %s --check-prefix=CHECK-AST
// RUN: %clang_cc1 -fcheckedc-extension -verify -verify-ignore-unexpected=note %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR
// RUN: %clang_cc1 -fcheckedc-extension -fno-checkedc-null-ptr-arith -verify -verify-ignore-unexpected=note %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR

// In the following generated IR, we do not verify the alignment of any loads/stores
// ie, the IR checked by line 37 might read "%1 = load i32*, i32** @gp1, align 4"
Expand Down
2 changes: 1 addition & 1 deletion test/CheckedC/dynamic-checks/array-read-code-gen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

// RUN: %clang_cc1 -fcheckedc-extension -verify -verify-ignore-unexpected=note %s -ast-dump | FileCheck %s --check-prefix=CHECK-AST
// RUN: %clang_cc1 -fcheckedc-extension -verify -verify-ignore-unexpected=note %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR
// RUN: %clang_cc1 -fcheckedc-extension -fno-checkedc-null-ptr-arith -verify -verify-ignore-unexpected=note %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR

// In the following generated IR, we do not verify the alignment of any loads/stores
// ie, the IR checked by line 37 might read "%1 = load i32*, i32** @gp1, align 4"
Expand Down
2 changes: 1 addition & 1 deletion test/CheckedC/dynamic-checks/array-write-code-gen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

// RUN: %clang_cc1 -fcheckedc-extension -verify -verify-ignore-unexpected=note %s -ast-dump | FileCheck %s --check-prefix=CHECK-AST
// RUN: %clang_cc1 -fcheckedc-extension -verify -verify-ignore-unexpected=note %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR
// RUN: %clang_cc1 -fcheckedc-extension -fno-checkedc-null-ptr-arith -verify -verify-ignore-unexpected=note %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR

// In the following generated IR, we do not verify the alignment of any loads/stores
// ie, the IR checked by line 37 might read "%1 = load i32*, i32** @gp1, align 4"
Expand Down
111 changes: 111 additions & 0 deletions test/CheckedC/dynamic-checks/ptr-arith-null-check-code-gen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s

#include <stdchecked.h>
#include <stddef.h>

array_ptr<char> p : count(3) = "abc";
array_ptr<char> q : count(3) = "abc";
// NOCHECK-NOT: _Dynamic_check.non_null

void f1() {
// CHECK-LABEL: f1
// CHECK: [[REG0:%[a-zA-Z0-9.]*]] = load i8*, i8** @p, align 8
// CHECK-NEXT: [[REG_DYNN:%_Dynamic_check.non_null[a-zA-Z0-9.]*]] = icmp ne i8* [[REG0]], null
// CHECK-NEXT: br i1 [[REG_DYNN]], label %[[LAB_DYSUC:_Dynamic_check.succeeded[a-zA-Z0-9.]*]], label %[[LAB_DYFAIL:_Dynamic_check.failed[a-zA-Z0-9.]*]]
p++;
}

void f2() {
// CHECK-LABEL: f2
// CHECK: [[REG0:%[a-zA-Z0-9.]*]] = load i8*, i8** @p, align 8
// CHECK-NEXT: [[REG_DYNN:%_Dynamic_check.non_null[a-zA-Z0-9.]*]] = icmp ne i8* [[REG0]], null
// CHECK-NEXT: br i1 [[REG_DYNN]], label %[[LAB_DYSUC:_Dynamic_check.succeeded[a-zA-Z0-9.]*]], label %[[LAB_DYFAIL:_Dynamic_check.failed[a-zA-Z0-9.]*]]
++p;
}

void f3() {
// CHECK-LABEL: f3
// CHECK: [[REG0:%[a-zA-Z0-9.]*]] = load i8*, i8** @p, align 8
// CHECK-NEXT: [[REG_DYNN:%_Dynamic_check.non_null[a-zA-Z0-9.]*]] = icmp ne i8* [[REG0]], null
// CHECK-NEXT: br i1 [[REG_DYNN]], label %[[LAB_DYSUC:_Dynamic_check.succeeded[a-zA-Z0-9.]*]], label %[[LAB_DYFAIL:_Dynamic_check.failed[a-zA-Z0-9.]*]]
p--;
}

void f4() {
// CHECK-LABEL: f4
// CHECK: [[REG0:%[a-zA-Z0-9.]*]] = load i8*, i8** @p, align 8
// CHECK-NEXT: [[REG_DYNN:%_Dynamic_check.non_null[a-zA-Z0-9.]*]] = icmp ne i8* [[REG0]], null
// CHECK-NEXT: br i1 [[REG_DYNN]], label %[[LAB_DYSUC:_Dynamic_check.succeeded[a-zA-Z0-9.]*]], label %[[LAB_DYFAIL:_Dynamic_check.failed[a-zA-Z0-9.]*]]
--p;
}

void f5() {
// CHECK-LABEL: f5
// CHECK: [[REG0:%[a-zA-Z0-9.]*]] = load i8*, i8** @p, align 8
// CHECK-NEXT: [[REG_DYNN:%_Dynamic_check.non_null[a-zA-Z0-9.]*]] = icmp ne i8* [[REG0]], null
// CHECK-NEXT: br i1 [[REG_DYNN]], label %[[LAB_DYSUC:_Dynamic_check.succeeded[a-zA-Z0-9.]*]], label %[[LAB_DYFAIL:_Dynamic_check.failed[a-zA-Z0-9.]*]]
p += 5;
}

void f6() {
// CHECK-LABEL: f6
// CHECK: [[REG0:%[a-zA-Z0-9.]*]] = load i8*, i8** @p, align 8
// CHECK-NEXT: [[REG_DYNN:%_Dynamic_check.non_null[a-zA-Z0-9.]*]] = icmp ne i8* [[REG0]], null
// CHECK-NEXT: br i1 [[REG_DYNN]], label %[[LAB_DYSUC:_Dynamic_check.succeeded[a-zA-Z0-9.]*]], label %[[LAB_DYFAIL:_Dynamic_check.failed[a-zA-Z0-9.]*]]
p -= 10;
}

void f7() {
// CHECK-LABEL: f7
// CHECK: [[REG0:%[a-zA-Z0-9.]*]] = load i8*, i8** @p, align 8
// CHECK-NEXT: [[REG_DYNN:%_Dynamic_check.non_null[a-zA-Z0-9.]*]] = icmp ne i8* [[REG0]], null
// CHECK-NEXT: br i1 [[REG_DYNN]], label %[[LAB_DYSUC:_Dynamic_check.succeeded[a-zA-Z0-9.]*]], label %[[LAB_DYFAIL:_Dynamic_check.failed[a-zA-Z0-9.]*]]
p = 2 + p;
}

void f8() {
// CHECK-LABEL: f8
// CHECK: [[REG0:%[a-zA-Z0-9.]*]] = load i8*, i8** @p, align 8
// CHECK-NEXT: [[REG_DYNN:%_Dynamic_check.non_null[a-zA-Z0-9.]*]] = icmp ne i8* [[REG0]], null
// CHECK-NEXT: br i1 [[REG_DYNN]], label %[[LAB_DYSUC:_Dynamic_check.succeeded[a-zA-Z0-9.]*]], label %[[LAB_DYFAIL:_Dynamic_check.failed[a-zA-Z0-9.]*]]
p = p - 100;
}

void f9() {
// CHECK-LABEL: f9
// CHECK: [[REG0:%[a-zA-Z0-9.]*]] = load i8*, i8** @p, align 8
// CHECK-NEXT: [[REG_DYNN:%_Dynamic_check.non_null[a-zA-Z0-9.]*]] = icmp ne i8* [[REG0]], null
// CHECK-NEXT: br i1 [[REG_DYNN]], label %[[LAB_DYSUC:_Dynamic_check.succeeded[a-zA-Z0-9.]*]], label %[[LAB_DYFAIL:_Dynamic_check.failed[a-zA-Z0-9.]*]]
p[1]++;
}

void f10() {
// CHECK-LABEL: f10
// CHECK: [[REG0:%[a-zA-Z0-9.]*]] = load i8*, i8** @p, align 8
// CHECK-NEXT: [[REG_DYNN:%_Dynamic_check.non_null[a-zA-Z0-9.]*]] = icmp ne i8* [[REG0]], null
// CHECK-NEXT: br i1 [[REG_DYNN]], label %[[LAB_DYSUC:_Dynamic_check.succeeded[a-zA-Z0-9.]*]], label %[[LAB_DYFAIL:_Dynamic_check.failed[a-zA-Z0-9.]*]]

// CHECK: [[REG1:%[a-zA-Z0-9.]*]] = load i8*, i8** @p, align 8
// CHECK: [[REG2:%[a-zA-Z0-9.]*]] = load i8*, i8** @p, align 8
// CHECK-NEXT: [[REG_DYNN:%_Dynamic_check.non_null[a-zA-Z0-9.]*]] = icmp ne i8* [[REG2]], null
// CHECK-NEXT: br i1 [[REG_DYNN]], label %[[LAB_DYSUC:_Dynamic_check.succeeded[a-zA-Z0-9.]*]], label %[[LAB_DYFAIL:_Dynamic_check.failed[a-zA-Z0-9.]*]]
p[1] = p[2] + (*p)++;
}

void f11(array_ptr<char> a);
void f12() {
// CHECK-LABEL: f12
// CHECK: [[REG0:%[a-zA-Z0-9.]*]] = load i8*, i8** @p, align 8
// CHECK-NEXT: [[REG_DYNN:%_Dynamic_check.non_null[a-zA-Z0-9.]*]] = icmp ne i8* [[REG0]], null
// CHECK-NEXT: br i1 [[REG_DYNN]], label %[[LAB_DYSUC:_Dynamic_check.succeeded[a-zA-Z0-9.]*]], label %[[LAB_DYFAIL:_Dynamic_check.failed[a-zA-Z0-9.]*]]
f11(++p);
}

void f13() {
// CHECK-LABEL: f13
// CHECK: [[REG0:%[a-zA-Z0-9.]*]] = load i8*, i8** @p, align 8
// CHECK: [[REG1:%[a-zA-Z0-9.]*]] = load i8*, i8** @q, align 8
// CHECK-NEXT: [[REG_DYNN:%_Dynamic_check.non_null[a-zA-Z0-9.]*]] = icmp ne i8* [[REG1]], null
// CHECK-NEXT: br i1 [[REG_DYNN]], label %[[LAB_DYSUC:_Dynamic_check.succeeded[a-zA-Z0-9.]*]], label %[[LAB_DYFAIL:_Dynamic_check.failed[a-zA-Z0-9.]*]]
p[q[1]++]++;
}
2 changes: 1 addition & 1 deletion test/CheckedC/dynamic-checks/struct-compound-assign-code-gen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

// RUN: %clang_cc1 -fcheckedc-extension %s -ast-dump | FileCheck %s --check-prefix=CHECK-AST
// RUN: %clang_cc1 -fcheckedc-extension %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR
// RUN: %clang_cc1 -fcheckedc-extension -fno-checkedc-null-ptr-arith %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR

// In the following generated IR, we do not verify the alignment of any loads/stores
// ie, the IR checked by line 37 might read "%1 = load i32*, i32** @gp1, align 4"
Expand Down
2 changes: 1 addition & 1 deletion test/CheckedC/dynamic-checks/struct-increment-code-gen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

// RUN: %clang_cc1 -fcheckedc-extension %s -ast-dump | FileCheck %s --check-prefix=CHECK-AST
// RUN: %clang_cc1 -fcheckedc-extension %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR
// RUN: %clang_cc1 -fcheckedc-extension -fno-checkedc-null-ptr-arith %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR

// In the following generated IR, we do not verify the alignment of any loads/stores
// ie, the IR checked by line 37 might read "%1 = load i32*, i32** @gp1, align 4"
Expand Down
2 changes: 1 addition & 1 deletion test/CheckedC/dynamic-checks/struct-read-code-gen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

// RUN: %clang_cc1 -fcheckedc-extension %s -ast-dump | FileCheck %s --check-prefix=CHECK-AST
// RUN: %clang_cc1 -fcheckedc-extension %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR
// RUN: %clang_cc1 -fcheckedc-extension -fno-checkedc-null-ptr-arith %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR

// In the following generated IR, we do not verify the alignment of any loads/stores
// ie, the IR checked by line 37 might read "%1 = load i32*, i32** @gp1, align 4"
Expand Down
2 changes: 1 addition & 1 deletion test/CheckedC/dynamic-checks/struct-write-code-gen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

// RUN: %clang_cc1 -fcheckedc-extension %s -ast-dump | FileCheck %s --check-prefix=CHECK-AST
// RUN: %clang_cc1 -fcheckedc-extension %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR
// RUN: %clang_cc1 -fcheckedc-extension -fno-checkedc-null-ptr-arith %s -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-IR

// In the following generated IR, we do not verify the alignment of any loads/stores
// ie, the IR checked by line 37 might read "%1 = load i32*, i32** @gp1, align 4"
Expand Down