Skip to content

Commit 3be112e

Browse files
author
MalavikaSamak
committed
[Wunsafe-buffer-usage] Fix false positives in handling string literals.
Do not warn when a string literal is indexed and the idex value is within the bounds of the length of the string. (rdar://139106996)
1 parent 5b697ef commit 3be112e

File tree

2 files changed

+26
-9
lines changed

2 files changed

+26
-9
lines changed

clang/lib/Analysis/UnsafeBufferUsage.cpp

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -436,21 +436,31 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
436436

437437
const auto *BaseDRE =
438438
dyn_cast<DeclRefExpr>(Node.getBase()->IgnoreParenImpCasts());
439-
if (!BaseDRE)
440-
return false;
441-
if (!BaseDRE->getDecl())
442-
return false;
443-
const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
444-
BaseDRE->getDecl()->getType());
445-
if (!CATy)
439+
const auto *SLiteral = dyn_cast<StringLiteral>(Node.getBase()->IgnoreParenImpCasts());
440+
uint64_t size;
441+
442+
if (!BaseDRE && !SLiteral)
446443
return false;
447444

445+
if(BaseDRE) {
446+
if (!BaseDRE->getDecl())
447+
return false;
448+
const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
449+
BaseDRE->getDecl()->getType());
450+
if (!CATy) {
451+
return false;
452+
}
453+
size = CATy->getLimitedSize();
454+
} else if(SLiteral) {
455+
size = SLiteral->getLength();
456+
}
457+
448458
if (const auto *IdxLit = dyn_cast<IntegerLiteral>(Node.getIdx())) {
449459
const APInt ArrIdx = IdxLit->getValue();
450460
// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's a
451461
// bug
452462
if (ArrIdx.isNonNegative() &&
453-
ArrIdx.getLimitedValue() < CATy->getLimitedSize())
463+
ArrIdx.getLimitedValue() < size)
454464
return true;
455465
}
456466

@@ -1142,7 +1152,7 @@ class ArraySubscriptGadget : public WarningGadget {
11421152
// clang-format off
11431153
return stmt(arraySubscriptExpr(
11441154
hasBase(ignoringParenImpCasts(
1145-
anyOf(hasPointerType(), hasArrayType()))),
1155+
anyOf(hasPointerType(), hasArrayType(), stringLiteral()))),
11461156
unless(anyOf(
11471157
isSafeArraySubscript(),
11481158
hasIndex(

clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,10 @@ void constant_idx_unsafe(unsigned idx) {
3838
// expected-note@-1{{change type of 'buffer' to 'std::array' to label it for hardening}}
3939
buffer[10] = 0; // expected-note{{used in buffer access here}}
4040
}
41+
42+
void constant_id_string() {
43+
char safe_char = "abc"[1]; // no-warning
44+
char abcd[5] = "abc";
45+
abcd[2]; // no-warning
46+
safe_char = "abc"[3]; //expected-warning{{unsafe buffer access}}
47+
}

0 commit comments

Comments
 (0)