| Summary: | [8 Regression] AddressSanitizer: heap-use-after-free in strlen_optimize_stmt .././../gcc/tree-ssa-strlen.c:2971 | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Martin Liška <marxin> |
| Component: | tree-optimization | Assignee: | Jakub Jelinek <jakub> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | dcb314, msebor |
| Priority: | P3 | Keywords: | ice-on-valid-code, needs-bisection, needs-reduction |
| Version: | 8.0 | ||
| Target Milestone: | 8.0 | ||
| See Also: | https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81117 | ||
| Host: | Target: | ||
| Build: | Known to work: | ||
| Known to fail: | Last reconfirmed: | 2017-11-14 00:00:00 | |
| Attachments: |
test-case
gcc8-pr82977.patch |
||
|
Description
Martin Liška
2017-11-13 21:53:21 UTC
Created attachment 42595 [details]
test-case
I don't have a sanitizer build but I think it's safe to confirm there's something wrong based on the Valgrind output below. $ gcc -O2 -S -Wall pr82977.i -wrapper valgrind ==10920== Memcheck, a memory error detector ==10920== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al. ==10920== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info ==10920== Command: /ssd/build/gcc-git/gcc/cc1 -fpreprocessed pr82977.i -quiet -dumpbase pr82977.i -mtune=generic -march=x86-64 -auxbase pr82977 -O2 -Wall -o pr82977.s ==10920== In file included from cp-demangle.c:140: /home/marxin/BIG/buildbot/slave/gcc-master-bootstrap-asan/build/libstdc++-v3/../libiberty/cp-demangle.h:184:1: warning: ‘cplus_demangle_mangled_name’ declared ‘static’ but never defined [-Wunused-function] /home/marxin/BIG/buildbot/slave/gcc-master-bootstrap-asan/build/libstdc++-v3/../libiberty/cp-demangle.h:188:1: warning: ‘cplus_demangle_type’ declared ‘static’ but never defined [-Wunused-function] cp-demangle.c:4323:1: warning: ‘d_print’ defined but not used [-Wunused-function] ==10920== Invalid read of size 8 ==10920== at 0x101C826: hash_map<tree_node*, std::pair<int, unsigned int>, simple_hashmap_traits<default_hash_traits<tree_node*>, std::pair<int, unsigned int> > >::put(tree_node* const&, std::pair<int, unsigned int> const&) (hash-map.h:142) ==10920== by 0x101B8DF: strlen_optimize_stmt(gimple_stmt_iterator*) (tree-ssa-strlen.c:2972) ==10920== by 0x101BFA8: strlen_dom_walker::before_dom_children(basic_block_def*) (tree-ssa-strlen.c:3138) ==10920== by 0x17BB4F9: dom_walker::walk(basic_block_def*) (domwalk.c:308) ==10920== by 0x101C1A5: (anonymous namespace)::pass_strlen::execute(function*) (tree-ssa-strlen.c:3210) ==10920== by 0xCDD02C: execute_one_pass(opt_pass*) (passes.c:2497) ==10920== by 0xCDD37B: execute_pass_list_1(opt_pass*) (passes.c:2586) ==10920== by 0xCDD3AC: execute_pass_list_1(opt_pass*) (passes.c:2587) ==10920== by 0xCDD404: execute_pass_list(function*, opt_pass*) (passes.c:2597) ==10920== by 0x92DF5F: cgraph_node::expand() (cgraphunit.c:2139) ==10920== by 0x92E403: expand_all_functions() (cgraphunit.c:2275) ==10920== by 0x92EE87: symbol_table::compile() (cgraphunit.c:2623) ==10920== Address 0x63cd178 is 136 bytes inside a block of size 208 free'd ==10920== at 0x4C2ED4A: free (vg_replace_malloc.c:530) ==10920== by 0x101D837: xcallocator<hash_map<tree_node*, std::pair<int, unsigned int>, simple_hashmap_traits<default_hash_traits<tree_node*>, std::pair<int, unsigned int> > >::hash_entry>::data_free(hash_map<tree_node*, std::pair<int, unsigned int>, simple_hashmap_traits<default_hash_traits<tree_node*>, std::pair<int, unsigned int> > >::hash_entry*) (hash-table.h:273) ==10920== by 0x101DE6B: hash_table<hash_map<tree_node*, std::pair<int, unsigned int>, simple_hashmap_traits<default_hash_traits<tree_node*>, std::pair<int, unsigned int> > >::hash_entry, xcallocator>::expand() (hash-table.h:765) ==10920== by 0x101D28B: hash_table<hash_map<tree_node*, std::pair<int, unsigned int>, simple_hashmap_traits<default_hash_traits<tree_node*>, std::pair<int, unsigned int> > >::hash_entry, xcallocator>::find_slot_with_hash(tree_node* const&, unsigned int, insert_option) (hash-table.h:879) ==10920== by 0x101C7EE: hash_map<tree_node*, std::pair<int, unsigned int>, simple_hashmap_traits<default_hash_traits<tree_node*>, std::pair<int, unsigned int> > >::put(tree_node* const&, std::pair<int, unsigned int> const&) (hash-map.h:137) ==10920== by 0x101B8DF: strlen_optimize_stmt(gimple_stmt_iterator*) (tree-ssa-strlen.c:2972) ==10920== by 0x101BFA8: strlen_dom_walker::before_dom_children(basic_block_def*) (tree-ssa-strlen.c:3138) ==10920== by 0x17BB4F9: dom_walker::walk(basic_block_def*) (domwalk.c:308) ==10920== by 0x101C1A5: (anonymous namespace)::pass_strlen::execute(function*) (tree-ssa-strlen.c:3210) ==10920== by 0xCDD02C: execute_one_pass(opt_pass*) (passes.c:2497) ==10920== by 0xCDD37B: execute_pass_list_1(opt_pass*) (passes.c:2586) ==10920== by 0xCDD3AC: execute_pass_list_1(opt_pass*) (passes.c:2587) ==10920== Block was alloc'd at ==10920== at 0x4C2FA50: calloc (vg_replace_malloc.c:711) ==10920== by 0x197B203: xcalloc (xmalloc.c:162) ==10920== by 0x101E156: xcallocator<hash_map<tree_node*, std::pair<int, unsigned int>, simple_hashmap_traits<default_hash_traits<tree_node*>, std::pair<int, unsigned int> > >::hash_entry>::data_alloc(unsigned long) (hash-table.h:263) ==10920== by 0x101D765: hash_table<hash_map<tree_node*, std::pair<int, unsigned int>, simple_hashmap_traits<default_hash_traits<tree_node*>, std::pair<int, unsigned int> > >::hash_entry, xcallocator>::alloc_entries(unsigned long) const (hash-table.h:650) ==10920== by 0x101CAD8: hash_table<hash_map<tree_node*, std::pair<int, unsigned int>, simple_hashmap_traits<default_hash_traits<tree_node*>, std::pair<int, unsigned int> > >::hash_entry, xcallocator>::hash_table(unsigned long, bool, bool, mem_alloc_origin) (hash-table.h:586) ==10920== by 0x101C509: hash_map<tree_node*, std::pair<int, unsigned int>, simple_hashmap_traits<default_hash_traits<tree_node*>, std::pair<int, unsigned int> > >::hash_map(unsigned long, bool, bool) (hash-map.h:112) ==10920== by 0x101C3EC: __static_initialization_and_destruction_0(int, int) (tree-ssa-strlen.c:157) ==10920== by 0x101C416: _GLOBAL__sub_I_laststmt (tree-ssa-strlen.c:3235) ==10920== by 0x198A38C: __libc_csu_init (in /ssd/build/gcc-git/gcc/cc1) ==10920== by 0x601438F: (below main) (in /usr/lib64/libc-2.24.so) ==10920== ==10920== ==10920== HEAP SUMMARY: ==10920== in use at exit: 8,264,061 bytes in 11,998 blocks ==10920== total heap usage: 631,542 allocs, 619,544 frees, 289,006,672 bytes allocated ==10920== ==10920== LEAK SUMMARY: ==10920== definitely lost: 2,480 bytes in 24 blocks ==10920== indirectly lost: 720 bytes in 18 blocks ==10920== possibly lost: 0 bytes in 0 blocks ==10920== still reachable: 8,260,861 bytes in 11,956 blocks ==10920== suppressed: 0 bytes in 0 blocks ==10920== Rerun with --leak-check=full to see details of leaked memory ==10920== ==10920== For counts of detected and suppressed errors, rerun with: -v ==10920== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0) Created attachment 42599 [details] gcc8-pr82977.patch Untested fix. The bug is obvious, hash_map::put does: bool put (const Key &k, const Value &v) { hash_entry *e = m_table.find_slot_with_hash (k, Traits::hash (k), INSERT); bool existed = !hash_entry::is_empty (*e); if (!existed) e->m_key = k; e->m_value = v; return existed; } so passing it a reference to a value inside of the hash map is wrong, because if the hash map needs to be reallocated, it will make the reference refer to freed memory. I'll bootstrap/regtest this. In any case, static hash_map<tree, stridx_strlenloc> strlen_to_stridx; is also wrong because it uselessly requires static initialization. See e.g. decl_to_stridxlist_htab next to it, that is a pointer to hash_map instead. Also, because strlen_to_stridx is only useful if warn_stringop_truncation, I think if it is turned into a pointer to hash_map, then it could be left NULL if !warn_stringop_truncation, and all the strlen_to_stridx related stuff only performed if strlen_to_stridx != NULL. Author: jakub Date: Wed Nov 15 08:40:32 2017 New Revision: 254757 URL: https://gcc.gnu.org/viewcvs?rev=254757&root=gcc&view=rev Log: PR tree-optimization/82977 * tree-ssa-strlen.c (strlen_optimize_stmt): Pass a reference to a copy constructed temporary to strlen_to_stridx.put. Modified: trunk/gcc/ChangeLog trunk/gcc/tree-ssa-strlen.c The regression is fixed, the other issues still need work. |