Skip to content

Correctly determine bounds for predefined literals #673

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

Merged
merged 8 commits into from
Sep 14, 2019
Merged

Correctly determine bounds for predefined literals #673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7d165ad
Correctly determine bounds for predefined literals
Aug 22, 2019
a268dc8
Make some vars const
Aug 23, 2019
421db65
Add simple test case
Aug 24, 2019
bd4b6e8
Add more tests
Aug 26, 2019
9284855
Bind string literal to temporary expr
Aug 27, 2019
e6ada3e
Capitalize function name
Aug 28, 2019
1330739
Remove extra braces
Aug 28, 2019
7c2ad31
Handled subscripting of predefined literals
Sep 5, 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
7 changes: 4 additions & 3 deletions lib/Sema/Sema.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,11 +518,12 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
E = Materialized.get();
}

// For Checked C, create a temporary for string literal or compound literals
// for use during bounds checking.
// For Checked C, create a temporary for string literal, compound literals or
// predefined literals for use during bounds checking.
if (Kind == CK_ArrayToPointerDecay && getLangOpts().CheckedC) {
Expr *S = E->IgnoreParens();
if (isa<StringLiteral>(S) || isa<CompoundLiteralExpr>(S))
if (isa<StringLiteral>(S) || isa<CompoundLiteralExpr>(S) ||
isa<PredefinedExpr>(S))
E = new (Context) CHKCBindTemporaryExpr(E);
}

Expand Down
63 changes: 41 additions & 22 deletions lib/Sema/SemaBounds.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,36 @@ namespace {
return ExpandToRange(Base, BE);
}

// Infer bounds for string literals.
BoundsExpr *InferBoundsForStringLiteral(Expr *E, StringLiteral *SL,
CHKCBindTemporaryExpr *Binding) {
// Use the number of characters in the string (excluding the null
// terminator) to calcaulte size. Don't use the array type of the
// literal. In unchecked scopes, the array type is unchecked and its
// size includes the null terminator. It converts to an ArrayPtr that
// could be used to overwrite the null terminator. We need to prevent
// this because literal strings may be shared and writeable, depending on
// the C implementation.
auto *Size = CreateIntegerLiteral(llvm::APInt(64, SL->getLength()));
auto *CBE =
new (Context) CountBoundsExpr(BoundsExpr::Kind::ElementCount,
Size, SourceLocation(),
SourceLocation());

auto PtrType = Context.getDecayedType(E->getType());

// For a string literal expression, we always bind the result of the
// expression to a temporary. We then use this temporary in the bounds
// expression for the string literal expression. Otherwise, a runtime
// bounds check based on accessing the predefined expression could be
// incorrect: the base value could be different for the lower and upper
// bounds.
auto *ArrLValue = CreateTemporaryUse(Binding);
auto *Base = CreateImplicitCast(PtrType,
CastKind::CK_ArrayToPointerDecay,
ArrLValue);
return ExpandToRange(Base, CBE);
}

