Bug 82042 - signed integer overflow in ao_ref_init_from_ptr_and_size
Summary: signed integer overflow in ao_ref_init_from_ptr_and_size
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks: ubsan
  Show dependency treegraph
 
Reported: 2017-08-30 19:23 UTC by Martin Sebor
Modified: 2021-12-04 23:50 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 8.0
Last reconfirmed: 2017-09-11 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2017-08-30 19:23:24 UTC
When compiled with today's top of trunk (GCC 8.0) configured for x86_64-linux --with-build-config=bootstrap-ubsan the following test case triggers a runtime error in the ao_ref_init_from_ptr_and_size() function in tree-ssa-alias.c (besides a number of others).

$ cat t.c && gcc -O2 -S -Wall -ftracer t.c

char *p;

extern char a[];

void f (int i)
{
  __SIZE_TYPE__ idx = __SIZE_MAX__ / 2 - 1;
  p = __builtin_stpcpy (&a[idx], i ? "123" : "12345");
}

/src/gcc/git/gcc/tree-ssa-alias.c:704:30: runtime error: signed integer overflow: 9223372036854775806 * 8 cannot be represented in type 'long int'
/src/gcc/git/gcc/alias.c:2583:21: runtime error: signed integer overflow: -9223372036854775806 - 9223372036854775806 cannot be represented in type 'long int'
/src/gcc/git/gcc/cse.c:2195:10: runtime error: signed integer overflow: -9223372036854775805 - 9223372036854775806 cannot be represented in type 'long int'
/src/gcc/git/gcc/dse.c:932:38: runtime error: signed integer overflow: 9223372036854775806 + 4 cannot be represented in type 'long int'
/src/gcc/git/gcc/dse.c:1539:28: runtime error: signed integer overflow: 4 + 9223372036854775806 cannot be represented in type 'long int'
Comment 1 Martin Sebor 2017-08-30 19:24:34 UTC
The code is valid but has undefined behavior, thus ice-on-valid-code.
Comment 2 Martin Liška 2017-09-11 09:20:57 UTC
Confirmed, I've got patch for 3/4 of ubsan errors.

