Merge pull request #17035 from geoffw0/allocerr

C++: Fix issue with cpp/incorrect-allocation-error-handling

Author: geoffw0

cpp/ql/src/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.ql

@@ -232,6 +232,7 @@ predicate nullCheckInThrowingNew(NewOrNewArrayExpr newExpr, GuardCondition guard
 from NewOrNewArrayExpr newExpr, Element element, string msg, string elementString
 where
   not newExpr.isFromUninstantiatedTemplate(_) and
+  not newExpr.isFromTemplateInstantiation(_) and
   (
     noThrowInTryBlock(newExpr, element) and
     msg = "This allocation cannot throw. $@ is unnecessary." and

cpp/ql/src/change-notes/2024-07-22-incorrect-allocation-error-handling.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The `cpp/incorrect-allocation-error-handling` ("Incorrect allocation-error handling") query no longer produces occasional false positive results inside template instantiations.

cpp/ql/test/query-tests/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.expected

@@ -13,7 +13,7 @@
 | test.cpp:92:5:92:31 | new[] | This allocation cannot throw. $@ is unnecessary. | test.cpp:97:36:98:3 | { ... } | This catch block |
 | test.cpp:93:15:93:41 | new[] | This allocation cannot throw. $@ is unnecessary. | test.cpp:97:36:98:3 | { ... } | This catch block |
 | test.cpp:96:10:96:36 | new[] | This allocation cannot throw. $@ is unnecessary. | test.cpp:97:36:98:3 | { ... } | This catch block |
-| test.cpp:151:9:151:24 | new | This allocation cannot throw. $@ is unnecessary. | test.cpp:152:15:152:18 | { ... } | This catch block |
-| test.cpp:199:15:199:35 | new | This allocation cannot throw. $@ is unnecessary. | test.cpp:201:16:201:19 | { ... } | This catch block |
-| test.cpp:212:14:212:34 | new | This allocation cannot throw. $@ is unnecessary. | test.cpp:213:34:213:36 | { ... } | This catch block |
-| test.cpp:246:17:246:31 | new[] | This allocation cannot return null. $@ is unnecessary. | test.cpp:247:8:247:12 | ! ... | This check |
+| test.cpp:160:9:160:24 | new | This allocation cannot throw. $@ is unnecessary. | test.cpp:161:15:161:18 | { ... } | This catch block |
+| test.cpp:229:15:229:35 | new | This allocation cannot throw. $@ is unnecessary. | test.cpp:231:16:231:19 | { ... } | This catch block |
+| test.cpp:242:14:242:34 | new | This allocation cannot throw. $@ is unnecessary. | test.cpp:243:34:243:36 | { ... } | This catch block |
+| test.cpp:276:17:276:31 | new[] | This allocation cannot return null. $@ is unnecessary. | test.cpp:277:8:277:12 | ! ... | This check |

cpp/ql/test/query-tests/Security/CWE/CWE-570/test.cpp

@@ -136,6 +136,8 @@ void good_new_handles_nullptr() {
     return; // GOOD
 }
 
+// ---
+
 void* operator new(std::size_t count, void*) noexcept;
 void* operator new[](std::size_t count, void*) noexcept;
 
@@ -146,18 +148,46 @@ struct Foo {
   operator bool();
 };
 
+struct Bar {
+  Bar();
+
+  operator bool();
+};
+
 void bad_placement_new_with_exception_handling() {
   char buffer[1024];
-  try { new (buffer) Foo; } // BAD
+
+  try { new (buffer) Foo; } // BAD (placement new should not fail)
   catch (...) {  }
 }
 
 void good_placement_new_with_exception_handling() {
   char buffer[1024];
+
   try { new (buffer) Foo(42); } // GOOD: Foo constructor might throw
   catch (...) {  }
+
+  try { new (buffer) Bar; } // GOOD: Bar constructor might throw
+  catch (...) {  }
 }
 
+template<typename F> F *test_template_platement_new() {
+  char buffer[1024];
+
+  try {
+    return new (buffer) F; // GOOD: `F` constructor might throw (when `F` is `Bar`)
+  } catch (...) {
+    return 0;
+  }
+}
+
+void test_template_platement_new_caller() {
+  test_template_platement_new<Foo>();
+  test_template_platement_new<Bar>();
+}
+
+// ---
+
 int unknown_value_without_exceptions() noexcept;
 
 void may_throw() {