[PATCH] D158156: [analyzer] Add C++ array delete checker

[Discookie via Phabricator via cfe-commits]

Discookie added a comment.

@steakhal gentle ping for one last round of reviews


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

https://reviews.llvm.org/D158156

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

[PATCH] D158156: [analyzer] Add C++ array delete checker

[Discookie via Phabricator via cfe-commits]

Discookie updated this revision to Diff 557438.
Discookie added a comment.

Replaced backticks with single quotes.


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

https://reviews.llvm.org/D158156

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
  clang/test/Analysis/ArrayDelete.cpp
  clang/test/Analysis/DeleteWithNonVirtualDtor.cpp
  clang/www/analyzer/alpha_checks.html

Index: clang/www/analyzer/alpha_checks.html
===
--- clang/www/analyzer/alpha_checks.html
+++ clang/www/analyzer/alpha_checks.html
@@ -330,6 +330,26 @@
 
 
 
+
+alpha.cplusplus.ArrayDelete
+(C++)
+Reports destructions of arrays of polymorphic objects that are destructed as
+their base class
+
+
+
+Base *create() {
+  Base *x = new Derived[10]; // note: Casting from 'Derived' to 'Base' here
+  return x;
+}
+
+void sink(Base *x) {
+  delete[] x; // warn: Deleting an array of 'Derived' objects as their base class 'Base' undefined
+}
+
+
+
+
 
 alpha.cplusplus.DeleteWithNonVirtualDtor
 (C++)
@@ -339,8 +359,8 @@
 
 
 NonVirtual *create() {
-  NonVirtual *x = new NVDerived(); // note: conversion from derived to base
-   //   happened here
+  NonVirtual *x = new NVDerived(); // note: Casting from 'NVDerived' to
+   //   'NonVirtual' here
   return x;
 }
 
Index: clang/test/Analysis/DeleteWithNonVirtualDtor.cpp
===
--- clang/test/Analysis/DeleteWithNonVirtualDtor.cpp
+++ clang/test/Analysis/DeleteWithNonVirtualDtor.cpp
@@ -33,7 +33,7 @@
 NVDerived *get();
 
 NonVirtual *create() {
-  NonVirtual *x = new NVDerived(); // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *x = new NVDerived(); // expected-note{{Casting from 'NVDerived' to 'NonVirtual' here}}
   return x;
 }
 
