[Bug c/101292] New: recent valgrind error in warning-control.cc

[Bug c/101292] New: recent valgrind error in warning-control.cc

[dcb314 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292

            Bug ID: 101292
           Summary: recent valgrind error in warning-control.cc
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dcb314 at hotmail dot com
  Target Milestone: ---

Recent gcc trunk build does:

/home/dcb/gcc/working.valgrind/./gcc/xgcc -shared-libgcc
-B/home/dcb/gcc/working.valgrind/./gcc -nostdinc++
-L/home/dcb/gcc/working.valgrind/x86_64-pc-linux-gnu/libstdc++-v3/src
-L/home/dcb/gcc/working.valgrind/x86_64-pc-linux-gnu/libstdc++-v3/src/.libs
-L/home/dcb/gcc/working.valgrind/x86_64-pc-linux-gnu/libstdc++-v3/libsupc++/.libs
-B/home/dcb/gcc/results.20210701.valgrind/x86_64-pc-linux-gnu/bin/
-B/home/dcb/gcc/results.20210701.valgrind/x86_64-pc-linux-gnu/lib/ -isystem
/home/dcb/gcc/results.20210701.valgrind/x86_64-pc-linux-gnu/include -isystem
/home/dcb/gcc/results.20210701.valgrind/x86_64-pc-linux-gnu/sys-include    -x
c++-header -nostdinc++ -g -O3 -D_GNU_SOURCE 
-I/home/dcb/gcc/working.valgrind/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu
-I/home/dcb/gcc/working.valgrind/x86_64-pc-linux-gnu/libstdc++-v3/include
-I/home/dcb/gcc/trunk.git/libstdc++-v3/libsupc++  -O2 -g
/home/dcb/gcc/trunk.git/libstdc++-v3/include/precompiled/extc++.h -o
x86_64-pc-linux-gnu/bits/extc++.h.gch/O2g.gch
==29810== Invalid read of size 4
==29810==    at 0x114F9C0: put (hash-map.h:179)
==29810==    by 0x114F9C0: copy_warning<tree_node*, const tree_node*>
(warning-control.cc:194)
==29810==    by 0x114F9C0: copy_warning(tree_node*, tree_node const*)
(warning-control.cc:213)
==29810==    by 0x6C0F5F: cp_fold(tree_node*) (cp-gimplify.c:2718)

This is for git hash 91c771ec8a3b6497. This error did not occur
last time I tried this build with git hash b4e21c80462682c4.

[Bug c/101292] recent valgrind error in warning-control.cc

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-07-02
     Ever confirmed|0                           |1
                 CC|                            |marxin at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Confirmed, I'm reducing that right now.

[Bug middle-end/101292] [12 Regression] recent valgrind error in warning-control.cc

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0
          Component|c                           |middle-end
            Summary|recent valgrind error in    |[12 Regression] recent
                   |warning-control.cc          |valgrind error in
                   |                            |warning-control.cc
                 CC|                            |msebor at gcc dot gnu.org

[Bug middle-end/101292] [12 Regression] recent valgrind error in warning-control.cc

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r12-1804-g65870e75616ee4359d1c13b99be794e6a577bc65, one can see it
with:

diff --git a/gcc/warning-control.cc b/gcc/warning-control.cc
index ec8ed232763..6b1e5f26c8a 100644
--- a/gcc/warning-control.cc
+++ b/gcc/warning-control.cc
@@ -191,6 +191,7 @@ void copy_warning (ToType to, FromType from)
       if (!nowarn_map)
        nowarn_map = xint_hash_map_t::create_ggc (32);

+      gcc_assert (*from_map != 0xa5a5a5a5);
       nowarn_map->put (to_key, *from_map);
       set_no_warning_bit (to, true);
     }

[Bug middle-end/101292] [12 Regression] recent valgrind error in warning-control.cc since r12-1804-g65870e75616ee4359d1c13b99be794e6a577bc65

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
Reduced test-case:

