[PATCH] D112642: [clang][NFC] Inclusive terms: Replace uses of whitelist in clang/lib/StaticAnalyzer

[Zarko Todorovski via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8659b241ae94: [clang][NFC] Inclusive terms: Replace uses of 
whitelist in… (authored by ZarkoCA).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112642/new/

https://reviews.llvm.org/D112642

Files:
  clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/test/Analysis/vfork.c

Index: clang/test/Analysis/vfork.c
===
--- clang/test/Analysis/vfork.c
+++ clang/test/Analysis/vfork.c
@@ -15,7 +15,7 @@
   case 0:
 // Ensure that modifying pid is ok.
 pid = 1; // no-warning
-// Ensure that calling whitelisted routines is ok.
+// Ensure that calling allowlisted routines is ok.
 switch (y) {
 case 0:
   execl("", "", 0); // no-warning
@@ -65,7 +65,7 @@
   case 0:
 // Ensure that writing pid is ok.
 pid = 1; // no-warning
-// Ensure that calling whitelisted routines is ok.
+// Ensure that calling allowlisted routines is ok.
 execl("", "", 0); // no-warning
 _exit(1); // no-warning
 break;
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -974,7 +974,7 @@
 
 // First handle the globals defined in system headers.
 if (Ctx.getSourceManager().isInSystemHeader(D->getLocation())) {
-  // Whitelist the system globals which often DO GET modified, assume the
+  //  Allow the system globals which often DO GET modified, assume the
   // rest are immutable.
   if (D->getName().contains("errno"))
 sReg = getGlobalsRegion(MemRegion::GlobalSystemSpaceRegionKind);
Index: clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
===
--- clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
+++ clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
@@ -29,7 +29,7 @@
 /// values).
 ///
 /// For more context see Static Analyzer checkers documentation - specifically
-/// webkit.UncountedCallArgsChecker checker. Whitelist of transformations:
+/// webkit.UncountedCallArgsChecker checker. Allowed list of transformations:
 /// - constructors of ref-counted types (including factory methods)
 /// - getters of ref-counted types
 /// - member overloaded operators
Index: clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
@@ -9,7 +9,7 @@
 //  This file defines vfork checker which checks for dangerous uses of vfork.
 //  Vforked process shares memory (including stack) with parent so it's
 //  range of actions is significantly limited: can't write variables,
-//  can't call functions not in whitelist, etc. For more details, see
+//  can't call functions not in the allowed list, etc. For more details, see
 //  http://man7.org/linux/man-pages/man2/vfork.2.html
 //
 //  This checker checks for prohibited constructs in vforked process.
@@ -44,13 +44,14 @@
 class VforkChecker : public Checker> {
   mutable std::unique_ptr BT;
-  mutable llvm::SmallSet VforkWhitelist;
+  mutable llvm::SmallSet VforkAllowlist;
   mutable const IdentifierInfo *II_vfork;
 
   static bool isChildProcess(const ProgramStateRef State);
 
   bool isVforkCall(const Decl *D, CheckerContext ) const;
-  bool isCallWhitelisted(const IdentifierInfo *II, CheckerContext ) const;
+  bool isCallExplicitelyAllowed(const IdentifierInfo *II,
+CheckerContext ) const;
 
   void reportBug(const char *What, CheckerContext ,
  const char *Details = nullptr) const;
@@ -93,9 +94,9 @@
 }
 
 // Returns true iff ok to call function after successful vfork.
-bool VforkChecker::isCallWhitelisted(const IdentifierInfo *II,
- CheckerContext ) const {
-  if (VforkWhitelist.empty()) {
+bool VforkChecker::isCallExplicitelyAllowed(const IdentifierInfo *II,
+CheckerContext ) const {
+  if (VforkAllowlist.empty()) {
 // According to manpage.
 const char *ids[] = {
   "_Exit",
@@ -112,10 +113,10 @@
 
 ASTContext  = C.getASTContext();
 for (const char **id = ids; *id; ++id)
-  VforkWhitelist.insert((*id));
+  VforkAllowlist.insert((*id));
   }
 
-  return VforkWhitelist.count(II);
+  return VforkAllowlist.count(II);
 }
 
 void VforkChecker::reportBug(const char *What, CheckerContext ,
@@ -179,12 +180,13 @@
   C.addTransition(ChildState);
 }
 

[PATCH] D112642: [clang][NFC] Inclusive terms: Replace uses of whitelist in clang/lib/StaticAnalyzer

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112642/new/

https://reviews.llvm.org/D112642

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112642: [clang][NFC] Inclusive terms: Replace uses of whitelist in clang/lib/StaticAnalyzer

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA marked 6 inline comments as done.
ZarkoCA added a comment.

In D112642#3093559 , @aaron.ballman 
wrote:

> Thank you for this! Mostly just bikeshedding on names (allowlist as a verb 
> sounds weird to me), feel free to take or leave the suggestions. LG aside 
> from the formatting nits.

Yes, it didn't seem that right to me either but thought I'd get the ball 
rolling with something for now. Thanks for the suggestions, it looks much 
better to me now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112642/new/

https://reviews.llvm.org/D112642

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112642: [clang][NFC] Inclusive terms: Replace uses of whitelist in clang/lib/StaticAnalyzer

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA updated this revision to Diff 383359.
ZarkoCA added a comment.

- Fix formatting and change variable names to make better grammatical sense


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112642/new/

https://reviews.llvm.org/D112642

Files:
  clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/test/Analysis/vfork.c

Index: clang/test/Analysis/vfork.c
===
--- clang/test/Analysis/vfork.c
+++ clang/test/Analysis/vfork.c
@@ -15,7 +15,7 @@
   case 0:
 // Ensure that modifying pid is ok.
 pid = 1; // no-warning
-// Ensure that calling whitelisted routines is ok.
+// Ensure that calling allowlisted routines is ok.
 switch (y) {
 case 0:
   execl("", "", 0); // no-warning
@@ -65,7 +65,7 @@
   case 0:
 // Ensure that writing pid is ok.
 pid = 1; // no-warning
-// Ensure that calling whitelisted routines is ok.
+// Ensure that calling allowlisted routines is ok.
 execl("", "", 0); // no-warning
 _exit(1); // no-warning
 break;
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -974,7 +974,7 @@
 
 // First handle the globals defined in system headers.
 if (Ctx.getSourceManager().isInSystemHeader(D->getLocation())) {
-  // Whitelist the system globals which often DO GET modified, assume the
+  //  Allow the system globals which often DO GET modified, assume the
   // rest are immutable.
   if (D->getName().contains("errno"))
 sReg = getGlobalsRegion(MemRegion::GlobalSystemSpaceRegionKind);
Index: clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
===
--- clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
+++ clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
@@ -29,7 +29,7 @@
 /// values).
 ///
 /// For more context see Static Analyzer checkers documentation - specifically
-/// webkit.UncountedCallArgsChecker checker. Whitelist of transformations:
+/// webkit.UncountedCallArgsChecker checker. Allowed list of transformations:
 /// - constructors of ref-counted types (including factory methods)
 /// - getters of ref-counted types
 /// - member overloaded operators
Index: clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
@@ -9,7 +9,7 @@
 //  This file defines vfork checker which checks for dangerous uses of vfork.
 //  Vforked process shares memory (including stack) with parent so it's
 //  range of actions is significantly limited: can't write variables,
-//  can't call functions not in whitelist, etc. For more details, see
+//  can't call functions not in the allowed list, etc. For more details, see
 //  http://man7.org/linux/man-pages/man2/vfork.2.html
 //
 //  This checker checks for prohibited constructs in vforked process.
@@ -44,13 +44,14 @@
 class VforkChecker : public Checker> {
   mutable std::unique_ptr BT;
-  mutable llvm::SmallSet VforkWhitelist;
+  mutable llvm::SmallSet VforkAllowlist;
   mutable const IdentifierInfo *II_vfork;
 
   static bool isChildProcess(const ProgramStateRef State);
 
   bool isVforkCall(const Decl *D, CheckerContext ) const;
-  bool isCallWhitelisted(const IdentifierInfo *II, CheckerContext ) const;
+  bool isCallExplicitelyAllowed(const IdentifierInfo *II,
+CheckerContext ) const;
 
   void reportBug(const char *What, CheckerContext ,
  const char *Details = nullptr) const;
@@ -93,9 +94,9 @@
 }
 
 // Returns true iff ok to call function after successful vfork.
-bool VforkChecker::isCallWhitelisted(const IdentifierInfo *II,
- CheckerContext ) const {
-  if (VforkWhitelist.empty()) {
+bool VforkChecker::isCallExplicitelyAllowed(const IdentifierInfo *II,
+CheckerContext ) const {
+  if (VforkAllowlist.empty()) {
 // According to manpage.
 const char *ids[] = {
   "_Exit",
@@ -112,10 +113,10 @@
 
 ASTContext  = C.getASTContext();
 for (const char **id = ids; *id; ++id)
-  VforkWhitelist.insert((*id));
+  VforkAllowlist.insert((*id));
   }
 
-  return VforkWhitelist.count(II);
+  return VforkAllowlist.count(II);
 }
 
 void VforkChecker::reportBug(const char *What, CheckerContext ,
@@ -179,12 +180,13 @@
   C.addTransition(ChildState);
 }
 
-// Prohibit calls to non-whitelist 

[PATCH] D112642: [clang][NFC] Inclusive terms: Replace uses of whitelist in clang/lib/StaticAnalyzer

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
Herald added a subscriber: rnkovacs.

Thanks @ZarkoCA, LGTM, but please address the sensible changes advised by 
Aaron. And thanks @aaron.ballman for the review!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112642/new/

https://reviews.llvm.org/D112642

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112642: [clang][NFC] Inclusive terms: Replace uses of whitelist in clang/lib/StaticAnalyzer

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

Thank you for this! Mostly just bikeshedding on names (allowlist as a verb 
sounds weird to me), feel free to take or leave the suggestions. LG aside from 
the formatting nits.




Comment at: clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:97
   // all other cast kinds (where enum range checks are unnecessary or invalid),
-  // just return immediately.  TODO: The set of casts whitelisted for enum
+  // just return immediately.  TODO: The set of casts allowlisted for enum
   // range checking may be incomplete.  Better to add a missing cast kind to





Comment at: 
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp:1191-1199
+  llvm::SmallPtrSet AllowlistedSymbols;
 
   for (const MemRegion *I : ExplicitRegions)
 if (const SymbolicRegion *SR = I->StripCasts()->getAs())
-  WhitelistedSymbols.insert(SR->getSymbol());
+  AllowlistedSymbols.insert(SR->getSymbol());
 
   for (SymbolRef sym : *invalidated) {





Comment at: clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp:12
 //  range of actions is significantly limited: can't write variables,
-//  can't call functions not in whitelist, etc. For more details, see
+//  can't call functions not in allowed list, etc. For more details, see
 //  http://man7.org/linux/man-pages/man2/vfork.2.html





Comment at: clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp:53
   bool isVforkCall(const Decl *D, CheckerContext ) const;
-  bool isCallWhitelisted(const IdentifierInfo *II, CheckerContext ) const;
+  bool isCallAllowlisted(const IdentifierInfo *II, CheckerContext ) const;
 





Comment at: clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp:96
 // Returns true iff ok to call function after successful vfork.
-bool VforkChecker::isCallWhitelisted(const IdentifierInfo *II,
+bool VforkChecker::isCallAllowlisted(const IdentifierInfo *II,
  CheckerContext ) const {

Also, should fix the clang-format issue.



Comment at: clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp:182
 
-// Prohibit calls to non-whitelist functions in child process.
+// Prohibit calls to non-allowlisted functions in child process.
 void VforkChecker::checkPreCall(const CallEvent ,





Comment at: clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp:187
   if (isChildProcess(State)
-  && !isCallWhitelisted(Call.getCalleeIdentifier(), C))
+  && !isCallAllowlisted(Call.getCalleeIdentifier(), C))
 reportBug("This function call", C);

Might as well handle this formatting issue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112642/new/

https://reviews.llvm.org/D112642

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112642: [clang][NFC] Inclusive terms: Replace uses of whitelist in clang/lib/StaticAnalyzer

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA created this revision.
ZarkoCA added reviewers: zaks.anna, jkorous, george.karpenkov, ygribov, 
Szelethus, quinnp, aaron.ballman.
ZarkoCA added a project: clang.
Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy, 
a.sidorin, baloghadamsoftware.
ZarkoCA requested review of this revision.
Herald added a subscriber: cfe-commits.

Replace variable and functions names, as well as comments that contain 
whitelist with
more inclusive terms.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112642

Files:
  clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/test/Analysis/vfork.c

Index: clang/test/Analysis/vfork.c
===
--- clang/test/Analysis/vfork.c
+++ clang/test/Analysis/vfork.c
@@ -15,7 +15,7 @@
   case 0:
 // Ensure that modifying pid is ok.
 pid = 1; // no-warning
-// Ensure that calling whitelisted routines is ok.
+// Ensure that calling allowlisted routines is ok.
 switch (y) {
 case 0:
   execl("", "", 0); // no-warning
@@ -65,7 +65,7 @@
   case 0:
 // Ensure that writing pid is ok.
 pid = 1; // no-warning
-// Ensure that calling whitelisted routines is ok.
+// Ensure that calling allowlisted routines is ok.
 execl("", "", 0); // no-warning
 _exit(1); // no-warning
 break;
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -974,7 +974,7 @@
 
 // First handle the globals defined in system headers.
 if (Ctx.getSourceManager().isInSystemHeader(D->getLocation())) {
-  // Whitelist the system globals which often DO GET modified, assume the
+  //  Allow the system globals which often DO GET modified, assume the
   // rest are immutable.
   if (D->getName().contains("errno"))
 sReg = getGlobalsRegion(MemRegion::GlobalSystemSpaceRegionKind);
Index: clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
===
--- clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
+++ clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
@@ -29,7 +29,7 @@
 /// values).
 ///
 /// For more context see Static Analyzer checkers documentation - specifically
-/// webkit.UncountedCallArgsChecker checker. Whitelist of transformations:
+/// webkit.UncountedCallArgsChecker checker. Allowed list of transformations:
 /// - constructors of ref-counted types (including factory methods)
 /// - getters of ref-counted types
 /// - member overloaded operators
Index: clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
@@ -9,7 +9,7 @@
 //  This file defines vfork checker which checks for dangerous uses of vfork.
 //  Vforked process shares memory (including stack) with parent so it's
 //  range of actions is significantly limited: can't write variables,
-//  can't call functions not in whitelist, etc. For more details, see
+//  can't call functions not in allowed list, etc. For more details, see
 //  http://man7.org/linux/man-pages/man2/vfork.2.html
 //
 //  This checker checks for prohibited constructs in vforked process.
@@ -44,13 +44,13 @@
 class VforkChecker : public Checker> {
   mutable std::unique_ptr BT;
-  mutable llvm::SmallSet VforkWhitelist;
+  mutable llvm::SmallSet VforkAllowlist;
   mutable const IdentifierInfo *II_vfork;
 
   static bool isChildProcess(const ProgramStateRef State);
 
   bool isVforkCall(const Decl *D, CheckerContext ) const;
-  bool isCallWhitelisted(const IdentifierInfo *II, CheckerContext ) const;
+  bool isCallAllowlisted(const IdentifierInfo *II, CheckerContext ) const;
 
   void reportBug(const char *What, CheckerContext ,
  const char *Details = nullptr) const;
@@ -93,9 +93,9 @@
 }
 
 // Returns true iff ok to call function after successful vfork.
-bool VforkChecker::isCallWhitelisted(const IdentifierInfo *II,
+bool VforkChecker::isCallAllowlisted(const IdentifierInfo *II,
  CheckerContext ) const {
-  if (VforkWhitelist.empty()) {
+  if (VforkAllowlist.empty()) {
 // According to manpage.
 const char *ids[] = {
   "_Exit",
@@ -112,10 +112,10 @@
 
 ASTContext  = C.getASTContext();
 for (const char **id = ids; *id; ++id)
-  VforkWhitelist.insert((*id));
+  VforkAllowlist.insert((*id));
   }
 
-  return VforkWhitelist.count(II);
+  return VforkAllowlist.count(II);
 }
 
 void