The only one which is remaining is:

   679  void
   680  ao_ref_init_from_ptr_and_size (ao_ref *ref, tree ptr, tree size)
   681  {
   682    HOST_WIDE_INT t, size_hwi, extra_offset = 0;
   683    ref->ref = NULL_TREE;
   684    if (TREE_CODE (ptr) == SSA_NAME)
   685      {
   686        gimple *stmt = SSA_NAME_DEF_STMT (ptr);
   687        if (gimple_assign_single_p (stmt)
   688            && gimple_assign_rhs_code (stmt) == ADDR_EXPR)
   689          ptr = gimple_assign_rhs1 (stmt);
   690        else if (is_gimple_assign (stmt)
   691                 && gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR
   692                 && TREE_CODE (gimple_assign_rhs2 (stmt)) == INTEGER_CST)
   693          {
   694            ptr = gimple_assign_rhs1 (stmt);
   695            extra_offset = BITS_PER_UNIT
   696                           * int_cst_value (gimple_assign_rhs2 (stmt));
   697          }
   698      }
   699  
   700    if (TREE_CODE (ptr) == ADDR_EXPR)
   701      {
   702        ref->base = get_addr_base_and_unit_offset (TREE_OPERAND (ptr, 0), &t);
   703        if (ref->base)
   704          ref->offset = BITS_PER_UNIT * t;
   705        else

Where offset should be probably offset_int type, which is not for free.
Or do we have a special value for such case Richi?
Comment 3 rguenther@suse.de 2017-09-12 12:50:13 UTC
On Mon, 11 Sep 2017, marxin at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82042
> 
> Martin Liška <marxin at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>              Status|UNCONFIRMED                 |NEW
>    Last reconfirmed|                            |2017-09-11
>                  CC|                            |marxin at gcc dot gnu.org,
>                    |                            |rguenth at gcc dot gnu.org
>            Assignee|unassigned at gcc dot gnu.org      |marxin at gcc dot gnu.org
>      Ever confirmed|0                           |1
> 
> --- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
> Confirmed, I've got patch for 3/4 of ubsan errors.
> 
> The only one which is remaining is:
> 
>    679  void
>    680  ao_ref_init_from_ptr_and_size (ao_ref *ref, tree ptr, tree size)
>    681  {
>    682    HOST_WIDE_INT t, size_hwi, extra_offset = 0;
>    683    ref->ref = NULL_TREE;
>    684    if (TREE_CODE (ptr) == SSA_NAME)
>    685      {
>    686        gimple *stmt = SSA_NAME_DEF_STMT (ptr);
>    687        if (gimple_assign_single_p (stmt)
>    688            && gimple_assign_rhs_code (stmt) == ADDR_EXPR)
>    689          ptr = gimple_assign_rhs1 (stmt);
>    690        else if (is_gimple_assign (stmt)
>    691                 && gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR
>    692                 && TREE_CODE (gimple_assign_rhs2 (stmt)) == INTEGER_CST)
>    693          {
>    694            ptr = gimple_assign_rhs1 (stmt);
>    695            extra_offset = BITS_PER_UNIT
>    696                           * int_cst_value (gimple_assign_rhs2 (stmt));
>    697          }
>    698      }
>    699  
>    700    if (TREE_CODE (ptr) == ADDR_EXPR)
>    701      {
>    702        ref->base = get_addr_base_and_unit_offset (TREE_OPERAND (ptr, 0),
> &t);
>    703        if (ref->base)
>    704          ref->offset = BITS_PER_UNIT * t;
>    705        else
> 
> Where offset should be probably offset_int type, which is not for free.
> Or do we have a special value for such case Richi?

Yeah, this is a know deficiency in ao_ref 'offset' (and also size and
maxsize).  Blowing up to offset_int isn't really a good idea.  size
and max_size have -1 as "unknown" but offset doesn't really have
such value and "failing" isn't an option for the alias machinery.

I've long thought about making ao_ref byte precision but that loses
bit-level disambiguation we get into with bitfield stores/loads so
I "postponed" that to until we (finally) get bitfield load/store 
lowering...

The issue is long-standing so I think we can just leave it that way...
Comment 4 Martin Liška 2017-09-12 14:19:51 UTC
I see, thanks for clarification. I'm going to send patch for the remaining bits, hopefully I made it in a right way.
Comment 5 Martin Liška 2017-11-21 13:39:45 UTC
Author: marxin
Date: Tue Nov 21 13:39:14 2017
New Revision: 255001

URL: https://gcc.gnu.org/viewcvs?rev=255001&root=gcc&view=rev
Log:
Fix UBSAN errors in dse.c (PR rtl-optimization/82044).

2017-11-21  Martin Liska  <mliska@suse.cz>

	PR rtl-optimization/82044
	PR tree-optimization/82042
	* dse.c (check_mem_read_rtx): Check for overflow.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/dse.c
Comment 6 Martin Liška 2017-11-21 16:03:07 UTC
Author: marxin
Date: Tue Nov 21 16:02:35 2017
New Revision: 255013

URL: https://gcc.gnu.org/viewcvs?rev=255013&root=gcc&view=rev
Log:
Backport r255001

2017-11-21  Martin Liska  <mliska@suse.cz>

	Backport from mainline
	2017-11-21  Martin Liska  <mliska@suse.cz>

	PR rtl-optimization/82044
	PR tree-optimization/82042
	* dse.c (check_mem_read_rtx): Check for overflow.

Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/dse.c
Comment 7 Richard Biener 2017-11-22 09:05:19 UTC
Author: rguenth
Date: Wed Nov 22 09:04:47 2017
New Revision: 255046

URL: https://gcc.gnu.org/viewcvs?rev=255046&root=gcc&view=rev
Log:
2017-11-22  Richard Biener  <rguenther@suse.de>

	Revert
	2017-11-21  Martin Liska  <mliska@suse.cz>
 
	Backport from mainline
	2017-11-21  Martin Liska  <mliska@suse.cz>

	PR rtl-optimization/82044
	PR tree-optimization/82042
	* dse.c (check_mem_read_rtx): Check for overflow.

Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/dse.c
Comment 8 Jakub Jelinek 2017-11-22 09:08:55 UTC
Author: jakub
Date: Wed Nov 22 09:08:23 2017
New Revision: 255048

URL: https://gcc.gnu.org/viewcvs?rev=255048&root=gcc&view=rev
Log:
	PR rtl-optimization/82044
	PR tree-optimization/82042
	* dse.c (record_store): Check for overflow.
	(check_mem_read_rtx): Properly check for overflow if width == -1, call
	add_wild_read instead of clear_rhs_from_active_local_stores on
	overflow and log it into dump_file.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/dse.c