Bug 98625 - UBSAN: gcc/cp/module.cc:977:15: runtime error: left shift of negative value -1
Summary: UBSAN: gcc/cp/module.cc:977:15: runtime error: left shift of negative value -1
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 11.0
: P3 normal
Target Milestone: ---
Assignee: Nathan Sidwell
URL:
Keywords:
Depends on:
Blocks: ubsan
  Show dependency treegraph
 
Reported: 2021-01-11 16:58 UTC by Martin Liška
Modified: 2021-01-21 08:58 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2021-01-11 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Liška 2021-01-11 16:58:18 UTC
Seen with:

$ ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/modules/enum-1_a.C -c -fmodules-ts
$ ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/modules/enum-1_b.C -c -fmodules-ts
...
/home/marxin/Programming/gcc2/gcc/cp/module.cc:977:15: runtime error: left shift of negative value -1
    #0 0xf3402f in bytes_in::wi() /home/marxin/Programming/gcc2/gcc/cp/module.cc:977
    #1 0xfeacc4 in bytes_in::wu() /home/marxin/Programming/gcc2/gcc/cp/module.cc:997
    #2 0xf5fa20 in trees_in::core_vals(tree_node*) /home/marxin/Programming/gcc2/gcc/cp/module.cc:6537
    #3 0xf673c7 in trees_in::tree_node_vals(tree_node*) /home/marxin/Programming/gcc2/gcc/cp/module.cc:7148
    #4 0xf7d365 in trees_in::tree_value() /home/marxin/Programming/gcc2/gcc/cp/module.cc:8996
    #5 0xf7f047 in trees_in::tree_node(bool) /home/marxin/Programming/gcc2/gcc/cp/module.cc:9214
    #6 0xfa7a98 in trees_in::read_enum_def(tree_node*, tree_node*) /home/marxin/Programming/gcc2/gcc/cp/module.cc:12203
    #7 0xfa96dd in trees_in::read_definition(tree_node*) /home/marxin/Programming/gcc2/gcc/cp/module.cc:12401
    #8 0xfbe6df in module_state::read_cluster(unsigned int) /home/marxin/Programming/gcc2/gcc/cp/module.cc:14905
    #9 0xfd59a9 in module_state::load_section(unsigned int, binding_slot*) /home/marxin/Programming/gcc2/gcc/cp/module.cc:18036
    #10 0xfdb7b7 in lazy_load_binding(unsigned int, tree_node*, tree_node*, binding_slot*) /home/marxin/Programming/gcc2/gcc/cp/module.cc:18718
    #11 0x101d434 in name_lookup::search_namespace_only(tree_node*) /home/marxin/Programming/gcc2/gcc/cp/name-lookup.c:918
    #12 0x101f47f in name_lookup::search_unqualified(tree_node*, cp_binding_level*) /home/marxin/Programming/gcc2/gcc/cp/name-lookup.c:1149
    #13 0x1061c02 in lookup_name_1 /home/marxin/Programming/gcc2/gcc/cp/name-lookup.c:7910
    #14 0x1061da7 in lookup_name(tree_node*, LOOK_where, LOOK_want) /home/marxin/Programming/gcc2/gcc/cp/name-lookup.c:7930
    #15 0xd8d60f in lookup_name(tree_node*, LOOK_want) /home/marxin/Programming/gcc2/gcc/cp/name-lookup.h:413
    #16 0x110d10c in cp_parser_lookup_name /home/marxin/Programming/gcc2/gcc/cp/parser.c:29285
    #17 0x10f7949 in cp_parser_class_name /home/marxin/Programming/gcc2/gcc/cp/parser.c:24618
    #18 0x110e73b in cp_parser_constructor_declarator_p /home/marxin/Programming/gcc2/gcc/cp/parser.c:29636
    #19 0x10ccefe in cp_parser_decl_specifier_seq /home/marxin/Programming/gcc2/gcc/cp/parser.c:14988
    #20 0x10c99dd in cp_parser_simple_declaration /home/marxin/Programming/gcc2/gcc/cp/parser.c:14260
    #21 0x10c9937 in cp_parser_block_declaration /home/marxin/Programming/gcc2/gcc/cp/parser.c:14207
    #22 0x10c907a in cp_parser_declaration /home/marxin/Programming/gcc2/gcc/cp/parser.c:14078
    #23 0x10c9381 in cp_parser_toplevel_declaration /home/marxin/Programming/gcc2/gcc/cp/parser.c:14107
    #24 0x109cc1e in cp_parser_translation_unit /home/marxin/Programming/gcc2/gcc/cp/parser.c:4936
    #25 0x116c842 in c_parse_file() /home/marxin/Programming/gcc2/gcc/cp/parser.c:45121
    #26 0x15ae6d5 in c_common_parse_file() /home/marxin/Programming/gcc2/gcc/c-family/c-opts.c:1211
    #27 0x2ccb435 in compile_file /home/marxin/Programming/gcc2/gcc/toplev.c:457
    #28 0x2cd3e17 in do_compile /home/marxin/Programming/gcc2/gcc/toplev.c:2193
    #29 0x2cd441a in toplev::main(int, char**) /home/marxin/Programming/gcc2/gcc/toplev.c:2332
    #30 0x59b3bff in main /home/marxin/Programming/gcc2/gcc/main.c:39
    #31 0x7ffff6ce5151 in __libc_start_main (/lib64/libc.so.6+0x28151)
    #32 0xa82bdd in _start (/home/marxin/Programming/gcc2/objdir/gcc/cc1plus+0xa82bdd)