// Infer bounds for an lvalue. The bounds determine whether
// it is valid to access memory using the lvalue. The bounds
Expand Down Expand Up @@ -879,6 +909,7 @@ namespace {
case Expr::CHKCBindTemporaryExprClass: {
CHKCBindTemporaryExpr *Binding = cast<CHKCBindTemporaryExpr>(E);
Expr *SE = Binding->getSubExpr()->IgnoreParens();

if (isa<CompoundLiteralExpr>(SE)) {
BoundsExpr *BE = CreateBoundsForArrayType(E->getType());
QualType PtrType = Context.getDecayedType(E->getType());
Expand All @@ -887,27 +918,15 @@ namespace {
CastKind::CK_ArrayToPointerDecay,
ArrLValue);
return ExpandToRange(Base, BE);
} else if (StringLiteral *SL = dyn_cast<StringLiteral>(SE)) {
// Use the number of characters in the string (excluding the
// null terminator) to calcaulte size. Don't use the
// array type of the literal. In unchecked scopes, the array type is
// unchecked and its size includes the null terminator. It converts
// to an ArrayPtr that could be used to overwrite the null terminator.
// We need to prevent this because literal strings may be shared and
// writeable, depending on the C implementation.
IntegerLiteral *Size = CreateIntegerLiteral(llvm::APInt(64, SL->getLength()));
CountBoundsExpr *CBE =
new (Context) CountBoundsExpr(BoundsExpr::Kind::ElementCount,
Size, SourceLocation(),
SourceLocation());
QualType PtrType = Context.getDecayedType(E->getType());
Expr *ArrLValue = CreateTemporaryUse(Binding);
Expr *Base = CreateImplicitCast(PtrType,
CastKind::CK_ArrayToPointerDecay,
ArrLValue);
return ExpandToRange(Base, CBE);
} else
return CreateBoundsAlwaysUnknown();
}

if (auto *SL = dyn_cast<StringLiteral>(SE))
return InferBoundsForStringLiteral(E, SL, Binding);

if (auto *PE = dyn_cast<PredefinedExpr>(SE)) {
auto *SL = PE->getFunctionName();
return InferBoundsForStringLiteral(E, SL, Binding);
}
}
default:
return CreateBoundsAlwaysUnknown();
Expand Down Expand Up @@ -3647,4 +3666,4 @@ void Sema::WarnDynamicCheckAlwaysFails(const Expr *Condition) {
<< Condition->getSourceRange();
}
}
}
}
10 changes: 7 additions & 3 deletions lib/Sema/SemaExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3264,25 +3264,29 @@ ExprResult Sema::BuildPredefinedExpr(SourceLocation Loc,
unsigned Length = Str.length();

llvm::APInt LengthI(32, Length + 1);
// Get an array type for the string, according to C99 6.4.5.
CheckedArrayKind ArrayKind = IsCheckedScope() ?
CheckedArrayKind::NtChecked : CheckedArrayKind::Unchecked;

if (IK == PredefinedExpr::LFunction || IK == PredefinedExpr::LFuncSig) {
ResTy =
Context.adjustStringLiteralBaseType(Context.WideCharTy.withConst());
SmallString<32> RawChars;
ConvertUTF8ToWideString(Context.getTypeSizeInChars(ResTy).getQuantity(),
Str, RawChars);

ResTy = Context.getConstantArrayType(ResTy, LengthI, ArrayType::Normal,
/*IndexTypeQuals*/ 0);
/*IndexTypeQuals*/ 0, ArrayKind);
SL = StringLiteral::Create(Context, RawChars, StringLiteral::Wide,
/*Pascal*/ false, ResTy, Loc);
} else {
ResTy = Context.adjustStringLiteralBaseType(Context.CharTy.withConst());
ResTy = Context.getConstantArrayType(ResTy, LengthI, ArrayType::Normal,
/*IndexTypeQuals*/ 0);
/*IndexTypeQuals*/ 0, ArrayKind);
SL = StringLiteral::Create(Context, Str, StringLiteral::Ascii,
/*Pascal*/ false, ResTy, Loc);
}
}

return PredefinedExpr::Create(Context, Loc, ResTy, IK, SL);
}

Expand Down
93 changes: 93 additions & 0 deletions test/CheckedC/inferred-bounds/predefined-literals.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// Tests of inferred bounds for predefined literals like __func__, etc.
//
// RUN: %clang_cc1 -fdump-inferred-bounds %s | FileCheck %s
// expected-no-diagnostics

void f1() {
// CHECK: ImplicitCastExpr {{0x[0-9a-f]+}} '_Ptr<const char>' <BitCast>
// CHECK-NEXT: |-Inferred SubExpr Bounds
// CHECK-NEXT: | `-RangeBoundsExpr {{0x[0-9a-f]+}} 'NULL TYPE'
// CHECK-NEXT: | |-ImplicitCastExpr {{0x[0-9a-f]+}} 'const char *':'const char *' <ArrayToPointerDecay>
// CHECK-NEXT: | | `-BoundsValueExpr {{0x[0-9a-f]+}} 'const char [3]' lvalue _BoundTemporary {{0x[0-9a-f]+}}
// CHECK-NEXT: | `-BinaryOperator {{0x[0-9a-f]+}} 'const char *':'const char *' '+'
// CHECK-NEXT: | |-ImplicitCastExpr {{0x[0-9a-f]+}} 'const char *':'const char *' <ArrayToPointerDecay>
// CHECK-NEXT: | | `-BoundsValueExpr {{0x[0-9a-f]+}} 'const char [3]' lvalue _BoundTemporary {{0x[0-9a-f]+}}
// CHECK-NEXT: | `-IntegerLiteral {{0x[0-9a-f]+}} 'int' 2
// CHECK-NEXT: `-ImplicitCastExpr {{0x[0-9a-f]+}} 'const char *' <ArrayToPointerDecay>
// CHECK-NEXT: `-CHKCBindTemporaryExpr {{0x[0-9a-f]+}} 'const char [3]' lvalue
// CHECK-NEXT: `-PredefinedExpr {{0x[0-9a-f]+}} 'const char [3]' lvalue __func__
// CHECK-NEXT: `-StringLiteral {{0x[0-9a-f]+}} 'const char [3]' lvalue "f1"
_Ptr<const char> s1 = __func__;
}

