https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118024
--- Comment #5 from GCC Commits <cvs-commit at gcc dot gnu.org> --- The master branch has been updated by Jakub Jelinek <ja...@gcc.gnu.org>: https://gcc.gnu.org/g:9537ca5ad9bc23d7e9c446b4a7cbb98f63bddb6a commit r15-6253-g9537ca5ad9bc23d7e9c446b4a7cbb98f63bddb6a Author: Jakub Jelinek <ja...@redhat.com> Date: Sat Dec 14 11:27:20 2024 +0100 warn-access: Fix up matching_alloc_calls_p [PR118024] The following testcase ICEs because of a bug in matching_alloc_calls_p. The loop was apparently meant to be walking the two attribute chains in lock-step, but doesn't really do that. If the first lookup_attribute returns non-NULL, the second one is not done, so rmats in that case can be some random unrelated attribute rather than "malloc" attribute; the body assumes even rmats if non-NULL is "malloc" attribute and relies on its argument to be a "malloc" argument and if it is some other attribute with incompatible attribute, it just crashes. Now, fixing that in the obvious way, instead of doing (amats = lookup_attribute ("malloc", amats)) || (rmats = lookup_attribute ("malloc", rmats)) in the condition do ((amats = lookup_attribute ("malloc", amats)), (rmats = lookup_attribute ("malloc", rmats)), (amats || rmats)) fixes the testcase but regresses Wmismatched-dealloc-{2,3}.c tests. The problem is that walking the attribute lists in a lock-step is obviously a very bad idea, there is no requirement that the same deallocators are present in the same order on both decls, e.g. there could be an extra malloc attribute without argument in just one of the lists, or the order of say free/realloc could be swapped, etc. We don't generally document nor enforce any particular ordering of attributes (even when for some attributes we just handle the first one rather than all). So, this patch instead simply splits it into two loops, the first one walks alloc_decl attributes, the second one walks dealloc_decl attributes. If the malloc attribute argument is a built-in, that doesn't change anything, and otherwise we have the chance to populate the whole common_deallocs hash_set in the first loop and then can check it in the second one (and don't need to use more expensive add method on it, can just check contains there). Not to mention that it also fixes the case when the function would incorrectly return true if there wasn't a common deallocator between the two, but dealloc_decl had 2 malloc attributes with the same deallocator. 2024-12-14 Jakub Jelinek <ja...@redhat.com> PR middle-end/118024 * gimple-ssa-warn-access.cc (matching_alloc_calls_p): Walk malloc attributes of alloc_decl and dealloc_decl in separate loops rather than in lock-step. Use common_deallocs.contains rather than common_deallocs.add in the second loop. * gcc.dg/pr118024.c: New test.
