Skip to content

[-Wunsafe-buffer-usage] Relax passing to __counted_by_or_null() #11170

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.

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

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions clang/lib/Analysis/UnsafeBufferUsage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,9 @@ static const DeclRefExpr *tryGetAddressofDRE(const Expr *E) {

// Checks if the argument passed to count-attributed pointer is one of the
// following forms:
// 0. `NULL/nullptr`, if the argument to dependent count/size is `0`.
// 0. `NULL/nullptr`, if the argument to dependent count/size is `0`, or
// anything if the type of the pointer is
// __counted_by_or_null()/__sized_by_or_null().
// 1. `&var`, if `var` is a variable identifier and (1.a.) the dependent count
// is `1`; or (1.b.) the counted_by expression is constant `1`
// 2. `&var`, if `var` is a variable identifier and the dependent size is
Expand Down Expand Up @@ -947,6 +949,8 @@ static bool isCountAttributedPointerArgumentSafeImpl(

// check form 0:
if (PtrArgNoImp->getType()->isNullPtrType()) {
if (isOrNull)
return true;
if (CountArg)
return hasIntegeralConstant(CountArg, 0, Context);
// When there is no argument representing the count/size, it is safe iff
Expand Down Expand Up @@ -1017,7 +1021,7 @@ static bool isCountAttributedPointerArgumentSafeImpl(
bool areBoundsAttributesCompatible =
(ArgCAT->isCountInBytes() == isSizedBy || (ArgInBytes && ParamInBytes));

if (ArgCAT->isOrNull() == isOrNull && areBoundsAttributesCompatible)
if ((isOrNull || !ArgCAT->isOrNull()) && areBoundsAttributesCompatible)
ActualCount = ArgCAT->getCountExpr();
} else {
// Form 6-7.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ void cb_cchar(const char *__counted_by(len) s, size_t len);
// expected-note@+1 3{{consider using 'std::span' and passing '.first(...).data()' to the parameter 's'}}
void cb_cchar_42(const char *__counted_by(42) s);

// expected-note@+1 19{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'count' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
// expected-note@+1 22{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'count' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
void cb_int(int *__counted_by(count) p, size_t count);

// expected-note@+1 34{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'count' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
Expand All @@ -81,6 +81,9 @@ void cb_cint_42(const int *__counted_by(42) p);
// expected-note@+1 6{{consider using 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
void cb_cint_multi(const int *__counted_by((a + b) * (c - d)) p, int a, int b, int c, int d);

// expected-note@+1 3{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'size' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
void sb_void(void *__sized_by(size) p, size_t size);

// expected-note@+1 13{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'size' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
void sb_cvoid(const void *__sized_by(size) p, size_t size);

Expand All @@ -98,6 +101,10 @@ void sb_cint_42(const int *__sized_by(42) p);

void cb_cint_array(const int (* __counted_by(size) p)[10], size_t size);

void cbn_int(int *__counted_by_or_null(count) p, size_t count);

void sbn_void(void *__sized_by_or_null(size) p, size_t size);

} // extern "C"

// Check allowed classes and method.
Expand Down Expand Up @@ -389,9 +396,22 @@ void from_cb_int_multi(int *__counted_by((a + b) * (c - d)) p, int a, int b, int
cb_int(p, 42); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
}

void nullptr_as_arg(void * __sized_by(size) p, unsigned size) { //expected-note{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'size' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
nullptr_as_arg(nullptr, 0);
nullptr_as_arg(nullptr, size); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
void nullptr_as_arg(size_t n) {
cb_int(nullptr, 0);
cb_int(nullptr, 42); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
cb_int(nullptr, n); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}

sb_void(nullptr, 0);
sb_void(nullptr, 42); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
sb_void(nullptr, n); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}

cbn_int(nullptr, 0);
cbn_int(nullptr, 42);
cbn_int(nullptr, n);

sbn_void(nullptr, 0);
sbn_void(nullptr, 42);
sbn_void(nullptr, n);
}

void single_variable() {
Expand Down Expand Up @@ -665,6 +685,30 @@ static void previous_infinite_loop3(int * __counted_by(n + n * m) p, size_t n,
previous_infinite_loop3(p, n, q, r, m, o + 1); // expected-warning 2{{unsafe assignment to function parameter of count-attributed type}}
}

// Check nullable variants.

void nonnull_tofrom_nullable(size_t n,
int *__counted_by(n) cb,
int *__counted_by_or_null(n) cbn,
void *__sized_by(n) sb,
void *__sized_by_or_null(n) sbn) {
cb_int(cb, n);
// __counted_by_or_null(count) can have arbitrary count if the pointer is
// null. Since __counted_by() does not allow this, we diagnose
// __counted_by_or_null() -> __counted_by().

Choose a reason for hiding this comment

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

I meant as in emitting a note diagnostic

cb_int(cbn, n); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}

cbn_int(cb, n);
// __counted_by() -> __counted_by_or_null() is fine.
cbn_int(cbn, n);

sb_void(sb, n);
sb_void(sbn, n); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}

sbn_void(sb, n);
sbn_void(sbn, n);
}

Choose a reason for hiding this comment

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

Can we add more tests:

  • unsafe cases of cbn_int and sun_void;
  • passing __counted/sized_by_or_null pointers to std::span
  • passing __counted/sized_by_or_null pointers to the 1st and 2nd parameters of snprintf functions


// Check default args.

void cb_def_arg_null_0(int *__counted_by(count) p = nullptr, size_t count = 0);
Expand Down