Bug 80362 - [5 Regression] gcc miscompiles arithmetic with signed char
Summary: [5 Regression] gcc miscompiles arithmetic with signed char
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 7.0.1
: P2 normal
Target Milestone: 5.5
Assignee: Richard Biener
URL:
Keywords: wrong-code
Depends on:
Blocks: yarpgen
  Show dependency treegraph
 
Reported: 2017-04-07 22:29 UTC by Vsevolod Livinskii
Modified: 2021-11-01 23:07 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work: 3.4.6, 4.1.2, 5.4.1, 6.3.1, 7.0.1
Known to fail: 4.3.6, 5.4.0, 6.3.0
Last reconfirmed: 2017-04-08 00:00:00


Attachments
Reproducer. (165 bytes, text/x-csrc)
2017-04-07 22:29 UTC, Vsevolod Livinskii
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vsevolod Livinskii 2017-04-07 22:29:22 UTC
Created attachment 41156 [details]
Reproducer.

GCC produces wrong code for a bunch of different architectures.
Reproducer:
#include <stdio.h>

signed char var_0 = 0;
signed char var_1 = 128; 

void foo () {
    //(signed char)(-(signed char)(128)) / 3 should be equal to (signed char)(-var_1) / 3;
    var_0 = (signed char)(-var_1) / 3;
}

int main () {
    foo ();
    printf("%d\n", var_0);
    return 0;
}
Error:
>$ g++ -O0 -march=broadwell repr.c ; ./a.out
-42
>$ g++ -O3 -march=broadwell repr.c ; ./a.out 
42

GCC version:
rev. 246778
Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/home/vsevolod/workspace/gcc-dev/bin-trunk/libexec/gcc/x86_64-pc-linux-gnu/7.0.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /home/vsevolod/workspace/gcc-dev/trunk/configure --prefix=/home/vsevolod/workspace/gcc-dev/bin-trunk --disable-bootstrap
Thread model: posix
gcc version 7.0.1 20170407 (experimental) (GCC)
Comment 1 Markus Trippelsdorf 2017-04-08 06:17:16 UTC
Even 4.4 calls abort with -O2:

int main() {
  signed char var_0, var_1 = -128;
  var_0 = (signed char)(-var_1) / 3;
  if (var_0 > 0)
    __builtin_abort();
}
Comment 2 Richard Biener 2017-04-09 18:11:08 UTC
Fails with -O0 -fstrict-overflow already.  Folded to

    signed char var_1 = -128;
  var_0 = var_1 / -3;
  if (var_0 > 0)

