-
Notifications
You must be signed in to change notification settings - Fork 14.5k
WebKit checkers: recgonize @YES / @NO as safe constants #148721
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
Conversation
@llvm/pr-subscribers-clang Author: Ryosuke Niwa (rniwa) ChangesFull diff: https://github.com/llvm/llvm-project/pull/148721.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index cfcc47c5e71c8..478bd85177143 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -177,7 +177,10 @@ bool tryToFindPtrOrigin(
E = unaryOp->getSubExpr();
continue;
}
-
+ if (auto *BoxedExpr = dyn_cast<ObjCBoxedExpr>(E)) {
+ E = BoxedExpr->getSubExpr();
+ continue;
+ }
break;
}
// Some other expression.
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
index c33d53b047c2e..c69113c48806d 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
@@ -329,13 +329,17 @@ void foo() {
}
}
+#define YES 1
+
namespace call_with_cf_constant {
void bar(const NSArray *);
void baz(const NSDictionary *);
+ void boo(NSNumber *);
void foo() {
CFArrayCreateMutable(kCFAllocatorDefault, 10);
bar(@[@"hello"]);
baz(@{@"hello": @3});
+ boo(@YES);
}
}
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Ryosuke Niwa (rniwa) ChangesFull diff: https://github.com/llvm/llvm-project/pull/148721.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index cfcc47c5e71c8..478bd85177143 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -177,7 +177,10 @@ bool tryToFindPtrOrigin(
E = unaryOp->getSubExpr();
continue;
}
-
+ if (auto *BoxedExpr = dyn_cast<ObjCBoxedExpr>(E)) {
+ E = BoxedExpr->getSubExpr();
+ continue;
+ }
break;
}
// Some other expression.
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
index c33d53b047c2e..c69113c48806d 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
@@ -329,13 +329,17 @@ void foo() {
}
}
+#define YES 1
+
namespace call_with_cf_constant {
void bar(const NSArray *);
void baz(const NSDictionary *);
+ void boo(NSNumber *);
void foo() {
CFArrayCreateMutable(kCFAllocatorDefault, 10);
bar(@[@"hello"]);
baz(@{@"hello": @3});
+ boo(@YES);
}
}
|
@@ -329,13 +329,17 @@ void foo() { | |||
} | |||
} | |||
|
|||
#define YES 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it just @YES/@no that are safe constants? What's the expected behavior for say #define RANDOM 3
and boo(@ RANDOM);
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are also considered safe. Basically @ any number should be treated as safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, got it. Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thanks for the review! |
FYI I think it's good practice to not tag people in commit title/summaries unless it's strictly desired. |
oh, I didn't even realize |
No description provided.