Skip to content

Commit 81af812

Browse files
author
MalavikaSamak
committed
[Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0])
Do not warn when a constant sized array is indexed with an expression that contains bitwise and operation involving constants and it always results in a bound safe access. (rdar://136684050)
1 parent 2ff4c25 commit 81af812

File tree

2 files changed

+63
-1
lines changed

2 files changed

+63
-1
lines changed

clang/lib/Analysis/UnsafeBufferUsage.cpp

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,48 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
427427
// - e. g. "Try harder to find a NamedDecl to point at in the note."
428428
// already duplicated
429429
// - call both from Sema and from here
430+
std::function<llvm::APInt(const Expr *exp, unsigned int limit)>
431+
SafeMaskedAccess;
432+
unsigned int RecLimit = 5;
433+
434+
SafeMaskedAccess = [&](const Expr *exp, unsigned int RecLimit) -> llvm::APInt {
435+
llvm::APInt Max = llvm::APInt::getMaxValue(Finder->getASTContext().getIntWidth(exp->getType()));
436+
Max.setAllBits();
437+
438+
if (RecLimit == 0)
439+
return Max;
440+
441+
//RecLimit--;
442+
443+
if (const auto *IntLit = dyn_cast<IntegerLiteral>(exp)) {
444+
const APInt ArrIdx = IntLit->getValue();
445+
return ArrIdx;
446+
}
447+
448+
if (const auto *BinEx = dyn_cast<BinaryOperator>(exp)) {
449+
llvm::APInt LHS = SafeMaskedAccess(BinEx->getLHS()->IgnoreParenCasts(), RecLimit);
450+
llvm::APInt RHS = SafeMaskedAccess(BinEx->getRHS()->IgnoreParenCasts(), RecLimit);
451+
452+
switch(BinEx->getOpcode()) {
453+
case BO_And:
454+
LHS = LHS & RHS.getLimitedValue();
455+
return LHS;
456+
case BO_Or:
457+
LHS = LHS | RHS.getLimitedValue();
458+
return LHS;
459+
case BO_Shl:
460+
LHS = LHS << RHS.getLimitedValue();
461+
return LHS;
462+
case BO_Xor:
463+
LHS = LHS ^ RHS.getLimitedValue();
464+
return LHS;
465+
default:
466+
return Max;
467+
}
468+
}
469+
470+
return Max;
471+
};
430472

431473
const auto *BaseDRE =
432474
dyn_cast<DeclRefExpr>(Node.getBase()->IgnoreParenImpCasts());
@@ -446,6 +488,10 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
446488
if (ArrIdx.isNonNegative() &&
447489
ArrIdx.getLimitedValue() < CATy->getLimitedSize())
448490
return true;
491+
} else if (const auto *BinEx = dyn_cast<BinaryOperator>(Node.getIdx())) {
492+
APInt result = SafeMaskedAccess(Node.getIdx(), RecLimit);
493+
if (result.isNonNegative() && result.getLimitedValue() < CATy->getLimitedSize())
494+
return true;
449495
}
450496

451497
return false;
@@ -1152,7 +1198,7 @@ class ArraySubscriptGadget : public WarningGadget {
11521198
Handler.handleUnsafeOperation(ASE, IsRelatedToDecl, Ctx);
11531199
}
11541200
SourceLocation getSourceLoc() const override { return ASE->getBeginLoc(); }
1155-
1201+
11561202
DeclUseList getClaimedVarUseSites() const override {
11571203
if (const auto *DRE =
11581204
dyn_cast<DeclRefExpr>(ASE->getBase()->IgnoreParenImpCasts())) {

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,22 @@ void constant_idx_safe0(unsigned idx) {
3333
buffer[0] = 0;
3434
}
3535

36+
int array[10]; // expected-warning{{'array' is an unsafe buffer that does not perform bounds checks}}
37+
38+
void masked_idx(unsigned long long idx) {
39+
array[idx & 5] = 10; // no warning
40+
array[idx & 11 & 5] = 3; // no warning
41+
array[idx & 11] = 20; // expected-note{{used in buffer access here}}
42+
}
43+
44+
int array2[5];
45+
46+
void mased_idx_false(unsigned long long idx) {
47+
array2[6 & 5]; // no warning
48+
array2[6 & idx & (idx + 1) & 5]; // no warning
49+
array2[6 & idx & 5 & (idx + 1) | 4];
50+
}
51+
3652
void constant_idx_unsafe(unsigned idx) {
3753
int buffer[10]; // expected-warning{{'buffer' is an unsafe buffer that does not perform bounds checks}}
3854
// expected-note@-1{{change type of 'buffer' to 'std::array' to label it for hardening}}

0 commit comments

Comments
 (0)