either extract_muldiv or fold_negate.  Mine.
Comment 3 Richard Biener 2017-04-10 08:51:50 UTC
10205         /* Convert -A / -B to A / B when the type is signed and overflow is
10206            undefined.  */
10207         if ((!INTEGRAL_TYPE_P (type) || TYPE_OVERFLOW_UNDEFINED (type))
10208             && TREE_CODE (arg0) == NEGATE_EXPR
10209             && negate_expr_p (op1))
10210           {

where op1 is 3.  We're folding (signed char) -(unsigned char) var_1 / 3 and
arg0 is - (unsigned char) var_1 but _unsigned_.

Trivial fix:

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c    (revision 246797)
+++ gcc/fold-const.c    (working copy)
@@ -10205,7 +10205,7 @@ fold_binary_loc (location_t loc,
       /* Convert -A / -B to A / B when the type is signed and overflow is
         undefined.  */
       if ((!INTEGRAL_TYPE_P (type) || TYPE_OVERFLOW_UNDEFINED (type))
-         && TREE_CODE (arg0) == NEGATE_EXPR
+         && TREE_CODE (op0) == NEGATE_EXPR
          && negate_expr_p (op1))
        {
          if (INTEGRAL_TYPE_P (type))
Comment 4 Richard Biener 2017-04-10 13:02:24 UTC
Fixed on trunk sofar.
Comment 5 Richard Biener 2017-04-10 13:02:45 UTC
Author: rguenth
Date: Mon Apr 10 13:02:12 2017
New Revision: 246805

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

	PR middle-end/80362
	* fold-const.c (fold_binary_loc): Look at unstripped ops when
	looking for NEGATE_EXPR in -A / -B to A / B folding.

	* gcc.dg/torture/pr80362.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr80362.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog
Comment 6 Richard Biener 2017-05-09 12:27:58 UTC
Author: rguenth
Date: Tue May  9 12:27:24 2017
New Revision: 247790

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

	Backport from mainline
	2017-03-28  Richard Biener  <rguenther@suse.de>

	PR middle-end/80222
	* gimple-fold.c (gimple_fold_indirect_ref): Do not touch
	TYPE_REF_CAN_ALIAS_ALL references.
	* fold-const.c (fold_indirect_ref_1): Likewise.

	* g++.dg/pr80222.C: New testcase.

	2017-04-06  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80262
	* tree-sra.c (build_ref_for_offset): Preserve address-space
	information.
	* tree-ssa-sccvn.c (vn_reference_maybe_forwprop_address):
	Drop useless address-space information on MEM_REF offsets.

	* gcc.target/i386/pr80262.c: New testcase.

	2017-04-03  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80275
	* fold-const.c (split_address_to_core_and_offset): Handle
	POINTER_PLUS_EXPR.

	* g++.dg/opt/pr80275.C: New testcase.

	2017-04-06  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80334
	* tree-ssa-loop-ivopts.c (rewrite_use_address): Properly
	preserve alignment of accesses.

	* g++.dg/torture/pr80334.C: New testcase.

	2017-04-10  Richard Biener  <rguenther@suse.de>

	PR middle-end/80362
	* fold-const.c (fold_binary_loc): Look at unstripped ops when
	looking for NEGATE_EXPR in -A / -B to A / B folding.

	* gcc.dg/torture/pr80362.c: New testcase.

	2017-04-25  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80492
	* alias.c (compare_base_decls): Handle registers with asm
	specification conservatively.

	* gcc.dg/pr80492.c: New testcase.

	2017-04-27  Richard Biener  <rguenther@suse.de>

	PR middle-end/80539
	* tree-chrec.c (chrec_fold_plus_poly_poly): Deal with not
	being in loop-closed SSA form conservatively.
	(chrec_fold_multiply_poly_poly): Likewise.

	* gcc.dg/torture/pr80539.c: New testcase.

Added:
    branches/gcc-6-branch/gcc/testsuite/g++.dg/opt/pr80275.C
    branches/gcc-6-branch/gcc/testsuite/g++.dg/pr80222.C
    branches/gcc-6-branch/gcc/testsuite/g++.dg/torture/pr80334.C
    branches/gcc-6-branch/gcc/testsuite/gcc.dg/pr80492.c
    branches/gcc-6-branch/gcc/testsuite/gcc.dg/torture/pr80362.c
    branches/gcc-6-branch/gcc/testsuite/gcc.dg/torture/pr80539.c
    branches/gcc-6-branch/gcc/testsuite/gcc.target/i386/pr80262.c
Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/alias.c
    branches/gcc-6-branch/gcc/fold-const.c
    branches/gcc-6-branch/gcc/gimple-fold.c
    branches/gcc-6-branch/gcc/testsuite/ChangeLog
    branches/gcc-6-branch/gcc/tree-chrec.c
    branches/gcc-6-branch/gcc/tree-sra.c
    branches/gcc-6-branch/gcc/tree-ssa-loop-ivopts.c
    branches/gcc-6-branch/gcc/tree-ssa-sccvn.c
Comment 7 Richard Biener 2017-09-18 13:15:17 UTC
Author: rguenth
Date: Mon Sep 18 13:14:45 2017
New Revision: 252926

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

	Backport from mainline
	2017-04-10  Richard Biener  <rguenther@suse.de>

	PR middle-end/80362
	* fold-const.c (fold_binary_loc): Look at unstripped ops when
	looking for NEGATE_EXPR in -A / -B to A / B folding.

	* gcc.dg/torture/pr80362.c: New testcase.

	2015-11-25  Richard Biener  <rguenther@suse.de>

	PR middle-end/68528
	* fold-const.c (fold_binary_loc): Do not call negate_expr_p
	on stripped operands.

	* gcc.dg/torture/pr68528.c: New testcase.

	2017-03-27  Richard Biener  <rguenther@suse.de>

	PR middle-end/80171
	* gimple-fold.c (fold_ctor_reference): Properly guard against
	NULL return value from canonicalize_constructor_val.

	* g++.dg/torture/pr80171.C: New testcase.

	2016-06-13  Richard Biener  <rguenther@suse.de>

	PR middle-end/64516
	* fold-const.c (fold_unary_loc): Preserve alignment when
	folding a VIEW_CONVERT_EXPR into a MEM_REF.

	* gcc.dg/align-3.c: New testcase.

Added:
    branches/gcc-5-branch/gcc/testsuite/g++.dg/torture/pr80171.C
    branches/gcc-5-branch/gcc/testsuite/gcc.dg/align-3.c
    branches/gcc-5-branch/gcc/testsuite/gcc.dg/torture/pr68528.c
    branches/gcc-5-branch/gcc/testsuite/gcc.dg/torture/pr80362.c
Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/fold-const.c
    branches/gcc-5-branch/gcc/gimple-fold.c
    branches/gcc-5-branch/gcc/testsuite/ChangeLog
Comment 8 Richard Biener 2017-09-18 13:15:46 UTC
Fixed.
Comment 9 Alexandre Oliva 2019-01-03 04:21:19 UTC
I see the same issue of using arg vs op is present in the reciprocal tests right below the patched ones.  AFAICT it is not possible to trigger the problem in a way that affects the outcome, but...  is there any good reason to keep them different and have people puzzle over the difference?  I think it was not the first time that difference caught my attention and got me distracted trying to reason it out.
Comment 10 Richard Biener 2019-01-03 08:40:03 UTC
(In reply to Alexandre Oliva from comment #9)
> I see the same issue of using arg vs op is present in the reciprocal tests
> right below the patched ones.  AFAICT it is not possible to trigger the
> problem in a way that affects the outcome, but...  is there any good reason
> to keep them different and have people puzzle over the difference?  I think
> it was not the first time that difference caught my attention and got me
> distracted trying to reason it out.

Feel free to patch it.