On 6/13/21 5:45 PM, Jeff Law wrote:
On 6/2/2021 3:40 PM, Martin Sebor via Gcc-patches wrote:The two forms of placement operator new defined in <new> return their pointer argument and may not be displaced by user-defined functions. But because they are ordinary (not built-in) functions this property isn't reflected in their declarations alone, and there's no user- level attribute to annotate them with. When they are inlined the property is transparent in the IL but when they are not (without inlining such as -O0), calls to the operators appear in the IL and cause -Wmismatched-new-delete to try to match them with the functions called to deallocate memory. When the pointer to the memory was obtained from a function that matches the deallocator but not the placement new, the warning falsely triggers. The attached patch solves this by detecting calls to placement new and treating them the same as those to other pass-through calls (such as memset). In addition, it also teaches -Wfree-nonheap-object about placement delete, for a similar reason as above. Finally, it also adds a test for attribute fn spec indicating a function returns its argument. It's not necessary for the fix (I had initially though placement new might have the attribute) but it seems appropriate to check. Tested on x86_64-linux. Martin gcc-100876.diff PR c++/100876 - -Wmismatched-new-delete should understand placement new when it's not inlined gcc/ChangeLog: PR c++/100876 * builtins.c (gimple_call_return_array): Check for attribute fn spec. Handle calls to placement new. (ndecl_dealloc_argno): Avoid placement delete. gcc/testsuite/ChangeLog: PR c++/100876 * g++.dg/warn/Wmismatched-new-delete-4.C: New test. * g++.dg/warn/Wmismatched-new-delete-5.C: New test. * g++.dg/warn/Wstringop-overflow-7.C: New test. * g++.dg/warn/Wfree-nonheap-object-6.C: New test. * g++.dg/analyzer/placement-new.C: Prune out expected warning. diff --git a/gcc/builtins.c b/gcc/builtins.c index af1fe49bb48..fb0717a0248 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -5159,11 +5159,43 @@ static tree gimple_call_return_array (gimple *stmt, offset_int offrng[2], range_query *rvals) { - if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL) - || gimple_call_num_args (stmt) < 1) + { + /* Check for attribute fn spec to see if the function returns one + of its arguments. */ + attr_fnspec fnspec = gimple_call_fnspec (as_a <gcall *>(stmt)); + unsigned int argno; + if (fnspec.returns_arg (&argno)) + { + offrng[0] = offrng[1] = 0; + return gimple_call_arg (stmt, argno); + } + } + + if (gimple_call_num_args (stmt) < 1) return NULL_TREE;Nit. You've got an unnecessary {} at the outer level of this hunk. OK with the nit fixed.
That's intentional, to minimize the scope of of the two new local variables. Martin
THanks, Jeff