--- /tmp/x.ii ---
struct type_info {
  void operator==(type_info) { ((__name) || (0)); }
  char __name;
};
enum byte : char;
void operator>>(byte, int) { ((0)); }
struct __class_type_info {
  enum __sub_kind {
    __contained_virtual_mask,
    __contained_mask,
    __contained_public
  };
  struct __dyncast_result;
};
__class_type_info::__sub_kind __do_dyncast_base_access;
struct __vmi_class_type_info : __class_type_info {
  int __flags;
  enum { __non_diamond_repeat_mask, __diamond_shaped_mask };
  bool __do_dyncast(__dyncast_result &) const;
};
bool virtual_p(__class_type_info::__sub_kind access_path) {
  (access_path & __class_type_info::__contained_virtual_mask);
  ((access_path & __class_type_info::__contained_public) ==
   __class_type_info::__contained_public);
  ((access_path & __class_type_info::__contained_public) ==
   __class_type_info::__contained_mask);
}
struct __class_type_info::__dyncast_result {
  void *dst_ptr;
  int whole_details;
  __dyncast_result(int);
};
bool __vmi_class_type_info::__do_dyncast(__dyncast_result &result) const {
  result = (0);
  (__do_dyncast_base_access |
   (__non_diamond_repeat_mask | __diamond_shaped_mask));
  (__non_diamond_repeat_mask);
  (0) || (result.dst_ptr) || (0);
  (!(result.whole_details));
  (0) && (0 || (__flags)) || (__flags & __diamond_shaped_mask);
}

$ gcc x.ii -c
x.ii: In function ‘bool virtual_p(__class_type_info::__sub_kind)’:
x.ii:27:1: warning: no return statement in function returning non-void
[-Wreturn-type]
   27 | }
      | ^
x.ii: In member function ‘bool
__vmi_class_type_info::__do_dyncast(__class_type_info::__dyncast_result&)
const’:
x.ii:40:62: internal compiler error: in copy_warning, at warning-control.cc:194
   40 |   (0) && (0 || (__flags)) || (__flags & __diamond_shaped_mask);
      |                                                              ^
0x1508cce void copy_warning<tree_node*, tree_node const*>(tree_node*, tree_node
const*)
        /home/marxin/Programming/gcc/gcc/warning-control.cc:194
0x9e3467 cp_fold
        /home/marxin/Programming/gcc/gcc/cp/cp-gimplify.c:2718
0x9e53f0 cp_fold_maybe_rvalue(tree_node*, bool)
        /home/marxin/Programming/gcc/gcc/cp/cp-gimplify.c:2109
0x9ecebf cp_convert_and_check(tree_node*, tree_node*, int)
        /home/marxin/Programming/gcc/gcc/cp/cvt.c:666
0xbdd417 cp_build_binary_op(op_location_t const&, tree_code, tree_node*,
tree_node*, int)
        /home/marxin/Programming/gcc/gcc/cp/typeck.c:5876
0x97efea build_new_op_1
        /home/marxin/Programming/gcc/gcc/cp/call.c:6760
0x97fb81 build_new_op(op_location_t const&, tree_code, int, tree_node*,
tree_node*, tree_node*, tree_node**, int)
        /home/marxin/Programming/gcc/gcc/cp/call.c:6806
0xbcf21f build_x_binary_op(op_location_t const&, tree_code, tree_node*,
tree_code, tree_node*, tree_code, tree_node**, int)
        /home/marxin/Programming/gcc/gcc/cp/typeck.c:4330
0xae1c44 cp_parser_binary_expression
        /home/marxin/Programming/gcc/gcc/cp/parser.c:10023
0xae244c cp_parser_assignment_expression
        /home/marxin/Programming/gcc/gcc/cp/parser.c:10161
0xae3aea cp_parser_expression
        /home/marxin/Programming/gcc/gcc/cp/parser.c:10331
0xae7188 cp_parser_expression_statement
        /home/marxin/Programming/gcc/gcc/cp/parser.c:12075
0xaf34f6 cp_parser_statement
        /home/marxin/Programming/gcc/gcc/cp/parser.c:11871
0xaf453e cp_parser_statement_seq_opt
        /home/marxin/Programming/gcc/gcc/cp/parser.c:12223
0xaf4630 cp_parser_compound_statement
        /home/marxin/Programming/gcc/gcc/cp/parser.c:12172
0xb12ff7 cp_parser_function_body
        /home/marxin/Programming/gcc/gcc/cp/parser.c:24238
