Hi David,
On 05/07/2023 22:59, David Malcolm wrote:
diff --git a/gcc/analyzer/kf-lang-cp.cc b/gcc/analyzer/kf-lang-cp.cc
index 393b4f25e79..258d92919d7 100644
--- a/gcc/analyzer/kf-lang-cp.cc
+++ b/gcc/analyzer/kf-lang-cp.cc
@@ -35,6 +35,34 @@ along with GCC; see the file COPYING3. If not see
#if ENABLE_ANALYZER
+/* Return TRUE if CALL is non-allocating operator new or operator
new[]*/
+
+bool is_placement_new_p (const gcall *call)
Please can you extend the leading comment, giving the expected
signatures of the functions, and a link to cppreference.org.
In particular, there's some special-casing here of "nothrow_t" which
would make more sense with a comment up here.
I've now extended the leading comment of is_placement_new_p so that the
special cases appears clearer.
Leading comment is now:
/* Return true if CALL is a non-allocating operator new or operator
new []
that contains no user-defined args, i.e. having any signature of:
- void* operator new ( std::size_t count, void* ptr );
- void* operator new[]( std::size_t count, void* ptr );
See https://en.cppreference.com/w/cpp/memory/new/operator_new . */
Whereas above the "nothrow_t" special case now reads
/* We must distinguish between an allocating non-throwing new
and a non-allocating new.
The former might have one of the following signatures :
void* operator new ( std::size_t count, const std::nothrow_t&
tag );
void* operator new[]( std::size_t count, const std::nothrow_t&
tag );
However, debugging has shown that TAG is actually a POINTER_TYPE,
not a REFERENCE_TYPE.
Thus, we cannot easily differentiate the types, but we instead
have to
check if the second argument's type identifies as nothrow_t. */
+{
+ gcc_assert (call);
+
+ tree fndecl = gimple_call_fndecl (call);
+ if (!fndecl)
+ return false;
+
+ if (!is_named_call_p (fndecl, "operator new", call, 2)
+ && !is_named_call_p (fndecl, "operator new []", call, 2))
+ return false;
+ tree arg1 = gimple_call_arg (call, 1);
+
+ if (!POINTER_TYPE_P (TREE_TYPE (arg1)))
+ return false;
+
+ /* Sadly, for non-throwing new, the second argument type
+ is not REFERENCE_TYPE but also POINTER_TYPE
+ so a simple check is out of the way. */
+ tree identifier = TYPE_IDENTIFIER (TREE_TYPE (TREE_TYPE (arg1)));
+ if (!identifier)
+ return true;
+ const char *name = IDENTIFIER_POINTER (identifier);
+ return 0 != strcmp (name, "nothrow_t");
+}
+
namespace ana {
/* Implementations of specific functions. */
@@ -46,7 +74,7 @@ class kf_operator_new : public known_function
public:
bool matches_call_types_p (const call_details &cd) const final
override
{
- return cd.num_args () == 1;
+ return cd.num_args () == 1 || cd.num_args () == 2;
Looks like we should also check that arg 0 is of integral type, and
that arg 1 is of pointer type.
Well technically some standard signatures use an align_val_t as a second
argument,
which is a size_t value. But since we don't handle such signatures
properly yet, I'm going
with your suggestion.
}
void impl_call_pre (const call_details &cd) const final override
@@ -54,13 +82,60 @@ public:
region_model *model = cd.get_model ();
region_model_manager *mgr = cd.get_manager ();
const svalue *size_sval = cd.get_arg_svalue (0);
- const region *new_reg
- = model->get_or_create_region_for_heap_alloc (size_sval,
cd.get_ctxt ());
- if (cd.get_lhs_type ())
+ region_model_context *ctxt = cd.get_ctxt ();
+ const gcall *call = cd.get_call_stmt ();
+
+ /* If the call is an allocating new, then create a heap
allocated
+ region. */
+ if (!is_placement_new_p (call))
+ {
You have:
if (!condition)
suite_a;
else
suite_b; // this is implicitly a double negative
Please change it to:
if (condition)
suite_b;
else
suite_a;
to avoid the implicit double negative.
(nods)
diff --git a/gcc/testsuite/g++.dg/analyzer/new-2.C
b/gcc/testsuite/g++.dg/analyzer/new-2.C
new file mode 100644
index 00000000000..4e696040a54
--- /dev/null
+++ b/gcc/testsuite/g++.dg/analyzer/new-2.C
@@ -0,0 +1,50 @@
+// { dg-additional-options "-O0" }
+
+struct A
+{
+ int x;
+ int y;
+};
We've run into issues with bounds-checking testcases when using types
like "int" that have target-specific sizes.
Please use <stdint.h> in these test cases, and types with explicit
sizes, such as int32_t, to avoid the behavior of the test cases being
affected by sizeof the various types.
Thanks, I've now changed it in placement-new-size.C
[..snip...]
Other than those issues, looks good
Thanks again
Dave
Thanks for the review !
I'll submit the updated patch tomorrow on the mail list.
Benjamin
