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

Jump to bottom

Short-term solution for issue 1009. #1014

Merged
merged 2 commits into from
Mar 29, 2021
Merged

Short-term solution for issue 1009. #1014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f7dec3
Short-term solution for issue 1009.
sulekhark Mar 25, 2021
5c65781
Corrected a slip.
sulekhark Mar 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion clang/include/clang/Sema/Sema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5667,7 +5667,8 @@ class Sema final {
BoundsExpr *MakeMemberBoundsConcrete(Expr *MemberBase, bool IsArrow,
BoundsExpr *Bounds);
BoundsExpr *ConcretizeFromFunctionTypeWithArgs(BoundsExpr *Bounds, ArrayRef<Expr *> Args,
NonModifyingContext ErrorKind);
NonModifyingContext ErrorKind,
NonModifyingMessage Message);

/// ConvertToFullyCheckedType: convert an expression E to a fully checked type. This
/// is used to retype declrefs and member exprs in checked scopes with bounds-safe
Expand Down
38 changes: 27 additions & 11 deletions clang/lib/Sema/SemaBounds.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,14 +259,17 @@ namespace {
const ArrayRef<Expr *> Arguments;
llvm::SmallBitVector VisitedArgs;
Sema::NonModifyingContext ErrorKind;
Sema::NonModifyingMessage Message;
bool ModifyingArg;
public:
CheckForModifyingArgs(Sema &SemaRef, ArrayRef<Expr *> Args,
Sema::NonModifyingContext ErrorKind) :
Sema::NonModifyingContext ErrorKind,
Sema::NonModifyingMessage Message) :
SemaRef(SemaRef),
Arguments(Args),
VisitedArgs(Args.size()),
ErrorKind(ErrorKind),
Message(Message),
ModifyingArg(false) {}

