Created attachment 51887 [details] auto-reduced testcase Compiler output: $ x86_64-pc-linux-gnu-gcc -O2 -fno-tree-ccp -fno-tree-forwprop -fno-tree-fre -g -c -w mcf.ii -wrapper valgrind,-q ==23072== Invalid read of size 8 ==23072== at 0x140040E: hash_map<tree_node*, tree_node*, simple_hashmap_traits<default_hash_traits<tree_node*>, tree_node*> >::put(tree_node* const&, tree_node* const&) [clone .isra.0] (hash-map.h:176) ==23072== by 0x14016B6: ipa_param_body_adjustments::prepare_debug_expressions(tree_node*) (ipa-param-manipulation.c:1283) ==23072== by 0x1400EDB: ipa_param_body_adjustments::prepare_debug_expressions(tree_node*) (ipa-param-manipulation.c:1263) ==23072== by 0x14021D7: ipa_param_body_adjustments::common_initialization(tree_node*, tree_node**, vec<ipa_replace_map*, va_gc, vl_embed>*) (ipa-param-manipulation.c:1461) ==23072== by 0x16C8094: tree_function_versioning(tree_node*, tree_node*, vec<ipa_replace_map*, va_gc, vl_embed>*, ipa_param_adjustments*, bool, bitmap_head*, basic_block_def*) (tree-inline.c:6303) ==23072== by 0x11738AD: cgraph_node::materialize_clone() (cgraphclones.c:1142) ==23072== by 0x1162035: cgraph_node::get_untransformed_body() (cgraph.c:3965) ==23072== by 0x13BE9CB: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:720) ==23072== by 0x13BE9FC: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:715) ==23072== by 0x13BE9FC: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:715) ==23072== by 0x13C08CB: inline_transform(cgraph_node*) (ipa-inline-transform.c:777) ==23072== by 0x1539F15: execute_one_ipa_transform_pass (passes.c:2290) ==23072== by 0x1539F15: execute_all_ipa_transforms(bool) (passes.c:2337) ==23072== Address 0x5abaf68 is 168 bytes inside a block of size 208 free'd ==23072== at 0x484240F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==23072== by 0x1400448: find_slot_with_hash (hash-table.h:967) ==23072== by 0x1400448: hash_map<tree_node*, tree_node*, simple_hashmap_traits<default_hash_traits<tree_node*>, tree_node*> >::put(tree_node* const&, tree_node* const&) [clone .isra.0] (hash-map.h:170) ==23072== by 0x14016B6: ipa_param_body_adjustments::prepare_debug_expressions(tree_node*) (ipa-param-manipulation.c:1283) ==23072== by 0x1400EDB: ipa_param_body_adjustments::prepare_debug_expressions(tree_node*) (ipa-param-manipulation.c:1263) ==23072== by 0x14021D7: ipa_param_body_adjustments::common_initialization(tree_node*, tree_node**, vec<ipa_replace_map*, va_gc, vl_embed>*) (ipa-param-manipulation.c:1461) ==23072== by 0x16C8094: tree_function_versioning(tree_node*, tree_node*, vec<ipa_replace_map*, va_gc, vl_embed>*, ipa_param_adjustments*, bool, bitmap_head*, basic_block_def*) (tree-inline.c:6303) ==23072== by 0x11738AD: cgraph_node::materialize_clone() (cgraphclones.c:1142) ==23072== by 0x1162035: cgraph_node::get_untransformed_body() (cgraph.c:3965) ==23072== by 0x13BE9CB: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:720) ==23072== by 0x13BE9FC: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:715) ==23072== by 0x13BE9FC: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:715) ==23072== by 0x13C08CB: inline_transform(cgraph_node*) (ipa-inline-transform.c:777) ==23072== Block was alloc'd at ==23072== at 0x4844C0F: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==23072== by 0x28070D4: xcalloc (xmalloc.c:164) ==23072== by 0x100E416: data_alloc (hash-table.h:275) ==23072== by 0x100E416: alloc_entries (hash-table.h:711) ==23072== by 0x100E416: hash_table<hash_map<tree_node*, tree_node*, simple_hashmap_traits<default_hash_traits<tree_node*>, tree_node*> >::hash_entry, false, xcallocator>::hash_table(unsigned long, bool, bool, bool, mem_alloc_origin) (hash-table.h:628) ==23072== by 0x1402F5A: hash_map (hash-map.h:142) ==23072== by 0x1402F5A: ipa_param_body_adjustments::ipa_param_body_adjustments(ipa_param_adjustments*, tree_node*, tree_node*, copy_body_data*, tree_node**, vec<ipa_replace_map*, va_gc, vl_embed>*) (ipa-param-manipulation.c:1516) ==23072== by 0x16C8094: tree_function_versioning(tree_node*, tree_node*, vec<ipa_replace_map*, va_gc, vl_embed>*, ipa_param_adjustments*, bool, bitmap_head*, basic_block_def*) (tree-inline.c:6303) ==23072== by 0x11738AD: cgraph_node::materialize_clone() (cgraphclones.c:1142) ==23072== by 0x1162035: cgraph_node::get_untransformed_body() (cgraph.c:3965) ==23072== by 0x13BE9CB: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:720) ==23072== by 0x13BE9FC: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:715) ==23072== by 0x13BE9FC: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:715) ==23072== by 0x13C08CB: inline_transform(cgraph_node*) (ipa-inline-transform.c:777) ==23072== by 0x1539F15: execute_one_ipa_transform_pass (passes.c:2290) ==23072== by 0x1539F15: execute_all_ipa_transforms(bool) (passes.c:2337) ==23072== ==23072== Invalid read of size 8 ==23072== at 0x14016B7: ipa_param_body_adjustments::prepare_debug_expressions(tree_node*) (ipa-param-manipulation.c:1284) ==23072== by 0x1400EDB: ipa_param_body_adjustments::prepare_debug_expressions(tree_node*) (ipa-param-manipulation.c:1263) ==23072== by 0x14021D7: ipa_param_body_adjustments::common_initialization(tree_node*, tree_node**, vec<ipa_replace_map*, va_gc, vl_embed>*) (ipa-param-manipulation.c:1461) ==23072== by 0x16C8094: tree_function_versioning(tree_node*, tree_node*, vec<ipa_replace_map*, va_gc, vl_embed>*, ipa_param_adjustments*, bool, bitmap_head*, basic_block_def*) (tree-inline.c:6303) ==23072== by 0x11738AD: cgraph_node::materialize_clone() (cgraphclones.c:1142) ==23072== by 0x1162035: cgraph_node::get_untransformed_body() (cgraph.c:3965) ==23072== by 0x13BE9CB: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:720) ==23072== by 0x13BE9FC: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:715) ==23072== by 0x13BE9FC: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:715) ==23072== by 0x13C08CB: inline_transform(cgraph_node*) (ipa-inline-transform.c:777) ==23072== by 0x1539F15: execute_one_ipa_transform_pass (passes.c:2290) ==23072== by 0x1539F15: execute_all_ipa_transforms(bool) (passes.c:2337) ==23072== by 0x116D7ED: expand (cgraphunit.c:1827) ==23072== by 0x116D7ED: cgraph_node::expand() (cgraphunit.c:1787) ==23072== Address 0x5abaf68 is 168 bytes inside a block of size 208 free'd ==23072== at 0x484240F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==23072== by 0x1400448: find_slot_with_hash (hash-table.h:967) ==23072== by 0x1400448: hash_map<tree_node*, tree_node*, simple_hashmap_traits<default_hash_traits<tree_node*>, tree_node*> >::put(tree_node* const&, tree_node* const&) [clone .isra.0] (hash-map.h:170) ==23072== by 0x14016B6: ipa_param_body_adjustments::prepare_debug_expressions(tree_node*) (ipa-param-manipulation.c:1283) ==23072== by 0x1400EDB: ipa_param_body_adjustments::prepare_debug_expressions(tree_node*) (ipa-param-manipulation.c:1263) ==23072== by 0x14021D7: ipa_param_body_adjustments::common_initialization(tree_node*, tree_node**, vec<ipa_replace_map*, va_gc, vl_embed>*) (ipa-param-manipulation.c:1461) ==23072== by 0x16C8094: tree_function_versioning(tree_node*, tree_node*, vec<ipa_replace_map*, va_gc, vl_embed>*, ipa_param_adjustments*, bool, bitmap_head*, basic_block_def*) (tree-inline.c:6303) ==23072== by 0x11738AD: cgraph_node::materialize_clone() (cgraphclones.c:1142) ==23072== by 0x1162035: cgraph_node::get_untransformed_body() (cgraph.c:3965) ==23072== by 0x13BE9CB: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:720) ==23072== by 0x13BE9FC: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:715) ==23072== by 0x13BE9FC: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:715) ==23072== by 0x13C08CB: inline_transform(cgraph_node*) (ipa-inline-transform.c:777) ==23072== Block was alloc'd at ==23072== at 0x4844C0F: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==23072== by 0x28070D4: xcalloc (xmalloc.c:164) ==23072== by 0x100E416: data_alloc (hash-table.h:275) ==23072== by 0x100E416: alloc_entries (hash-table.h:711) ==23072== by 0x100E416: hash_table<hash_map<tree_node*, tree_node*, simple_hashmap_traits<default_hash_traits<tree_node*>, tree_node*> >::hash_entry, false, xcallocator>::hash_table(unsigned long, bool, bool, bool, mem_alloc_origin) (hash-table.h:628) ==23072== by 0x1402F5A: hash_map (hash-map.h:142) ==23072== by 0x1402F5A: ipa_param_body_adjustments::ipa_param_body_adjustments(ipa_param_adjustments*, tree_node*, tree_node*, copy_body_data*, tree_node**, vec<ipa_replace_map*, va_gc, vl_embed>*) (ipa-param-manipulation.c:1516) ==23072== by 0x16C8094: tree_function_versioning(tree_node*, tree_node*, vec<ipa_replace_map*, va_gc, vl_embed>*, ipa_param_adjustments*, bool, bitmap_head*, basic_block_def*) (tree-inline.c:6303) ==23072== by 0x11738AD: cgraph_node::materialize_clone() (cgraphclones.c:1142) ==23072== by 0x1162035: cgraph_node::get_untransformed_body() (cgraph.c:3965) ==23072== by 0x13BE9CB: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:720) ==23072== by 0x13BE9FC: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:715) ==23072== by 0x13BE9FC: maybe_materialize_called_clones(cgraph_node*) [clone .isra.0] (ipa-inline-transform.c:715) ==23072== by 0x13C08CB: inline_transform(cgraph_node*) (ipa-inline-transform.c:777) ==23072== by 0x1539F15: execute_one_ipa_transform_pass (passes.c:2290) ==23072== by 0x1539F15: execute_all_ipa_transforms(bool) (passes.c:2337) ==23072== $ x86_64-pc-linux-gnu-gcc -v Using built-in specs. COLLECT_GCC=/repo/gcc-trunk/binary-latest-amd64/bin/x86_64-pc-linux-gnu-gcc COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r12-5555-20211127001619-gf4ed2e3ae7d-checking-yes-rtl-df-extra-nobootstrap-amd64/bin/../libexec/gcc/x86_64-pc-linux-gnu/12.0.0/lto-wrapper Target: x86_64-pc-linux-gnu Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++ --enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra --disable-bootstrap --with-cloog --with-ppl --with-isl --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --target=x86_64-pc-linux-gnu --with-ld=/usr/bin/x86_64-pc-linux-gnu-ld --with-as=/usr/bin/x86_64-pc-linux-gnu-as --disable-libstdcxx-pch --prefix=/repo/gcc-trunk//binary-trunk-r12-5555-20211127001619-gf4ed2e3ae7d-checking-yes-rtl-df-extra-nobootstrap-amd64 Thread model: posix Supported LTO compression algorithms: zlib zstd gcc version 12.0.0 20211127 (experimental) (GCC)
Started with r12-4920-g1ece90ffa9ce63b4.
The second "Invalid read of size 8" can be avoided with the following (untested but correct): diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c index 479c20b3871..ff65dad0971 100644 --- a/gcc/ipa-param-manipulation.c +++ b/gcc/ipa-param-manipulation.c @@ -1280,8 +1280,9 @@ ipa_param_body_adjustments::prepare_debug_expressions (tree dead_ssa) && TREE_CODE (gimple_assign_rhs1 (def)) == SSA_NAME) { tree *d = m_dead_ssa_debug_equiv.get (gimple_assign_rhs1 (def)); + gcc_assert (*d); m_dead_ssa_debug_equiv.put (dead_ssa, *d); - return (*d != NULL_TREE); + return true; } tree val But the first one, at least at this point, is somewhat a mystery to me. It happens within the m_dead_ssa_debug_equiv.put() just before the return... and, if I understand the valgrind output well, it seems that inside that hash_map<tree, tree> its m_table.find_slot_with_hash returned a pointer to a memory the same m_table released before?
(In reply to Martin Jambor from comment #2) > The second "Invalid read of size 8" can be avoided with the following > (untested but correct): > > diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c > index 479c20b3871..ff65dad0971 100644 > --- a/gcc/ipa-param-manipulation.c > +++ b/gcc/ipa-param-manipulation.c > @@ -1280,8 +1280,9 @@ ipa_param_body_adjustments::prepare_debug_expressions > (tree dead_ssa) > && TREE_CODE (gimple_assign_rhs1 (def)) == SSA_NAME) > { > tree *d = m_dead_ssa_debug_equiv.get (gimple_assign_rhs1 (def)); > + gcc_assert (*d); > m_dead_ssa_debug_equiv.put (dead_ssa, *d); > - return (*d != NULL_TREE); > + return true; > } > > tree val > > > But the first one, at least at this point, is somewhat a mystery to > me. It happens within the m_dead_ssa_debug_equiv.put() just before > the return... and, if I understand the valgrind output well, it seems > that inside that hash_map<tree, tree> its m_table.find_slot_with_hash > returned a pointer to a memory the same m_table released before? I think the fix for that is: diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c index 479c20b3871..163af94cde0 100644 --- a/gcc/ipa-param-manipulation.c +++ b/gcc/ipa-param-manipulation.c @@ -1279,9 +1279,10 @@ ipa_param_body_adjustments::prepare_debug_expressions (tree dead_ssa) if (gimple_assign_copy_p (def) && TREE_CODE (gimple_assign_rhs1 (def)) == SSA_NAME) { - tree *d = m_dead_ssa_debug_equiv.get (gimple_assign_rhs1 (def)); - m_dead_ssa_debug_equiv.put (dead_ssa, *d); - return (*d != NULL_TREE); + tree d = *m_dead_ssa_debug_equiv.get (gimple_assign_rhs1 (def)); + gcc_assert (d); + m_dead_ssa_debug_equiv.put (dead_ssa, d); + return true; } tree val What likely happens is that 'tree *d' is a pointer to the hash_map. Then you want to put another item in the same hash_map (m_dead_ssa_debug_equiv.put), it's resized and then the dereference of d happens and it's the invalid read as it points to the map before it was grown (reallocated).
Making the hash_map big enough not to reallocate makes the valgrind complaint go away (of course, this is an experiment, not a suggested fix): diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c index ff65dad0971..a4238b51725 100644 --- a/gcc/ipa-param-manipulation.c +++ b/gcc/ipa-param-manipulation.c @@ -1514,7 +1514,7 @@ ipa_param_body_adjustments vec<ipa_replace_map *, va_gc> *tree_map) : m_adj_params (adjustments->m_adj_params), m_adjustments (adjustments), m_reset_debug_decls (), m_split_modifications_p (false), m_dead_stmts (), - m_dead_ssas (), m_dead_ssa_debug_equiv (), m_dead_stmt_debug_equiv (), + m_dead_ssas (), m_dead_ssa_debug_equiv (128), m_dead_stmt_debug_equiv (), m_fndecl (fndecl), m_id (id), m_oparms (), m_new_decls (), m_new_types (), m_replacements (), m_removed_decls (), m_removed_map (), m_method2func (false)
(In reply to Martin Liška from comment #3) > What likely happens is that 'tree *d' is a pointer to the hash_map. Then you > want to put another item in the same hash_map (m_dead_ssa_debug_equiv.put), > it's resized and then the dereference of d happens and it's the invalid read > as it points to the map before it was grown (reallocated). Stupid me, you are of course correct.
In my defense, even in my old code, in the call m_dead_ssa_debug_equiv.put (dead_ssa, *d) I would expect the load *d to be evaluated before the inline template function put is invoked and it feels strange that it isn't. Anyway, I have proposed your patch on the mailing list: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585665.html
The master branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>: https://gcc.gnu.org/g:b3f60112edcb85b459e60f66c44a55138b1cef49 commit r12-5630-gb3f60112edcb85b459e60f66c44a55138b1cef49 Author: Martin Jambor <mjambor@suse.cz> Date: Tue Nov 30 15:35:18 2021 +0100 ipa-param-manip: Be careful about a reallocating hash_map PR 103449 revealed that when I was storing result of one hash_map lookup into another entry in the hash_map, I was still accessing the entry in the table, which meanwhile could get reallocated, making the accesses invalid-after-free. Fixed with the following, which also simplifies the return statement which must have been true even now. gcc/ChangeLog: 2021-11-29 Martin Liska <mliska@suse.cz> Martin Jambor <mjambor@suse.cz> PR ipa/103449 * ipa-param-manipulation.c (ipa_param_body_adjustments::prepare_debug_expressions): Be careful about hash_map reallocating itself. Simpify a return which always returns true.
Fixed, thanks for reporting.