Comment 1 Nathan Sidwell 2021-01-13 14:06:44 UTC
magic configure: --with-build-config=bootstrap-ubsan
Comment 2 Nathan Sidwell 2021-01-19 19:41:06 UTC
This looks like a ubsan or optimizer bug.  I can't see a -ve shift in the source:
HOST_WIDE_INT
bytes_in::wi ()
{
  HOST_WIDE_INT v = 0;
  if (const char *ptr = read (1))
    {
      v = *ptr & 0xff;
      if (v & 0x80)
	{
	  unsigned bytes = (v >> 4) & 0x7;
	  v &= 0xf;
	  if (v & 0x8)
	    v |= -1 ^ 0x7;
	  if ((ptr = read (++bytes)))
	    while (bytes--)
	      v = (v << 8) | (*ptr++ & 0xff);
	}
      else if (v & 0x40)
	v |= -1 ^ 0x3f;
    }

  return v;
}

only >> 4 and << 8 shifts going on there.
Comment 3 Martin Liška 2021-01-19 20:05:13 UTC
> only >> 4 and << 8 shifts going on there.

The thing is here that -1 is being shifted. That's undefined, you likely want to do shifting in an unsigned type and later cast to an unsigned type.
Comment 4 Jakub Jelinek 2021-01-19 20:10:29 UTC
Note, -1 << x shifts are well defined in C++20 (assuming x < sizeof (int) * CHAR_BIT), but isn't well defined in older C++ versions.  For C++11 .. C++17
in particular, x << y is UB if x < 0 or ((unsigned) x >> (sizeof (int) * CHAR_BIT - 1 - y)) > 1 (in addition to the usual y < 0 or y >= sizeof (int) * CHAR_BIT).
Comment 5 Martin Liška 2021-01-21 08:58:04 UTC
Fixed with:

commit 911f797a9be2dc8ef59f5d5bd6d68baf650b8822
Author: Nathan Sidwell <nathan@acm.org>
Date:   Wed Jan 20 09:21:02 2021 -0800

    c++: Avoid UB in signed shift [PR 98625]
    
    I'd forgotten that left shifting a negative value is UB until C++20.
    Insert some casts to do unsigned shifts.
    
            PT c++/98625
            gcc/cp/
            * module.cc (bytes_in::i, bytes_in::wi): Avoid left shift of
            signed type.

@Nathan: That was very close, s/PT/PR :)