On 12/8/20 1:46 PM, Martin Sebor wrote:
PR 98160 reports an ICE in pretty printer code called from the newly
added -Wmismatched-new-delete. The ICE is just a simple oversight,
but much more interesting is the warning issued for the test case.
It highlights an issue I didn't consider in the initial implementation:
that inlining one of a pair of allocation/deallocation functions but
not the other might lead to false positives when the inlined function
calls another allocator that the deallocator isn't associated with.
In addition, tests for the changes exposed the overly simplistic
nature of the detection of calls to mismatched forms of operator
new and delete which fails to consider member operators, also
resulting in false positives.
Finally, in a comment on the initial implementation Jonathan notes
that the -Wmismatched-new-delete warning should trigger not only
in user code but also in libstdc++ functions inlined into user code.
I thought I had done that but as it turns out, the "standard code
sequence" I put in place isn't sufficient to make this work.
I forgot to mention one other issue: the initial implementation is
also susceptible to false positives for calls to __builtin_free (and
__builtin_realloc) when the library function (i.e., free or realloc)
was associated with an allocator. The patch also avoids those by
making the built-in handling more robust. Since Glibc headers are
not allowed to declare symbols from other headers (e.g., <stdio.h>
is not allowed to declare free()), referring to the __builtin_xxx
forms of the functions might be the only way to associate, say,
tempnam with free. I'm hoping to add this for the next Glibc
release.
The attached changes avoid the false positives a) by ignoring (with
a warning) the new form of the malloc attribute on inline functions,
and disabling the inlining of others by implicitly adding attribute
noinline to their declaration, and b) by making more robust
the detection of mismatched operators new and delete. Furthermore,
the patch also arranges for the warning to trigger even for inlined
calls to functions defined in system headers.
To make review a little (marginally) easier the change are two files:
1) gcc-98166-1.diff: introduces valid_new_delete_pair_p and
tree_inlined_location.
2) gcc-98166-2.diff: adjusts the atrribute/warning implementation .
Tested on x86_64-linux.
Martin