@@ -52,32 +52,32 @@
 
 void singleDerived() {
   NonVirtual *sd;
-  sd = new NVDerived(); // expected-note{{Conversion from derived to base happened here}}
+  sd = new NVDerived(); // expected-note{{Casting from 'NVDerived' to 'NonVirtual' here}}
   delete sd; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
 
 void singleDerivedArr() {
-  NonVirtual *sda = new NVDerived[5]; // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *sda = new NVDerived[5]; // expected-note{{Casting from 'NVDerived' to 'NonVirtual' here}}
   delete[] sda; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
 
 void doubleDerived() {
-  NonVirtual *dd = new NVDoubleDerived(); // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *dd = new NVDoubleDerived(); // expected-note{{Casting from 'NVDoubleDerived' to 'NonVirtual' here}}
   delete (dd); // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
 
 void assignThroughFunction() {
-  NonVirtual *atf = get(); // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *atf = get(); // expected-note{{Casting from 'NVDerived' to 'NonVirtual' here}}
   delete atf; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
 
 void assignThroughFunction2() {
   NonVirtual *atf2;
-  atf2 = get(); // expected-note{{Conversion from derived to base happened here}}
+  atf2 = get(); // expected-note{{Casting from 'NVDerived' to 'NonVirtual' here}}
   delete atf2; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
@@ -90,49 +90,49 @@
 }
 
 void deleteThroughFunction() {
-  NonVirtual *dtf = new NVDerived(); // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *dtf = new NVDerived(); // expected-note{{Casting from 'NVDerived' to 'NonVirtual' here}}
   sink(dtf); // expected-note{{Calling 'sink'}}
 }
 
 void singleCastCStyle() {
   NVDerived *sccs = new NVDerived();
-  NonVirtual *sccs2 = (NonVirtual*)sccs; // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *sccs2 = (NonVirtual*)sccs; // expected-note{{Casting from 'NVDerived' to 'NonVirtual' here}}
   delete sccs2; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expe

[PATCH] D158156: [analyzer] Add C++ array delete checker

[Discookie via Phabricator via cfe-commits]

Discookie updated this revision to Diff 557439.

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

https://reviews.llvm.org/D158156

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
  clang/test/Analysis/ArrayDelete.cpp
  clang/test/Analysis/DeleteWithNonVirtualDtor.cpp
  clang/www/analyzer/alpha_checks.html

Index: clang/www/analyzer/alpha_checks.html
===
--- clang/www/analyzer/alpha_checks.html
+++ clang/www/analyzer/alpha_checks.html
@@ -330,6 +330,26 @@
 
 
 
+
+alpha.cplusplus.ArrayDelete
+(C++)
+Reports destructions of arrays of polymorphic objects that are destructed as
+their base class
+
+
+
+Base *create() {
+  Base *x = new Derived[10]; // note: Casting from 'Derived' to 'Base' here
+  return x;
+}
+
+void sink(Base *x) {
+  delete[] x; // warn: Deleting an array of 'Derived' objects as their base class 'Base' undefined
+}
+
+
+
+
 
 alpha.cplusplus.DeleteWithNonVirtualDtor
 (C++)
@@ -339,8 +359,8 @@
 
 
 NonVirtual *create() {
-  NonVirtual *x = new NVDerived(); // note: conversion from derived to base
-   //   happened here
+  NonVirtual *x = new NVDerived(); // note: Casting from 'NVDerived' to
+   //   'NonVirtual' here
   return x;
 }
 
Index: clang/test/Analysis/DeleteWithNonVirtualDtor.cpp
===
--- clang/test/Analysis/DeleteWithNonVirtualDtor.cpp
+++ clang/test/Analysis/DeleteWithNonVirtualDtor.cpp
@@ -33,7 +33,7 @@
 NVDerived *get();
 
 NonVirtual *create() {
-  NonVirtual *x = new NVDerived(); // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *x = new NVDerived(); // expected-note{{Casting from 'NVDerived' to 'NonVirtual' here}}
   return x;
 }
 
@@ -52,32 +52,32 @@
 
 void singleDerived() {
   NonVirtual *sd;
-  sd = new NVDerived(); // expected-note{{Conversion from derived to base happened here}}
+  sd = new NVDerived(); // expected-note{{Casting from 'NVDerived' to 'NonVirtual' here}}
   delete sd; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
 
 void singleDerivedArr() {
-  NonVirtual *sda = new NVDerived[5]; // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *sda = new NVDerived[5]; // expected-note{{Casting from 'NVDerived' to 'NonVirtual' here}}
   delete[] sda; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
 
 void doubleDerived() {
-  NonVirtual *dd = new NVDoubleDerived(); // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *dd = new NVDoubleDerived(); // expected-note{{Casting from 'NVDoubleDerived' to 'NonVirtual' here}}
   delete (dd); // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
 
 void assignThroughFunction() {
-  NonVirtual *atf = get(); // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *atf = get(); // expected-note{{Casting from 'NVDerived' to 'NonVirtual' here}}
   delete atf; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
 
 void assignThroughFunction2() {
   NonVirtual *atf2;
-  atf2 = get(); // expected-note{{Conversion from derived to base happened here}}
+  atf2 = get(); // expected-note{{Casting from 'NVDerived' to 'NonVirtual' here}}
   delete atf2; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
@@ -90,49 +90,49 @@
 }
 
 void deleteThroughFunction() {
-  NonVirtual *dtf = new NVDerived(); // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *dtf = new NVDerived(); // expected-note{{Casting from 'NVDerived' to 'NonVirtual' here}}
   sink(dtf); // expected-note{{Calling 'sink'}}
 }
 
 void singleCastCStyle() {
   NVDerived *sccs = new NVDerived();
-  NonVirtual *sccs2 = (NonVirtual*)sccs; // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *sccs2 = (NonVirtual*)sccs; // expected-note{{Casting from 'NVDerived' to 'NonVirtual' here}}
   delete sccs2; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual de

[PATCH] D158156: [analyzer] Add C++ array delete checker

[Discookie via Phabricator via cfe-commits]

Discookie marked 5 inline comments as done.
Discookie added a comment.

Ran the checker on a couple larger projects, but no real-world reports found so 
far (same as DeleteWithNonVirtualDtor). Can't tell if it's an engine limitation 
or just the bug being uncommon in general.




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:762-763
+def CXXArrayDeleteChecker : Checker<"ArrayDelete">,
+  HelpText<"Reports destructions of arrays of polymorphic objects that are"
+   "destructed as their base class.">,
+  Documentation;

steakhal wrote:
> I thin these strings are concatenated like in C, thus it's gonna have 
> "aredestructed" joined.
Whoops, fixed.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:199-201
+  // FIXME: This way of getting base types does not support reference types.
+  QualType SourceType = CastE->getSubExpr()->getType()->getPointeeType();
+  QualType TargetType = CastE->getType()->getPointeeType();

steakhal wrote:
> What is the problem with this?
> I thought `getPointeeType()` works for ReferenceTypes.
Apparently not, because references aren't ReferenceTypes but qualified Types. I 
could add support for it in a future commit, but I'd think casting and deleting 
array-references wrongly is even less common than deleting array-pointers.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:218-219
+
+  OS << "Casting from `" << SourceType.getAsString() << "` to `"
+ << TargetType.getAsString() << "` here";
+

steakhal wrote:
> We use single apostrophes for quoting names in CSA.
Got it, thanks!


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

https://reviews.llvm.org/D158156

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

[PATCH] D158156: [analyzer] Add C++ array delete checker

[Discookie via Phabricator via cfe-commits]

Discookie created this revision.
Discookie added reviewers: NoQ, donat.nagy, balazske.
Discookie added projects: clang, All.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, dkrupp, 
Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Discookie requested review of this revision.
Herald added a subscriber: cfe-commits.

This checker reports cases where an array of polymorphic objects are deleted as 
their base class.
Deleting an array where the array's static type is different from its dynamic 
type is undefined.

Since the checker is similar to DeleteWithNonVirtualDtorChecker, I refactored 
that checker to support more detection types.

This checker corresponds to the SEI Cert rule EXP51-CPP: Do not delete an array 
through a pointer of the incorrect type 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158156

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
  clang/test/Analysis/ArrayDelete.cpp
  clang/www/analyzer/alpha_checks.html

Index: clang/www/analyzer/alpha_checks.html
===
--- clang/www/analyzer/alpha_checks.html
+++ clang/www/analyzer/alpha_checks.html
@@ -330,6 +330,27 @@
 
 
 
+
+alpha.cplusplus.ArrayDelete
+(C++)
+Reports destructions of arrays of polymorphic objects that are destructed as
+their base class
+
+
+
+Base *create() {
+  Base *x = new Derived[10]; // note: conversion from derived to base
+ //   happened here
+  return x;
+}
+
+void sink(Base *x) {
+  delete[] x; // warn: Deleting an array of polymorphic objects is undefined
+}
+
+
+
+
 
 alpha.cplusplus.DeleteWithNonVirtualDtor
 (C++)
Index: clang/test/Analysis/ArrayDelete.cpp
===
--- /dev/null
+++ clang/test/Analysis/ArrayDelete.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.cplusplus.ArrayDelete -std=c++11 -verify -analyzer-output=text %s
+
+struct Base {
+virtual ~Base() = default;
+};
+
+struct Derived : public Base {};
+
+struct DoubleDerived : public Derived {};
+
+Derived *get();
+
+Base *create() {
+Base *b = new Derived[3]; // expected-note{{Conversion from derived to base happened here}}
+return b;
+}
+
+void sink(Base *b) {
+delete[] b; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+}
+
+void sink_cast(Base *b) {
+delete[] reinterpret_cast(b); // no-warning
+}
+
+void sink_derived(Derived *d) {
+delete[] d; // no-warning
+}
+
+void same_function() {
+Base *sd = new Derived[10]; // expected-note{{Conversion from derived to base happened here}}
+delete[] sd; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *dd = new DoubleDerived[10]; // expected-note{{Conversion from derived to base happened here}}
+delete[] dd; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+}
+
+void different_function() {
+Base *assigned = get(); // expected-note{{Conversion from derived to base happened here}}
+delete[] assigned; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *indirect;
+indirect = get(); // expected-note{{Conversion from derived to base happened here}}
+delete[] indirect; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *created = create(); // expected-note{{Calling 'create'}}
+// expected-note@-1{{Returning from 'create'}}
+delete[] created; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *sb = new Derived[10]; // expected-note{{Conversion from derived to base happened here}}
+sink(sb); // expected-note{{Calling 'sink'}}
+}
+
+void safe_function() {
+Derived *d = new Derived[10];
+delete[] d; // no-warning
+
+Base *b = new Derived[10];
+delete[] reinterpret_cast(b); // no-warning
+
+Base *sb = new Derived[10];
+sink_cast(sb); // no-warning
+
+Derived *sd = new Derived[10];
+sink_derived(sd); // no-warning
+}
Index: clang/lib/Stati

[PATCH] D158156: [analyzer] Add C++ array delete checker

[Discookie via Phabricator via cfe-commits]

Discookie planned changes to this revision.
Discookie added a comment.

Indeed it would make more sense to separate checkers into their own classes, 
but using a shared base. This checker doesn't have quite as advanced logic as 
MallocChecker, so there's not that much of a need for a single class managing 
everything. I'll change the checker to that system.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158156

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

[PATCH] D158156: [analyzer] Add C++ array delete checker

[Discookie via Phabricator via cfe-commits]

Discookie updated this revision to Diff 556402.
Discookie marked 8 inline comments as done.
Discookie added a comment.

Added a test for taking last upcast only for the note tag. For now the visitor 
matches all explicit casts, because there are too many edge cases to count for 
now wrt. explicit upcasting.


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

https://reviews.llvm.org/D158156

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp
  clang/test/Analysis/ArrayDelete.cpp
  clang/www/analyzer/alpha_checks.html

Index: clang/www/analyzer/alpha_checks.html
===
--- clang/www/analyzer/alpha_checks.html
+++ clang/www/analyzer/alpha_checks.html
@@ -330,6 +330,27 @@
 
 
 
+
+alpha.cplusplus.ArrayDelete
+(C++)
+Reports destructions of arrays of polymorphic objects that are destructed as
+their base class
+
+
+
+Base *create() {
+  Base *x = new Derived[10]; // note: conversion from derived to base
+ //   happened here
+  return x;
+}
+
+void sink(Base *x) {
+  delete[] x; // warn: Deleting an array of polymorphic objects is undefined
+}
+
+
+
+
 
 alpha.cplusplus.DeleteWithNonVirtualDtor
 (C++)
Index: clang/test/Analysis/ArrayDelete.cpp
===
--- /dev/null
+++ clang/test/Analysis/ArrayDelete.cpp
@@ -0,0 +1,100 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.cplusplus.ArrayDelete -std=c++11 -verify -analyzer-output=text %s
+
+struct Base {
+virtual ~Base() = default;
+};
+
+struct Derived : public Base {};
+
+struct DoubleDerived : public Derived {};
+
+Derived *get();
+
+Base *create() {
+Base *b = new Derived[3]; // expected-note{{Conversion from derived to base happened here}}
+return b;
+}
+
+void sink(Base *b) {
+delete[] b; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+}
+
+void sink_cast(Base *b) {
+delete[] reinterpret_cast(b); // no-warning
+}
+
+void sink_derived(Derived *d) {
+delete[] d; // no-warning
+}
+
+void same_function() {
+Base *sd = new Derived[10]; // expected-note{{Conversion from derived to base happened here}}
+delete[] sd; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *dd = new DoubleDerived[10]; // expected-note{{Conversion from derived to base happened here}}
+delete[] dd; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+}
+
+void different_function() {
+Base *assigned = get(); // expected-note{{Conversion from derived to base happened here}}
+delete[] assigned; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *indirect;
+indirect = get(); // expected-note{{Conversion from derived to base happened here}}
+delete[] indirect; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *created = create(); // expected-note{{Calling 'create'}}
+// expected-note@-1{{Returning from 'create'}}
+delete[] created; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *sb = new Derived[10]; // expected-note{{Conversion from derived to base happened here}}
+sink(sb); // expected-note{{Calling 'sink'}}
+}
+
+void safe_function() {
+Derived *d = new Derived[10];
+delete[] d; // no-warning
+
+Base *b = new Derived[10];
+delete[] reinterpret_cast(b); // no-warning
+
+Base *sb = new Derived[10];
+sink_cast(sb); // no-warning
+
+Derived *sd = new Derived[10];
+sink_derived(sd); // no-warning
+}
+
+void multiple_derived() {
+Base *b = new DoubleDerived[10]; // expected-note{{Conversion from derived to base happened here}}
+delete[] b; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+// FIXME: Currently all explicit casts are reported as derived-to-base casts.
+Base *b2 = new DoubleDerived[10];
+Derived *d2 = reinterpret_cast(b2); // expected-note{{Conversion from derived to base happened here}}
+delete[] d2; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Derived *d3 = new DoubleD

[PATCH] D158156: [analyzer] Add C++ array delete checker

[Discookie via Phabricator via cfe-commits]

Discookie added inline comments.



Comment at: clang/docs/analyzer/checkers.rst:1793-1803
+.. code-block:: cpp
+
+ Base *create() {
+   Base *x = new Derived[10]; // note: conversion from derived to base
+  //   happened here
+   return x;
+ }

donat.nagy wrote:
> steakhal wrote:
> > Discookie wrote:
> > > steakhal wrote:
> > > > Make sure in the example the `create` is related (e.g. called/used).
> > > > Also, refrain from using `sink` in the docs. It's usually used in the 
> > > > context of taint analysis.
> > > Changed the example - should I change the DeleteWithNonVirtualDtor 
> > > example as well? That has the same issues as you said here.
> > I have no opinion. You could.
> I think you should definitely update it, for the sake of consistency and just 
> improving things whenever we can. This commit already touches the code of 
> that checker, there is no point in leaving bad documentation behind us...
Updated.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:126
+
+  if (!DerivedClass->isDerivedFrom(BaseClass))
+return;

steakhal wrote:
> Is this transitive?
> 
> BTW inheritance can only be expressed if the class is a definition, right?
> Thus passing this should imply has definition.
It's transitive indeed.

I thought there were some presumptions about hasDefinition() in 
isDerivedFrom(), but apparently not, and it didn't cause any crashes either. 
Removed the redundant checks.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:192
+
+  const Stmt *S = N->getStmtForDiagnostics();
+  if (!S)

steakhal wrote:
> Aren't you actually interested in 
> N->getLocation().getAs().getStmt()?
> 
> The diag stmt can be fuzzy, but the PP is exact.
As far as I can tell, getStmtForDiagnostics() does exactly that, but with a bit 
more edge case handling and a couple fallbacks.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:196
+
+  const auto *CastE = dyn_cast(S);
+  if (!CastE)

steakhal wrote:
> If you dyncast to ImplicitCastExpr, couldn't you have done it here?
I'm interested in all cast expressions, not just implicit ones. There can also 
be explicit upcasts as well.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:202
+  // Explicit casts can have different CastKinds.
+  if (const auto *ImplCastE = dyn_cast(CastE)) {
+if (ImplCastE->getCastKind() != CK_DerivedToBase)

steakhal wrote:
> How do you know that that this castexpr corresponds to the region for which 
> you report the bug? To mez this might be some unrelated castexpr.
> I was expecting the offending memregion being passed to the visitor, that it 
> can check against.
We know it's the one we are looking for, because the checker marked it 
interesting earlier. Not sure how memregion equality check would differ from 
taking isInteresting(), but it can be changed to that if needed.


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

https://reviews.llvm.org/D158156

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

[PATCH] D158156: [analyzer] Add C++ array delete checker

[Discookie via Phabricator via cfe-commits]

Discookie updated this revision to Diff 556933.
Discookie added a comment.

Updated the visitor to track all conversions, and have type names for clarity.


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

https://reviews.llvm.org/D158156

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
  clang/test/Analysis/ArrayDelete.cpp
  clang/test/Analysis/DeleteWithNonVirtualDtor.cpp
  clang/www/analyzer/alpha_checks.html

Index: clang/www/analyzer/alpha_checks.html
===
--- clang/www/analyzer/alpha_checks.html
+++ clang/www/analyzer/alpha_checks.html
@@ -330,6 +330,26 @@
 
 
 
+
+alpha.cplusplus.ArrayDelete
+(C++)
+Reports destructions of arrays of polymorphic objects that are destructed as
+their base class
+
+
+
+Base *create() {
+  Base *x = new Derived[10]; // note: Casting from `Derived` to `Base` here
+  return x;
+}
+
+void sink(Base *x) {
+  delete[] x; // warn: Deleting an array of `Derived` objects as their base class `Base` undefined
+}
+
+
+
+
 
 alpha.cplusplus.DeleteWithNonVirtualDtor
 (C++)
Index: clang/test/Analysis/DeleteWithNonVirtualDtor.cpp
===
--- clang/test/Analysis/DeleteWithNonVirtualDtor.cpp
+++ clang/test/Analysis/DeleteWithNonVirtualDtor.cpp
@@ -33,7 +33,7 @@
 NVDerived *get();
 
 NonVirtual *create() {
-  NonVirtual *x = new NVDerived(); // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *x = new NVDerived(); // expected-note{{Casting from `NVDerived` to `NonVirtual` here}}
   return x;
 }
 
@@ -52,32 +52,32 @@
 
 void singleDerived() {
   NonVirtual *sd;
-  sd = new NVDerived(); // expected-note{{Conversion from derived to base happened here}}
+  sd = new NVDerived(); // expected-note{{Casting from `NVDerived` to `NonVirtual` here}}
   delete sd; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
 
 void singleDerivedArr() {
-  NonVirtual *sda = new NVDerived[5]; // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *sda = new NVDerived[5]; // expected-note{{Casting from `NVDerived` to `NonVirtual` here}}
   delete[] sda; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
 
 void doubleDerived() {
-  NonVirtual *dd = new NVDoubleDerived(); // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *dd = new NVDoubleDerived(); // expected-note{{Casting from `NVDoubleDerived` to `NonVirtual` here}}
   delete (dd); // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
 
 void assignThroughFunction() {
-  NonVirtual *atf = get(); // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *atf = get(); // expected-note{{Casting from `NVDerived` to `NonVirtual` here}}
   delete atf; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
 
 void assignThroughFunction2() {
   NonVirtual *atf2;
-  atf2 = get(); // expected-note{{Conversion from derived to base happened here}}
+  atf2 = get(); // expected-note{{Casting from `NVDerived` to `NonVirtual` here}}
   delete atf2; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
@@ -90,49 +90,49 @@
 }
 
 void deleteThroughFunction() {
-  NonVirtual *dtf = new NVDerived(); // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *dtf = new NVDerived(); // expected-note{{Casting from `NVDerived` to `NonVirtual` here}}
   sink(dtf); // expected-note{{Calling 'sink'}}
 }
 
 void singleCastCStyle() {
   NVDerived *sccs = new NVDerived();
-  NonVirtual *sccs2 = (NonVirtual*)sccs; // expected-note{{Conversion from derived to base happened here}}
+  NonVirtual *sccs2 = (NonVirtual*)sccs; // expected-note{{Casting from `NVDerived` to `NonVirtual` here}}
   delete sccs2; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
   // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
 }
 
 void doubleCastCStyle() {
-  NonVirtual *dccs = new NVDerived();
-  NVDerived *dccs2 = (NVDerived*)dccs;
-  dccs = (NonVirtual*)dccs2; // expected-note{{Conversion from derived to base happened here}}
+  NonVirtua

[PATCH] D158156: [analyzer] Add C++ array delete checker

[Discookie via Phabricator via cfe-commits]

Discookie marked 12 inline comments as done.
Discookie added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:192
+
+  const Stmt *S = N->getStmtForDiagnostics();
+  if (!S)

steakhal wrote:
> Discookie wrote:
> > steakhal wrote:
> > > Aren't you actually interested in 
> > > N->getLocation().getAs().getStmt()?
> > > 
> > > The diag stmt can be fuzzy, but the PP is exact.
> > As far as I can tell, getStmtForDiagnostics() does exactly that, but with a 
> > bit more edge case handling and a couple fallbacks.
> If they do the same, and it does not depend on the mentioned fallbacks, I 
> think we should use the Stmt of the programpoint to be explicit.
I'd rather not, since this pattern isn't used anywhere else, while 
getStmtForDiagnostics is widely used across the codebase.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:217
+  // Stop traversal on this path.
+  Satisfied = true;
+

steakhal wrote:
> There are so many early returns going on. I feel, we could miss the program 
> point where it should have been marked satisfied. After this point, the 
> visitor will never or should never emit a note.
Since we're emitting notes for all casts, this isn't needed anymore.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:194-201
   // Only interested in DerivedToBase implicit casts.
   // Explicit casts can have different CastKinds.
+  // FIXME: The checker currently matches all explicit casts,
+  // but only ones casting to a base class (or simular) should be matcherd.
   if (const auto *ImplCastE = dyn_cast(CastE)) {
 if (ImplCastE->getCastKind() != CK_DerivedToBase)
   return nullptr;

steakhal wrote:
> Have you considered `dyn_cast()` to make it work for both implicit 
> and explicit casts?
> BTW `simular` -> `similar`
I did exactly that a little earlier :D but I reworked the logic to filter based 
on the availability of types instead of the CastType.



Comment at: clang/test/Analysis/ArrayDelete.cpp:85-88
+Derived *d3 = new DoubleDerived[10];
+Base *b3 = d3; // expected-note{{Conversion from derived to base happened 
here}}
+delete[] b3; // expected-warning{{Deleting an array of polymorphic objects 
is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is 
undefined}}

donat.nagy wrote:
> steakhal wrote:
> > donat.nagy wrote:
> > > steakhal wrote:
> > > > Hmm, the static type of `d3` doesn't tell us much about how it refers 
> > > > to an object of type `DoubleDerived`.
> > > > To me, it would make sense to have multiple `Conversion from derived to 
> > > > base happened here`, even telling us what static type it converted to 
> > > > what other static type in the message.
> > > > And it should add a new visitor of the same kind tracking the castee.
> > > > 
> > > > ```
> > > > Derived *d3 = new DoubleDerived[10]; // note: `DoubleDerived` -> 
> > > > `Derived` here
> > > > Base *b3 = d3; // note: `Derived` -> `Base` here
> > > > delete[] b3; // warn: Deleting `Derived` objects as `Base` objects.
> > > > ```
> > > > WDYT @donat.nagy ?
> > > I agree that it would be good to be good to mention the class names in 
> > > the message.
> > Do you also agree that we should have all steps where such a conversion 
> > happened?
> > Notice the 2 `note:` markers in my example. @donat.nagy 
> It would be a nice addition if it wouldn't seriously complicate the 
> implementation.
> 
> If we want to report multiple/all conversions, then we would need to create 
> messages for back-and-forth conversions (e.g. allocating Derived, converting 
> it to Base, back to Derived, back to Base, then deleting it illegally).
Added, as requested! While casts of reference types are not supported, it 
already makes the flow of classes much clearer.



Comment at: clang/test/Analysis/ArrayDelete.cpp:90-93
+Base *b4 = new DoubleDerived[10];
+Derived *d4 = reinterpret_cast(b4);
+DoubleDerived *dd4 = reinterpret_cast(d4);
+delete[] dd4; // no-warning

donat.nagy wrote:
> steakhal wrote:
> > I think in such cases a `static_cast` should suffice; unless you 
> > intentionally wanted to test `reinterpret_cast` of course.
> Note that the effects of `reinterpret_cast` and `static_cast` are different 
> [1]: `static_cast(BasePtr)` adjusts the pointer value to ensure 
> that it points to the derived object whose Base ancestor object would be 
> located at BasePtr, while a `reinterpret_cast` keeps the raw pointer value 
> (which is complete nonsense from an object-oriented point-of-view, but could 
> be relevant in some low-level hacks).
> 
> I'd guess that this part is directly testing the strange behavior of 
> `reinterpret_cast`; but it'd be good to add a comment that clarifies this.
> 
> [1]: 
> https://stackoverflow.com/questions/9138730/why-is-it-im

[PATCH] D158156: [analyzer] Add C++ array delete checker

[Discookie via Phabricator via cfe-commits]

Discookie marked 6 inline comments as done.
Discookie added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:220
  N->getLocationContext());
-  return std::make_shared(Pos, OS.str(), true);
+  return std::make_shared(Pos, OS.str(), 
/*addPosRange=*/true);
+}

steakhal wrote:
> We should assert that this visitor always adds a Note. In other words, that 
> it must find the Stmt where the derived->base conversion happened. If ever 
> that's not true, we have a bug.
Can't do that just yet as casts in the form of `Derived &d = ...; Base &b = d;` 
is not supported.


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

https://reviews.llvm.org/D158156

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

[PATCH] D158156: [analyzer] Add C++ array delete checker

[Discookie via Phabricator via cfe-commits]

Discookie updated this revision to Diff 555821.
Discookie added a comment.

Refactored the checkers to use a shared base class, otherwise I kept the same 
functionality.

I'll push this diff in a way where the blame transfers between the files, but I 
don't think keeping the file name the same makes sense, since this is 
definitely not a DeleteWithNonVirtualDtorChecker.


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

https://reviews.llvm.org/D158156

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
  clang/test/Analysis/ArrayDelete.cpp
  clang/www/analyzer/alpha_checks.html

Index: clang/www/analyzer/alpha_checks.html
===
--- clang/www/analyzer/alpha_checks.html
+++ clang/www/analyzer/alpha_checks.html
@@ -330,6 +330,27 @@
 
 
 
+
+alpha.cplusplus.ArrayDelete
+(C++)
+Reports destructions of arrays of polymorphic objects that are destructed as
+their base class
+
+
+
+Base *create() {
+  Base *x = new Derived[10]; // note: conversion from derived to base
+ //   happened here
+  return x;
+}
+
+void sink(Base *x) {
+  delete[] x; // warn: Deleting an array of polymorphic objects is undefined
+}
+
+
+
+
 
 alpha.cplusplus.DeleteWithNonVirtualDtor
 (C++)
Index: clang/test/Analysis/ArrayDelete.cpp
===
--- /dev/null
+++ clang/test/Analysis/ArrayDelete.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.cplusplus.ArrayDelete -std=c++11 -verify -analyzer-output=text %s
+
+struct Base {
+virtual ~Base() = default;
+};
+
+struct Derived : public Base {};
+
+struct DoubleDerived : public Derived {};
+
+Derived *get();
+
+Base *create() {
+Base *b = new Derived[3]; // expected-note{{Conversion from derived to base happened here}}
+return b;
+}
+
+void sink(Base *b) {
+delete[] b; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+}
+
+void sink_cast(Base *b) {
+delete[] reinterpret_cast(b); // no-warning
+}
+
+void sink_derived(Derived *d) {
+delete[] d; // no-warning
+}
+
+void same_function() {
+Base *sd = new Derived[10]; // expected-note{{Conversion from derived to base happened here}}
+delete[] sd; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *dd = new DoubleDerived[10]; // expected-note{{Conversion from derived to base happened here}}
+delete[] dd; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+}
+
+void different_function() {
+Base *assigned = get(); // expected-note{{Conversion from derived to base happened here}}
+delete[] assigned; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *indirect;
+indirect = get(); // expected-note{{Conversion from derived to base happened here}}
+delete[] indirect; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *created = create(); // expected-note{{Calling 'create'}}
+// expected-note@-1{{Returning from 'create'}}
+delete[] created; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *sb = new Derived[10]; // expected-note{{Conversion from derived to base happened here}}
+sink(sb); // expected-note{{Calling 'sink'}}
+}
+
+void safe_function() {
+Derived *d = new Derived[10];
+delete[] d; // no-warning
+
+Base *b = new Derived[10];
+delete[] reinterpret_cast(b); // no-warning
+
+Base *sb = new Derived[10];
+sink_cast(sb); // no-warning
+
+Derived *sd = new Derived[10];
+sink_derived(sd); // no-warning
+}
Index: clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
+++ /dev/null
@@ -1,155 +0,0 @@
-//===-- DeleteWithNonVirtualDtorChecker.cpp ---*- C++ -*--//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===

[PATCH] D158156: [analyzer] Add C++ array delete checker

[Discookie via Phabricator via cfe-commits]

Discookie marked 8 inline comments as done.
Discookie added a comment.

Fixed the formatting issues as well.




Comment at: clang/docs/analyzer/checkers.rst:1787-1804
+.. _alpha-cplusplus-ArrayDelete:
+
+alpha.cplusplus.ArrayDelete (C++)
+""
+Reports destructions of arrays of polymorphic objects that are destructed as 
their base class.
+
+.. code-block:: cpp

steakhal wrote:
> I think you should probably mention `EXP51-CPP CERT rule` somehow here.
Added a link to the CERT rules, similar to how others do it, thanks!



Comment at: clang/docs/analyzer/checkers.rst:1793-1803
+.. code-block:: cpp
+
+ Base *create() {
+   Base *x = new Derived[10]; // note: conversion from derived to base
+  //   happened here
+   return x;
+ }

steakhal wrote:
> Make sure in the example the `create` is related (e.g. called/used).
> Also, refrain from using `sink` in the docs. It's usually used in the context 
> of taint analysis.
Changed the example - should I change the DeleteWithNonVirtualDtor example as 
well? That has the same issues as you said here.


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

https://reviews.llvm.org/D158156

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

[PATCH] D146712: [clang-tidy] Add portability-non-portable-integer-constant check

[Discookie via Phabricator via cfe-commits]

Discookie created this revision.
Discookie added reviewers: aaron.ballman, njames93, carlosgalvezp.
Discookie added a project: clang-tools-extra.
Herald added subscribers: PiotrZSL, ChuanqiXu, xazax.hun.
Herald added a project: All.
Discookie requested review of this revision.

This check finds integer literals that are being used in a non-portable manner.

Currently, the check detects cases where maximum or minimum values should be 
used
instead, as well as error-prone integer literals having leading zeroes, or
relying on most significant bits.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146712

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/portability/CMakeLists.txt
  clang-tools-extra/clang-tidy/portability/NonPortableIntegerConstantCheck.cpp
  clang-tools-extra/clang-tidy/portability/NonPortableIntegerConstantCheck.h
  clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert/int17-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/portability/non-portable-integer-constant.rst
  
clang-tools-extra/test/clang-tidy/checkers/portability/non-portable-integer-constant.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/portability/non-portable-integer-constant.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/portability/non-portable-integer-constant.cpp
@@ -0,0 +1,258 @@
+// RUN: %check_clang_tidy %s -std=c++17-or-later portability-non-portable-integer-constant %t
+
+using int32_t = decltype(42);
+
+void regular() {
+  // no-warnings
+  0;
+  00;
+  0x0;
+  0x00;
+  0b0;
+  0b00;
+  0b0'0'0;
+
+  -1;
+  -0X1;
+
+  127;
+  0x7'f;
+
+  -128;
+  -0x80;
+
+  256;
+  0X100;
+
+  42;
+  0x2A;
+
+  180079837;
+  0xabbccdd;
+
+  // FIXME: (only 31 bits) False positive, reported as max signed int.
+  0b111;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: non-portable integer literal: hardcoded platform-specific maximum value [portability-non-portable-integer-constant]
+
+  // FIXME: Large numbers represented as hex are the most common false positive,
+  // eg. the following literal is a 64-bit prime.
+  0xff51afd7ed558ccd;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: non-portable integer literal: should not rely on the most significant bit [portability-non-portable-integer-constant]
+
+  // FIXME: Fixed-size integer literals are a common false positive as well.
+  int32_t literal = 0x4000;
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: non-portable integer literal: should not rely on bits of most significant byte [portability-non-portable-integer-constant]
+
+  // FIXME: According to the standard, the type of the integer literal is the
+  // smallest type it can fit into. While technically a false positive, it could
+  // signal literal width confusion regardless.
+  long long int long_literal = 0x7fff;
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: non-portable integer literal: hardcoded platform-specific maximum value [portability-non-portable-integer-constant]
+}
+
+enum LiteralClamp {
+
+};
+
+// INT_MIN, INT_MAX, UINT_MAX, UINT_MAX-1
+// All binary literals are 32 bits long
+void limits_int() {
+  // FIXME: not recognize as Min
+  -214748'3'648;
+  // --CHECK-MESSAGES: :[[@LINE-1]]:4: warning: non-portable integer literal: hardcoded platform-specific minimum value [portability-non-portable-integer-constant]
+
+  -0x80'00'00'00;
+  // CHECK-MESSAGES: :[[@LINE-1]]:4: warning: non-portable integer literal: hardcoded platform-specific minimum value [portability-non-portable-integer-constant]
+
+  -0200;
+  // CHECK-MESSAGES: :[[@LINE-1]]:4: warning: non-portable integer literal: hardcoded platform-specific minimum value [portability-non-portable-integer-constant]
+
+  -0b1000;
+  // CHECK-MESSAGES: :[[@LINE-1]]:4: warning: non-portable integer literal: hardcoded platform-specific minimum value [portability-non-portable-integer-constant]
+
+
+
+  21474'83647;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: non-portable integer literal: hardcoded platform-specific maximum value [portability-non-portable-integer-constant]
+
+  0x7FFF;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: non-portable integer literal: hardcoded platform-specific maximum value [portability-non-portable-integer-constant]
+
+  0177;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: non-portable integer literal: hardcoded platform-specific maximum value [portability-non-portable-integer-constant]
+
+  429'4'96'7295u;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: non-portable integer literal: hardcoded platform-specific maximum value [portability-non-portable-integer-constant]
+
+  0x;
+  // CHECK-MESSAGES