[Bug rtl-optimization/99680] New: [11 Regression] AddressSanitizer: global-buffer-overflow since g:04b4828c6dd2

[Bug rtl-optimization/99680] New: [11 Regression] AddressSanitizer: global-buffer-overflow since g:04b4828c6dd2

[marxin at gcc dot gnu.org]

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

            Bug ID: 99680
           Summary: [11 Regression] AddressSanitizer:
                    global-buffer-overflow since g:04b4828c6dd2
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: marxin at gcc dot gnu.org
                CC: vmakarov at gcc dot gnu.org
  Target Milestone: ---

Since the revision I see the following ASAN error for:

$ cat /tmp/ice.i
int __negti2_u2;

int __negti2_u() {
  int uu_0_0 = __negti2_u2;
  __int128 w_1 = uu_0_0 > 0;
  return w_1;
}

$ /home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/objdir/./gcc/xgcc
-B/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/objdir/./gcc/ -O2
/tmp/ice.i -c
=================================================================
==5474==ERROR: AddressSanitizer: global-buffer-overflow on address
0x0000049fe0a1 at pc 0x00000152ee4a bp 0x7fffffffb400 sp 0x7fffffffb3f8
READ of size 1 at 0x0000049fe0a1 thread T0
    #0 0x152ee49 in skip_contraint_modifiers
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/lra-constraints.c:3401
    #1 0x153cf3b in process_address_1
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/lra-constraints.c:3470
    #2 0x1544432 in process_address
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/lra-constraints.c:3765
    #3 0x1544432 in curr_insn_transform
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/lra-constraints.c:4080
    #4 0x155681e in lra_constraints(bool)
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/lra-constraints.c:5169
    #5 0x151831e in lra(_IO_FILE*)
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/lra.c:2336
    #6 0x141b206 in do_reload
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/ira.c:5834
    #7 0x141b206 in execute
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/ira.c:6020
    #8 0x177a7f1 in execute_one_pass(opt_pass*)
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/passes.c:2567
    #9 0x177c1e3 in execute_pass_list_1
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/passes.c:2656
    #10 0x177c209 in execute_pass_list_1
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/passes.c:2657
    #11 0x177c27f in execute_pass_list(function*, opt_pass*)
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/passes.c:2667
    #12 0xc4051f in cgraph_node::expand()
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/cgraphunit.c:1830
    #13 0xc43756 in expand_all_functions
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/cgraphunit.c:1998
    #14 0xc43756 in symbol_table::compile()
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/cgraphunit.c:2362
    #15 0xc4c4e6 in symbol_table::compile()
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/cgraphunit.c:2275
    #16 0xc4c4e6 in symbol_table::finalize_compilation_unit()
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/cgraphunit.c:2543
    #17 0x1a638b1 in compile_file
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/toplev.c:482
    #18 0x697a45 in do_compile
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/toplev.c:2201
    #19 0x697a45 in toplev::main(int, char**)
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/toplev.c:2340
    #20 0x6a454a in main
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/main.c:39
    #21 0x7ffff7852b24 in __libc_start_main ../csu/libc-start.c:332
    #22 0x6a584d in _start
(/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/objdir/gcc/cc1+0x6a584d)

0x0000049fe0a1 is located 63 bytes to the left of global variable '*.LC122'
defined in 'insn-output.c' (0x49fe0e0) of size 22
  '*.LC122' is ascii string 'knotw      {%1, %0|%0, %1}'
0x0000049fe0a1 is located 0 bytes to the right of global variable '*.LC121'
defined in 'insn-output.c' (0x49fe0a0) of size 1
  '*.LC121' is ascii string ''
SUMMARY: AddressSanitizer: global-buffer-overflow
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-asan/build/gcc/lra-constraints.c:3401
in skip_contraint_modifiers
Shadow bytes around the buggy address:
  0x000080937bc0: f9 f9 f9 f9 00 00 00 00 00 00 05 f9 f9 f9 f9 f9
  0x000080937bd0: 00 00 00 06 f9 f9 f9 f9 00 00 00 03 f9 f9 f9 f9
  0x000080937be0: 00 00 00 00 00 04 f9 f9 f9 f9 f9 f9 00 01 f9 f9
  0x000080937bf0: f9 f9 f9 f9 00 00 00 00 00 00 02 f9 f9 f9 f9 f9
  0x000080937c00: 00 00 00 00 00 05 f9 f9 f9 f9 f9 f9 00 00 00 06