void f2(_Ptr<const char> s2);
void f3() {
// CHECK: ImplicitCastExpr {{0x[0-9a-f]+}} '_Ptr<const char>' <BitCast>
// CHECK-NEXT: |-Inferred SubExpr Bounds
// CHECK-NEXT: | `-RangeBoundsExpr {{0x[0-9a-f]+}} 'NULL TYPE'
// CHECK-NEXT: | |-ImplicitCastExpr {{0x[0-9a-f]+}} 'const char *':'const char *' <ArrayToPointerDecay>
// CHECK-NEXT: | | `-BoundsValueExpr {{0x[0-9a-f]+}} 'const char [3]' lvalue _BoundTemporary {{0x[0-9a-f]+}}
// CHECK-NEXT: | `-BinaryOperator {{0x[0-9a-f]+}} 'const char *':'const char *' '+'
// CHECK-NEXT: | |-ImplicitCastExpr {{0x[0-9a-f]+}} 'const char *':'const char *' <ArrayToPointerDecay>
// CHECK-NEXT: | | `-BoundsValueExpr {{0x[0-9a-f]+}} 'const char [3]' lvalue _BoundTemporary {{0x[0-9a-f]+}}
// CHECK-NEXT: | `-IntegerLiteral {{0x[0-9a-f]+}} 'int' 2
// CHECK-NEXT: `-ImplicitCastExpr {{0x[0-9a-f]+}} 'const char *' <ArrayToPointerDecay>
// CHECK-NEXT: `-CHKCBindTemporaryExpr {{0x[0-9a-f]+}} 'const char [3]' lvalue
// CHECK-NEXT: `-PredefinedExpr {{0x[0-9a-f]+}} 'const char [3]' lvalue __func__
// CHECK-NEXT: `-StringLiteral {{0x[0-9a-f]+}} 'const char [3]' lvalue "f3"
f2(__func__);
}

void f4() {
// CHECK: ImplicitCastExpr {{0x[0-9a-f]+}} '_Ptr<const char>' <BitCast>
// CHECK-NEXT: |-Inferred SubExpr Bounds
// CHECK-NEXT: | `-RangeBoundsExpr {{0x[0-9a-f]+}} 'NULL TYPE'
// CHECK-NEXT: | |-ImplicitCastExpr {{0x[0-9a-f]+}} 'const char *':'const char *' <ArrayToPointerDecay>
// CHECK-NEXT: | | `-BoundsValueExpr {{0x[0-9a-f]+}} 'const char [3]' lvalue _BoundTemporary {{0x[0-9a-f]+}}
// CHECK-NEXT: | `-BinaryOperator {{0x[0-9a-f]+}} 'const char *':'const char *' '+'
// CHECK-NEXT: | |-ImplicitCastExpr {{0x[0-9a-f]+}} 'const char *':'const char *' <ArrayToPointerDecay>
// CHECK-NEXT: | | `-BoundsValueExpr {{0x[0-9a-f]+}} 'const char [3]' lvalue _BoundTemporary {{0x[0-9a-f]+}}
// CHECK-NEXT: | `-IntegerLiteral {{0x[0-9a-f]+}} 'int' 2
// CHECK-NEXT: `-ImplicitCastExpr {{0x[0-9a-f]+}} 'const char *' <ArrayToPointerDecay>
// CHECK-NEXT: `-CHKCBindTemporaryExpr {{0x[0-9a-f]+}} 'const char [3]' lvalue
// CHECK-NEXT: `-PredefinedExpr {{0x[0-9a-f]+}} 'const char [3]' lvalue __FUNCTION__
// CHECK-NEXT: `-StringLiteral {{0x[0-9a-f]+}} 'const char [3]' lvalue "f4"
_Ptr<const char> s4 = __FUNCTION__;
}