bool FoundModifyingArg() {
Expand All @@ -277,8 +280,7 @@ namespace {
unsigned index = E->getIndex();
if (index < Arguments.size() && !VisitedArgs[index]) {
VisitedArgs.set(index);
if (!SemaRef.CheckIsNonModifying(Arguments[index], ErrorKind,
Sema::NonModifyingMessage::NMM_Error)) {
if (!SemaRef.CheckIsNonModifying(Arguments[index], ErrorKind, Message)) {
ModifyingArg = true;
}
}
Expand Down Expand Up @@ -313,11 +315,11 @@ namespace {

BoundsExpr *Sema::ConcretizeFromFunctionTypeWithArgs(
BoundsExpr *Bounds, ArrayRef<Expr *> Args,
NonModifyingContext ErrorKind) {
NonModifyingContext ErrorKind, NonModifyingMessage Message) {
if (!Bounds || Bounds->isInvalid())
return Bounds;

auto CheckArgs = CheckForModifyingArgs(*this, Args, ErrorKind);
auto CheckArgs = CheckForModifyingArgs(*this, Args, ErrorKind, Message);
CheckArgs.TraverseStmt(Bounds);
if (CheckArgs.FoundModifyingArg())
return nullptr;
Expand Down Expand Up @@ -3279,7 +3281,7 @@ namespace {
BoundsExpr *CheckCallExpr(CallExpr *E, CheckedScopeSpecifier CSS,
CheckingState &State,
CHKCBindTemporaryExpr *Binding = nullptr) {
BoundsExpr *ResultBounds = CallExprBounds(E, Binding);
BoundsExpr *ResultBounds = CallExprBounds(E, Binding, CSS);

QualType CalleeType = E->getCallee()->getType();
// Extract the pointee type. The caller type could be a regular pointer
Expand Down Expand Up @@ -3367,7 +3369,8 @@ namespace {
S.ConcretizeFromFunctionTypeWithArgs(
const_cast<BoundsExpr *>(ParamBounds),
ArgExprs,
Sema::NonModifyingContext::NMC_Function_Parameter);
Sema::NonModifyingContext::NMC_Function_Parameter,
Sema::NonModifyingMessage::NMM_Error);

if (!SubstParamBounds)
continue;
Expand Down Expand Up @@ -5936,7 +5939,8 @@ namespace {
// If ResultName is non-null, it is a temporary variable where the result
// of the call expression is stored immediately upon return from the call.
BoundsExpr *CallExprBounds(const CallExpr *CE,
CHKCBindTemporaryExpr *ResultName) {
CHKCBindTemporaryExpr *ResultName,
CheckedScopeSpecifier CSS) {
BoundsExpr *ReturnBounds = nullptr;
if (CE->getType()->isCheckedPointerPtrType()) {
if (CE->getType()->isVoidPointerType())
Expand All @@ -5963,7 +5967,7 @@ namespace {
BoundsExpr *FunBounds = FunReturnAnnots.getBoundsExpr();
InteropTypeExpr *IType =FunReturnAnnots.getInteropTypeExpr();
// If there is no return bounds and there is an interop type
// annotation, use the bounds impied by the interop type
// annotation, use the bounds implied by the interop type
// annotation.
if (!FunBounds && IType)
FunBounds = CreateTypeBasedBounds(nullptr, IType->getType(),
Expand All @@ -5978,10 +5982,22 @@ namespace {
CE->getNumArgs());

// Concretize Call Bounds with argument expressions.
// We can only do this if the argument expressions are non-modifying
// We can only do this if the argument expressions are non-modifying.
// For argument expressions that are modifying, we issue an error
// message only in checked scope because the argument expressions may
// be re-evaluated during bounds validation.
// TODO: The long-term solution is to introduce temporaries for
// modifying argument expressions whose corresponding formals are used
// in return or parameter bounds expressions.
// Equality between the value computed by an argument expression and its
// associated temporary would also need to be recorded.
Sema::NonModifyingMessage Message =
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems incorrect. It can be written as:

Sema::NonModifyingMessage Message =
          (CSS == CheckedScopeSpecifier::CSS_Unchecked) ?
          Sema::NonModifyingMessage::NMM_None :
          Sema::NonModifyingMessage::NMM_Error;

(CSS == CheckedScopeSpecifier::CSS_Unchecked) ?
Sema::NonModifyingMessage::NMM_None :
Sema::NonModifyingMessage::NMM_Error;
ReturnBounds =
S.ConcretizeFromFunctionTypeWithArgs(FunBounds, ArgExprs,
Sema::NonModifyingContext::NMC_Function_Return);
Sema::NonModifyingContext::NMC_Function_Return, Message);
// If concretization failed, this means we tried to substitute with
// a non-modifying expression, which is not allowed by the
// specification.
Expand Down
6 changes: 0 additions & 6 deletions clang/test/CheckedC/inferred-bounds/calls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ void f10(_Array_ptr<int> a, _Array_ptr<int> b) {

void f11(_Array_ptr<int> a, _Array_ptr<int> b) {
_Array_ptr<int> c : bounds(a, a+1) = f_bounds(b++, a); // \
// expected-error {{expression not allowed in argument for parameter used in function return bounds}} \
// expected-error {{inferred bounds for 'c' are unknown after initialization}} \
// expected-note {{(expanded) declared bounds are 'bounds(a, a + 1)'}}
}
Expand Down Expand Up @@ -230,7 +229,6 @@ void f12(int i, int j) {

void f13(int i, int j) {
_Array_ptr<int> b : count(i) = f_count(j++, i); // \
// expected-error {{expression not allowed in argument for parameter used in function return bounds}} \
// expected-error {{inferred bounds for 'b' are unknown after initialization}} \
// expected-note {{(expanded) declared bounds are 'bounds(b, b + i)'}}
}
Expand Down Expand Up @@ -285,7 +283,6 @@ void f14(int i, int j) {

void f15(int i, int j) {
_Array_ptr<int> b : byte_count(i) = f_byte(j++, i); // \
// expected-error {{expression not allowed in argument for parameter used in function return bounds}} \
// expected-error {{inferred bounds for 'b' are unknown after initialization}} \
// expected-note {{(expanded) declared bounds are 'bounds((_Array_ptr<char>)b, (_Array_ptr<char>)b + i)'}}
}
Expand Down Expand Up @@ -349,7 +346,6 @@ void f20(int* a, int* b) {

void f21(int* a, int* b) {
_Array_ptr<int> c : bounds(a, a+1) = f_boundsi(b++, a); // \
// expected-error {{expression not allowed in argument for parameter used in function return bounds}} \
// expected-error {{inferred bounds for 'c' are unknown after initialization}} \
// expected-note {{(expanded) declared bounds are 'bounds(a, a + 1)'}}
}
Expand Down Expand Up @@ -412,7 +408,6 @@ void f22(int i, int j) {

void f23(int i, int j) {
_Array_ptr<int> b : count(i) = f_counti(j++, i); // \
// expected-error {{expression not allowed in argument for parameter used in function return bounds}} \
// expected-error {{inferred bounds for 'b' are unknown after initialization}} \
// expected-note {{(expanded) declared bounds are 'bounds(b, b + i)'}}
}
Expand Down Expand Up @@ -469,7 +464,6 @@ void f24(int i, int j) {

void f25(int i, int j) {
_Array_ptr<int> b : byte_count(i) = f_bytei(j++, i); // \
// expected-error {{expression not allowed in argument for parameter used in function return bounds}} \
// expected-error {{inferred bounds for 'b' are unknown after initialization}} \
// expected-note {{(expanded) declared bounds are 'bounds((_Array_ptr<char>)b, (_Array_ptr<char>)b + i)'}}
}
Expand Down
84 changes: 84 additions & 0 deletions clang/test/CheckedC/static-checking/bounds-side-effects.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,3 +227,87 @@ void f105(int len, _Array_ptr<int> p : count(len), int i) {
len += 1, p = alloc(len * sizeof(int));
}

extern int memcmp_test(const void *src1 : byte_count(n),
const void *src2 :byte_count(n), unsigned int n);
extern void *memchr_test(const void *s : byte_count(n), int c, unsigned int n) :
bounds(s, (_Array_ptr<char>) s + n);
extern _Itype_for_any(T) void *malloc_test(unsigned int size) :
itype(_Array_ptr<T>) byte_count(size);

// Checked pointers in checked scope
void f106()
_Checked
{
int len = 4;
_Nt_array_ptr<char> p : count(5) = "hello";
int i = memcmp_test(p, "hello", ++len); // expected-error {{increment expression not allowed in argument for parameter used in function parameter bounds expression}} \
// expected-error {{increment expression not allowed in argument for parameter used in function parameter bounds expression}}
}

// Checked pointers in unchecked scope
void f106_u1()
{
int len = 4;
_Nt_array_ptr<char> p : count(5) = "hello";
int i = memcmp_test(p, "hello", ++len); // expected-error {{increment expression not allowed in argument for parameter used in function parameter bounds expression}}
}

// Unchecked pointers in unchecked scope
void f106_u2()
{
int len = 4;
char *p = "hello";
int i = memcmp_test(p, "hello", ++len);
}

// Checked pointers in checked scope
void f107()
_Checked
{
int len = 4;
int p _Checked [5] = {'h', 'e', 'l', 'l', 'o'};
_Array_ptr<int> pos = memchr_test(p, 'l', ++len); // expected-error {{increment expression not allowed in argument for parameter used in function return bounds expression}} \
// expected-error {{increment expression not allowed in argument for parameter used in function parameter bounds expression}}
}

// Checked pointers in unchecked scope
void f107_u1()
{
int len = 4;
int p _Checked [5] = {'h', 'e', 'l', 'l', 'o'};
// Ideally, there should be an error for modifying expressions
// used in the return bounds expression also.
_Array_ptr<int> pos = memchr_test(p, 'l', ++len); // expected-error {{increment expression not allowed in argument for parameter used in function parameter bounds expression}}
}

// Unchecked pointers in unchecked scope
void f107_u2()
{
int len = 4;
int p[5] = {'h', 'e', 'l', 'l', 'o'};
int *pos = memchr_test(p, 'l', ++len);
}

// Checked pointers in checked scope
void f108()
_Checked
{
int len = 5;
_Array_ptr<char> p = malloc_test<char>(++len); // expected-error {{increment expression not allowed in argument for parameter used in function return bounds expression}}
}

// Checked pointers in unchecked scope
void f108_u1()
{
int len = 5;
// Ideally, there should be an error for modifying expressions
// used in the return bounds expression also.
_Array_ptr<char> p = malloc_test<char>(++len);
}

// Unchecked pointers in unchecked scope
void f108_u2()
{
int len = 5;
char *p = malloc_test(++len);
}