Bug 80350 - UBSAN changes code semantics when -fno-sanitize-recover=undefined is used
Summary: UBSAN changes code semantics when -fno-sanitize-recover=undefined is used
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: sanitizer (show other bugs)
Version: 7.0
: P3 normal
Target Milestone: ---
Assignee: Martin Liška
URL:
Keywords: wrong-code
Depends on:
Blocks: yarpgen
  Show dependency treegraph
 
Reported: 2017-04-07 00:29 UTC by Dmitry Babokin
Modified: 2021-11-01 23:07 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 6.3.1, 7.0.1
Known to fail: 5.4.1
Last reconfirmed: 2017-04-07 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Babokin 2017-04-07 00:29:14 UTC
Top of the trunk, x86_64.

The following test case when compiled with "-fsanitize=undefined -fno-sanitize-recover=undefined -O0" produces incorrect result. Correct result is 1. Incorrect is 0.

> cat f.cpp
#include <stdio.h>
unsigned int x = 3153848182U;
unsigned int y = 0;

void foo() {
  int a(!0 >> !x * 500740718);
  y = a;
}

int main () {
    foo ();
    printf("Result = %u\n", y);
    return 0;
}

> g++ f.cpp -o out -fsanitize=undefined -fno-sanitize-recover=undefined -O0
> ./out
Result = 0
> g++ f.cpp -o out -fsanitize=undefined -fno-sanitize-recover=undefined -O2
> ./out
Result = 1
> g++ f.cpp -o out -O0
> ./out
Result = 1
> g++ f.cpp -o out -O2
> ./out
Result = 1
Comment 1 Richard Biener 2017-04-07 08:23:31 UTC
Confirmed.
Comment 2 Martin Liška 2017-04-07 09:01:48 UTC
Working on that.
Comment 3 Martin Liška 2017-04-07 10:07:33 UTC
I've got patch that I'm going to submit to mailing list soon.
Comment 4 Martin Liška 2017-04-10 07:30:02 UTC
Author: marxin
Date: Mon Apr 10 07:29:29 2017
New Revision: 246799

URL: https://gcc.gnu.org/viewcvs?rev=246799&root=gcc&view=rev
Log:
Evaluate a SAVE_EXPR before an UBSAN check (PR sanitizer/80350).

2017-04-10  Martin Liska  <mliska@suse.cz>

	PR sanitizer/80350
	* c-ubsan.c (ubsan_instrument_shift): Evaluate RHS before
	doing an UBSAN check.
2017-04-10  Martin Liska  <mliska@suse.cz>

	PR sanitizer/80350
	* c-c++-common/ubsan/pr80350.c: New test.

Added:
    trunk/gcc/testsuite/c-c++-common/ubsan/pr80350.c
Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-ubsan.c
    trunk/gcc/testsuite/ChangeLog
Comment 5 Martin Liška 2017-04-10 07:31:56 UTC
Fixed on trunk, queued for active branches.
Comment 6 Martin Liška 2017-05-26 11:27:17 UTC
Author: marxin
Date: Fri May 26 11:26:45 2017
New Revision: 248495

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

2017-05-26  Martin Liska  <mliska@suse.cz>

	Backport from mainline
	2017-04-10  Martin Liska  <mliska@suse.cz>

	PR sanitizer/80350
	* c-ubsan.c (ubsan_instrument_shift): Evaluate RHS before
	doing an UBSAN check.
2017-05-26  Martin Liska  <mliska@suse.cz>

	Backport from mainline
	2017-04-10  Martin Liska  <mliska@suse.cz>

	PR sanitizer/80350
	* c-c++-common/ubsan/pr80350.c: New test.

Added:
    branches/gcc-6-branch/gcc/testsuite/c-c++-common/ubsan/pr80350.c
Modified:
    branches/gcc-6-branch/gcc/c-family/ChangeLog
    branches/gcc-6-branch/gcc/c-family/c-ubsan.c
    branches/gcc-6-branch/gcc/testsuite/ChangeLog
Comment 7 Martin Liška 2017-05-26 11:41:40 UTC
Fixed on GCC 6 branch.
Comment 8 Martin Liška 2017-05-26 13:32:29 UTC
As the patch can't be easily ported to GCC 5, I'm not planning to backport to the branch. Thus closing as resolved.