void f5() {
// CHECK: |-RangeBoundsExpr {{0x[0-9a-f]+}} <col:{{[0-9]+}}, col:{{[0-9]+}}> 'NULL TYPE'
// CHECK-NEXT: | |-ImplicitCastExpr {{0x[0-9a-f]+}} <col:{{[0-9]+}}> '_Array_ptr<const char>' <LValueToRValue>
// CHECK-NEXT: | | `-DeclRefExpr {{0x[0-9a-f]+}} <col:{{[0-9]+}}> '_Array_ptr<const char>' lvalue Var {{0x[0-9a-f]+}} 's5' '_Array_ptr<const char>'
// CHECK-NEXT: | `-BinaryOperator {{0x[0-9a-f]+}} <col:{{[0-9]+}}, col:{{[0-9]+}}> '_Array_ptr<const char>' '+'
// CHECK-NEXT: | |-ImplicitCastExpr {{0x[0-9a-f]+}} <col:{{[0-9]+}}> '_Array_ptr<const char>' <LValueToRValue>
// CHECK-NEXT: | | `-DeclRefExpr {{0x[0-9a-f]+}} <col:{{[0-9]+}}> '_Array_ptr<const char>' lvalue Var {{0x[0-9a-f]+}} 's5' '_Array_ptr<const char>'
// CHECK-NEXT: | `-IntegerLiteral {{0x[0-9a-f]+}} <col:{{[0-9]+}}> 'int' 2
// CHECK-NEXT: `-ImplicitCastExpr {{0x[0-9a-f]+}} <col:{{[0-9]+}}> '_Array_ptr<const char>' <BitCast>
// CHECK-NEXT: `-ImplicitCastExpr {{0x[0-9a-f]+}} <col:{{[0-9]+}}> 'const char *' <ArrayToPointerDecay>
// CHECK-NEXT: `-CHKCBindTemporaryExpr {{0x[0-9a-f]+}} <col:{{[0-9]+}}> 'const char [3]' lvalue
// CHECK-NEXT: `-PredefinedExpr {{0x[0-9a-f]+}} <col:{{[0-9]+}}> 'const char [3]' lvalue __func__
// CHECK-NEXT: `-StringLiteral {{0x[0-9a-f]+}} <col:{{[0-9]+}}> 'const char [3]' lvalue "f5"
_Array_ptr<const char> s5 : bounds(s5, s5+ 2) = __func__;
}

char f6() {
// CHECK: ImplicitCastExpr {{0x[0-9a-f]+}} 'char' <LValueToRValue>
// CHECK-NEXT: `-ArraySubscriptExpr {{0x[0-9a-f]+}} 'const char' lvalue
// CHECK-NEXT: |-Bounds Null-terminated read
// CHECK-NEXT: | `-RangeBoundsExpr {{0x[0-9a-f]+}} 'NULL TYPE'
// CHECK-NEXT: | |-ImplicitCastExpr {{0x[0-9a-f]+}} '_Nt_array_ptr<const char>':'_Nt_array_ptr<const char>' <ArrayToPointerDecay>
// CHECK-NEXT: | | `-BoundsValueExpr {{0x[0-9a-f]+}} 'const char _Nt_checked[3]' lvalue _BoundTemporary {{0x[0-9a-f]+}}
// CHECK-NEXT: | `-BinaryOperator {{0x[0-9a-f]+}} '_Nt_array_ptr<const char>':'_Nt_array_ptr<const char>' '+'
// CHECK-NEXT: | |-ImplicitCastExpr {{0x[0-9a-f]+}} '_Nt_array_ptr<const char>':'_Nt_array_ptr<const char>' <ArrayToPointerDecay>
// CHECK-NEXT: | | `-BoundsValueExpr {{0x[0-9a-f]+}} 'const char _Nt_checked[3]' lvalue _BoundTemporary {{0x[0-9a-f]+}}
// CHECK-NEXT: | `-IntegerLiteral {{0x[0-9a-f]+}} 'int' 2
// CHECK-NEXT: |-ImplicitCastExpr {{0x[0-9a-f]+}} '_Nt_array_ptr<const char>' <ArrayToPointerDecay>
// CHECK-NEXT: | `-CHKCBindTemporaryExpr {{0x[0-9a-f]+}} 'const char _Nt_checked[3]' lvalue
// CHECK-NEXT: | `-PredefinedExpr {{0x[0-9a-f]+}} 'const char _Nt_checked[3]' lvalue __func__
// CHECK-NEXT: | `-StringLiteral {{0x[0-9a-f]+}} 'const char _Nt_checked[3]' lvalue "f6"
// CHECK-NEXT: `-IntegerLiteral {{0x[0-9a-f]+}} 'int' 100
#pragma CHECKED_SCOPE ON
return __func__[100];
#pragma CHECKED_SCOPE OFF
}