=>0x000080937c10: f9 f9 f9 f9[01]f9 f9 f9 f9 f9 f9 f9 00 00 06 f9
  0x000080937c20: f9 f9 f9 f9 00 00 06 f9 f9 f9 f9 f9 00 00 06 f9
  0x000080937c30: f9 f9 f9 f9 00 00 06 f9 f9 f9 f9 f9 00 00 00 07
  0x000080937c40: f9 f9 f9 f9 00 00 00 07 f9 f9 f9 f9 00 00 00 07
  0x000080937c50: f9 f9 f9 f9 00 00 00 07 f9 f9 f9 f9 00 00 00 07
  0x000080937c60: f9 f9 f9 f9 00 00 00 07 f9 f9 f9 f9 00 00 00 07
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==5474==ABORTING

The problem is when curr_static_id->operand[nop].constraint is equal to "".

[Bug rtl-optimization/99680] [11 Regression] AddressSanitizer: global-buffer-overflow since g:04b4828c6dd2

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
           Priority|P3                          |P1
   Target Milestone|---                         |11.0
      Known to work|                            |10.2.0
           Keywords|                            |ice-on-valid-code
      Known to fail|                            |11.0
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-03-20

[Bug rtl-optimization/99680] [11 Regression] AddressSanitizer: global-buffer-overflow since g:04b4828c6dd2

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50438
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50438&action=edit
gcc11-pr99680.patch

Untested fix.

[Bug rtl-optimization/99680] [11 Regression] AddressSanitizer: global-buffer-overflow since g:04b4828c6dd2

[vmakarov at gcc dot gnu.org]

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

--- Comment #2 from Vladimir Makarov <vmakarov at gcc dot gnu.org> ---
Sorry for the troubles with my previous patch. I should have not be in hurry to
fix PR99663.

I'll fix it today.  Jakub's patch could be a candidate but I prefer check
constraint[0] on '\0'.

[Bug rtl-optimization/99680] [11 Regression] AddressSanitizer: global-buffer-overflow since g:04b4828c6dd2

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Vladimir Makarov <vmakarov@gcc.gnu.org>:

https://gcc.gnu.org/g:8bf983c71e42d5a9f9df8a7dc436b30cd9da42f5

commit r11-7748-g8bf983c71e42d5a9f9df8a7dc436b30cd9da42f5
Author: Vladimir N. Makarov <vmakarov@redhat.com>
Date:   Sat Mar 20 10:50:03 2021 -0400

    [PR99680] Check empty constraint before using CONSTRAINT_LEN.

    It seems CONSTRAINT_LEN treats constraint '\0' as one having length 1. 
Therefore we
    read after the constraint string.  The patch fixes it.

    gcc/ChangeLog:

            PR rtl-optimization/99680
            * lra-constraints.c (skip_contraint_modifiers): Rename to
skip_constraint_modifiers.
            (process_address_1): Check empty constraint before using
            CONSTRAINT_LEN.

[Bug rtl-optimization/99680] [11 Regression] AddressSanitizer: global-buffer-overflow since g:04b4828c6dd2

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I was worried that letters that introduce multi-letter constraints followed by
'\0' could be a problem too.  Or do we rely on those being dropped already
earlier?
Something like "=B" on x86_64 etc.  In what I've tried it was dropped during
vregs pass though.
And when cn already is CONSTRAINT__UNKNOWN, performing checks whether to set it
to CONSTRAINT__UNKNOWN is just wasted time.

[Bug rtl-optimization/99680] [11 Regression] AddressSanitizer: global-buffer-overflow since g:04b4828c6dd2

[vmakarov at gcc dot gnu.org]

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

--- Comment #5 from Vladimir Makarov <vmakarov at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #4)
> I was worried that letters that introduce multi-letter constraints followed
> by '\0' could be a problem too.  Or do we rely on those being dropped
> already earlier?
> Something like "=B" on x86_64 etc.  In what I've tried it was dropped during
> vregs pass though.
> And when cn already is CONSTRAINT__UNKNOWN, performing checks whether to set
> it to CONSTRAINT__UNKNOWN is just wasted time.

I like more direct approach.  Just looking at CONSTRAINT_LEN.  Multichracter
constraints returns their length, all others (including modifiers and '\0')
returns 1.  Using CONSTRAINT__UNKNOWN adds one more function
(lookup_constraint) in the decision chain.

If somebody uses starting character of multi-character constraint without all
constraint characters, a lot of things will be broken at least in RA.  If this
happens RA will read besides constraint string anyway in other RA code places
and also RA will also consider garbage after the string as additional
constraints and make unwanted reloads. Reading behind constraint string in
process_address_1 would have less serious consequences.

[Bug rtl-optimization/99680] [11 Regression] AddressSanitizer: global-buffer-overflow since g:04b4828c6dd2

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
           Assignee|jakub at gcc dot gnu.org           |vmakarov at gcc dot gnu.org
             Status|ASSIGNED                    |RESOLVED

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.