0xb12ff7 cp_parser_ctor_initializer_opt_and_function_body
        /home/marxin/Programming/gcc/gcc/cp/parser.c:24289
0xb14bea cp_parser_function_definition_after_declarator
        /home/marxin/Programming/gcc/gcc/cp/parser.c:30248
0xb1605d cp_parser_function_definition_from_specifiers_and_declarator
        /home/marxin/Programming/gcc/gcc/cp/parser.c:30164
0xb1605d cp_parser_init_declarator
        /home/marxin/Programming/gcc/gcc/cp/parser.c:21810
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

[Bug middle-end/101292] [12 Regression] recent valgrind error in warning-control.cc since r12-1804-g65870e75616ee4359d1c13b99be794e6a577bc65

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=101204

--- Comment #4 from Martin Sebor <msebor at gcc dot gnu.org> ---
Looks like the garbage collection markups either aren't correct for the
hash_map the code uses or aren't working correctly despite r12-1096.  The
problem might be related to pr101204.

[Bug middle-end/101292] [12 Regression] recent valgrind error in warning-control.cc since r12-1804-g65870e75616ee4359d1c13b99be794e6a577bc65

[dcb314 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292

--- Comment #5 from David Binderman <dcb314 at hotmail dot com> ---
I tried out the C++ testsuite and there are dozens of duplicates.

First four are 

./g++.dg/abi/mangle36.C
./g++.dg/abi/mangle40.C
./g++.dg/asan/asan_test.cc
./g++.dg/asan/pr81021.C

It looks like only -O1 is needed.

$ /home/dcb/gcc/results.20211014.valgrind/bin/g++ -c -O1
./g++.dg/abi/mangle36.C
==2656305== Invalid read of size 4
==2656305==    at 0x11AE4E0: put (hash-map.h:179)
==2656305==    by 0x11AE4E0: copy_warning<tree_node*, const tree_node*> (warn
ing-control.cc:209)
==2656305==    by 0x11AE4E0: copy_warning(tree_node*, tree_node const*) (warn
ing-control.cc:228)
==2656305==    by 0x6FEE17: cp_fold(tree_node*) (cp-gimplify.c:2778)

[Bug middle-end/101292] [12 Regression] recent valgrind error in warning-control.cc since r12-1804-g65870e75616ee4359d1c13b99be794e6a577bc65

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Priority|P3                          |P1
   Last reconfirmed|2021-07-02 00:00:00         |2022-1-17
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Re-confirmed.  The bug is simple - put can re-size but from_spec points to the
old contents...

Testing a patch.

[Bug middle-end/101292] [12 Regression] recent valgrind error in warning-control.cc since r12-1804-g65870e75616ee4359d1c13b99be794e6a577bc65

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:1374d4b963a6ac2e0ec1645c09e5162e68b009d6

commit r12-6640-g1374d4b963a6ac2e0ec1645c09e5162e68b009d6
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Jan 17 15:22:11 2022 +0100

    middle-end/101292 - invalid memory access with warning control

    The warning control falls into the C++ trap of using a reference
    to old hashtable contents for a put operation which can end up
    re-allocating that before reading from the old freed referenced to
    source.  Fixed by introducing a temporary.

    2022-01-17  Richard Biener  <rguenther@suse.de>

            PR middle-end/101292
            * diagnostic-spec.c (copy_warning): Make sure to not
            reference old hashtable content on possible resize.
            * warning-control.cc (copy_warning): Likewise.

[Bug middle-end/101292] [12 Regression] recent valgrind error in warning-control.cc since r12-1804-g65870e75616ee4359d1c13b99be794e6a577bc65

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.

[Bug middle-end/101292] [12 Regression] recent valgrind error in warning-control.cc since r12-1804-g65870e75616ee4359d1c13b99be794e6a577bc65

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292

Thomas Schwinge <tschwinge at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tschwinge at gcc dot gnu.org

--- Comment #9 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
(In reply to CVS Commits from comment #7)
>     The warning control falls into the C++ trap of using a reference
>     to old hashtable contents for a put operation which can end up
>     re-allocating that before reading from the old freed referenced to
>     source.  Fixed by introducing a temporary.

What the heck...  I'd so much looked into this code and even refactored it in
context of my earlier commit r12-3498-g6c79057fae6bbb36c4a4fd61c5b7107a16b71b17
"Don't maintain a warning spec for 'UNKNOWN_LOCATION'/'BUILTINS_LOCATION'
[PR101574]" -- but I had not noticed that pre-existing problem.  Thanks,
Richard!


In the general case, this is probably too complex to get a reliable diagnostic
for (say, GCC Analyzer)?

[Bug middle-end/101292] [12 Regression] recent valgrind error in warning-control.cc since r12-1804-g65870e75616ee4359d1c13b99be794e6a577bc65

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292

--- Comment #10 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 17 Jan 2022, tschwinge at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292
> 
> Thomas Schwinge <tschwinge at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |tschwinge at gcc dot gnu.org
> 
> --- Comment #9 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
> (In reply to CVS Commits from comment #7)
> >     The warning control falls into the C++ trap of using a reference
> >     to old hashtable contents for a put operation which can end up
> >     re-allocating that before reading from the old freed referenced to
> >     source.  Fixed by introducing a temporary.
> 
> What the heck...  I'd so much looked into this code and even refactored it in
> context of my earlier commit r12-3498-g6c79057fae6bbb36c4a4fd61c5b7107a16b71b17
> "Don't maintain a warning spec for 'UNKNOWN_LOCATION'/'BUILTINS_LOCATION'
> [PR101574]" -- but I had not noticed that pre-existing problem.  Thanks,
> Richard!

I've only noticed because I (repeatedly...) did the same mistake 
elsewhere.

[Bug middle-end/101292] [12 Regression] recent valgrind error in warning-control.cc since r12-1804-g65870e75616ee4359d1c13b99be794e6a577bc65

[dcb314 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292

--- Comment #11 from David Binderman <dcb314 at hotmail dot com> ---
(In reply to Thomas Schwinge from comment #9)
> (In reply to CVS Commits from comment #7)
> >     The warning control falls into the C++ trap of using a reference
> >     to old hashtable contents for a put operation which can end up
> >     re-allocating that before reading from the old freed referenced to
> >     source.  Fixed by introducing a temporary.
> 
> What the heck...  I'd so much looked into this code and even refactored it
> in context of my earlier commit
> r12-3498-g6c79057fae6bbb36c4a4fd61c5b7107a16b71b17 "Don't maintain a warning
> spec for 'UNKNOWN_LOCATION'/'BUILTINS_LOCATION' [PR101574]" -- but I had not
> noticed that pre-existing problem.  Thanks, Richard!
> 
> 
> In the general case, this is probably too complex to get a reliable
> diagnostic for (say, GCC Analyzer)?

I had a look for the reallocate in put but couldn't see it.

Does someone perhaps have a small chunk of C++ that demonstrates the trap ?

If the analyser doesn't do it already, it ought to.

[Bug middle-end/101292] [12 Regression] recent valgrind error in warning-control.cc since r12-1804-g65870e75616ee4359d1c13b99be794e6a577bc65

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292

--- Comment #12 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 18 Jan 2022, dcb314 at hotmail dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101292
> 
> --- Comment #11 from David Binderman <dcb314 at hotmail dot com> ---
> (In reply to Thomas Schwinge from comment #9)
> > (In reply to CVS Commits from comment #7)
> > >     The warning control falls into the C++ trap of using a reference
> > >     to old hashtable contents for a put operation which can end up
> > >     re-allocating that before reading from the old freed referenced to
> > >     source.  Fixed by introducing a temporary.
> > 
> > What the heck...  I'd so much looked into this code and even refactored it
> > in context of my earlier commit
> > r12-3498-g6c79057fae6bbb36c4a4fd61c5b7107a16b71b17 "Don't maintain a warning
> > spec for 'UNKNOWN_LOCATION'/'BUILTINS_LOCATION' [PR101574]" -- but I had not
> > noticed that pre-existing problem.  Thanks, Richard!
> > 
> > 
> > In the general case, this is probably too complex to get a reliable
> > diagnostic for (say, GCC Analyzer)?
> 
> I had a look for the reallocate in put but couldn't see it.

It's m_table::expand () that will use data_alloc from the allocator
which eventually ends up in xcalloc and ::free for the release.
Everything is in templates so it should be visible to the analyzer.