Bug 93052 - Wrong optimizations for pointers: `p == q ? p : q` -> `q`
Summary: Wrong optimizations for pointers: `p == q ? p : q` -> `q`
Status: UNCONFIRMED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks: 93051
  Show dependency treegraph
 
Reported: 2019-12-23 13:10 UTC by Alexander Cherepanov
Modified: 2021-11-21 05:25 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Cherepanov 2019-12-23 13:10:15 UTC
Similar to pr93051.

The optimizer sometimes changes `p == q ? p : q` to `q`. This is wrong when the actual provenance of `p` differs from that of `q`.
There are two forms -- with the actual conditional operator and with the `if` statement.

The ideal example would be constructed with the help of restricted pointers but it's run into a theoretical problem -- see the first testcase in pr92963.
My other examples require two conditionals to eliminate the possibility of UB. Comparison of integers should give stable results, hopefully that would be enough to demonstrate the problem.

Example with the conditional operator and with dead malloc (the wrong optimization seems to be applied before tree-opt):

----------------------------------------------------------------------
#include <stdint.h>
#include <stdlib.h>
#include <stdio.h>

__attribute__((noipa,optnone)) // imagine it in a separate TU
static void *opaque(void *p) { return p; }

int main()
{
    int *q = malloc(sizeof(int));
    opaque(q);
    uintptr_t iq = (uintptr_t)(void *)q;
    free(q);

    int *p = malloc(sizeof(int));
    opaque(p);
    uintptr_t ip = (uintptr_t)(void *)p;

    uintptr_t ir = ip == iq ? ip : iq;
    if (ip == iq) {
        *p = 1;
        *(int *)(void *)ir = 2;
        printf("result: %d\n", *p);
    }
}
----------------------------------------------------------------------
$ gcc -std=c11 -pedantic -Wall -Wextra -Wno-attributes test.c && ./a.out
result: 2
$ gcc -std=c11 -pedantic -Wall -Wextra -Wno-attributes -O3 test.c && ./a.out
result: 1
----------------------------------------------------------------------
gcc x86-64 version: gcc (GCC) 10.0.0 20191223 (experimental)
----------------------------------------------------------------------
Comment 1 Alexander Cherepanov 2019-12-23 13:10:54 UTC
Example with a past-the-end pointer (vrp1, similar to but 93051, comment 0 but this time with PHI):

----------------------------------------------------------------------
#include <stdio.h>

__attribute__((noipa,optnone)) // imagine it in a separate TU
static void *opaque(void *p) { return p; }

static int been_there = 0;

static int *f(int *p, int *q)
{
    if (p == q) {
        been_there = 1;
        return p;
    } else {
        been_there = 0;
        return q;
    }
}

int main()
{
    int x[5];
    int y[1];
    
    int *p = x;
    int *q = y + 1;
    opaque(q);

    int *p1 = opaque(p); // prevents early optimization of x==y+1
    int *r = f(p1, q);
    
    if (been_there) {
        *p = 1;
        *r = 2;
        printf("result: %d\n", *p);
    }
}
----------------------------------------------------------------------
$ gcc -std=c11 -pedantic -Wall -Wextra -Wno-attributes test.c && ./a.out
result: 2
$ gcc -std=c11 -pedantic -Wall -Wextra -Wno-attributes -O3 test.c && ./a.out
test.c: In function ‘main’:
test.c:33:9: warning: array subscript 1 is outside array bounds of ‘int[1]’ [-Warray-bounds]
   33 |         *r = 2;
      |         ^~
test.c:22:9: note: while referencing ‘y’
   22 |     int y[1];
      |         ^
result: 1
----------------------------------------------------------------------
gcc x86-64 version: gcc (GCC) 10.0.0 20191223 (experimental)
----------------------------------------------------------------------
Comment 2 Alexander Cherepanov 2019-12-23 13:11:32 UTC
Example with a dead malloc (phiopt2):

----------------------------------------------------------------------
#include <stdint.h>
#include <stdlib.h>
#include <stdio.h>

__attribute__((noipa,optnone)) // imagine it in a separate TU
static void *opaque(void *p) { return p; }

static int been_there = 0;

static uintptr_t f(uintptr_t ip, uintptr_t iq)
{
    if (ip == iq) {
        been_there = 1;
        return ip;
    } else {
        been_there = 0;
        return iq;
    }
}

int main()
{
    int *q = malloc(sizeof(int));
    opaque(q);
    uintptr_t iq = (uintptr_t)(void *)q;
    free(q);

    int *p = malloc(sizeof(int));
    opaque(p);
    uintptr_t ip = (uintptr_t)(void *)p;

    uintptr_t ir = f(ip, iq);
    if (been_there) {
        *p = 1;
        *(int *)(void *)ir = 2;
        printf("result: %d\n", *p);
    }
}
----------------------------------------------------------------------
$ gcc -std=c11 -pedantic -Wall -Wextra -Wno-attributes test.c && ./a.out
result: 2
$ gcc -std=c11 -pedantic -Wall -Wextra -Wno-attributes -O3 test.c && ./a.out
result: 1
----------------------------------------------------------------------
gcc x86-64 version: gcc (GCC) 10.0.0 20191223 (experimental)
----------------------------------------------------------------------
Comment 3 Andrew Pinski 2019-12-23 13:20:25 UTC
There is a C defect report about these cases.
Comment 4 Alexander Cherepanov 2019-12-23 17:22:48 UTC
Could you please provide a bit more specific reference? If you mean various discussions about C provenance semantics then they are not about these cases. All examples in pr93051 and in this pr fully respect provenance -- it's the compiler who changes the provenance. In some sense dealing with these bugs is a prerequisite for a meaningful discussion of C provenance semantics: it's hard to reason about possible boundaries of provenance when there are problems with cases where provenance is definitely right.
Comment 5 Alexander Cherepanov 2019-12-24 17:26:30 UTC
1. It should be noted that the idea of problems arising from `p == q ? p : q` is from Chung-Kil Hur via bug 65752, comment 15.

2. clang bug -- https://bugs.llvm.org/show_bug.cgi?id=44374.
Comment 6 Richard Biener 2020-01-09 08:45:58 UTC
I think for the integer issue there's an exact dup.

Time to add a meta-bug linking all of them?  All of the issues really point to
the same very fundamental issue - provenance does not affect the actual value
and since optimizing compilers track value equivalence provenance gets
messed up.

provenance is a dead end.