[Bug c/64185] New: Optimized code gives unexpected results

[Bug c/64185] New: Optimized code gives unexpected results

[zaz at ua7 dot net]

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

            Bug ID: 64185
           Summary: Optimized code gives unexpected results
           Product: gcc
           Version: 4.9.2
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zaz at ua7 dot net

Created attachment 34191
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34191&action=edit
Test code for reproduce problem

Hello 
I found a open-source SIP library sofia-sip-ua work incorrect with GCC 4.7.3
and above. For me it looks like GCC optimization/compilation issue. I have
reproduce some problem on small test application (full code in attachment):

#include <stdio.h>

struct kv_s
{
    int k;
    int v;
};
typedef struct kv_s kv_t;

struct dict_s
{
    kv_t kv1[1];
    kv_t kv2[1];
    kv_t kv3[1];
    kv_t kv4[1];
    kv_t kv5[1];
    kv_t kv6[1];
    kv_t kv7[1];
    kv_t kv8[1];
};
typedef struct dict_s dict_t;

void initDict(dict_t *dict)
{
    dict->kv1[0].k =  1;
    dict->kv1[0].v = -1;

    dict->kv2[0].k =  2;
    dict->kv2[0].v = -2;

    dict->kv3[0].k =  3;
    dict->kv3[0].v = -3;

    dict->kv4[0].k =  4;
    dict->kv4[0].v = -4;

    dict->kv5[0].k =  5;
    dict->kv5[0].v = -5;

    dict->kv6[0].k =  6;
    dict->kv6[0].v = -6;

    dict->kv7[0].k =  7;
    dict->kv7[0].v = -7;

    dict->kv8[0].k =  8;
    dict->kv8[0].v = -8;
}

int searchDict1(dict_t *dict, int key)
{
    int i;
    kv_t *kvs = dict->kv1;
    for(i=0; i<=6; i++)
    {
        if(key == kvs[i].k)
        {
            return kvs[i].v;
        }
    }

    return 0;
}


int searchDict2(dict_t *dict, int key)
{
    int i;
    for(i=0; i<=6; i++)
    {
        if(key == dict->kv1[i].k)
        {
            return dict->kv1[i].v;
        }
    }

    return 0;
}

int main(int argc, const char* argv[])
{
    dict_t dict;
    int res;

    initDict( &dict );

    res = searchDict1(&dict, 4);
    printf("Found1 %i\n", res);

    res = searchDict2(&dict, 4);
    printf("Found2 %i\n", res);

    return 0;
}

Expected otput of this application:
Found1 -4
Found2 -4

But I see next:
$ gcc --version
gcc (Gentoo 4.8.3 p1.1, pie-0.5.9) 4.8.3

$ gcc -O0 -Wall ./test.c -o test-O0 && ./test-O0
Found1 -4
Found2 -4

$ gcc -O2 -Wall ./test.c -o test-O2 && ./test-O2
Found1 -4
Bus error

$ gcc -O2 -fno-aggressive-loop-optimizations -Wall ./test.c -o test-O2-nalo &&
./test-O2-nalo
Found1 -4
Found2 -1

I got similar results on GCC: 4.7.3, 4.8.3 and 4.92

Looks like I reproduced 2 different problems:
1) "Loops do not terminate" which is posted as know issue on
https://gcc.gnu.org/bugs/
2) Something new when build with "-O2 -fno-aggressive-loop-optimizations", code
found incorrect entry.

I agree code looks not clean when I try access a array out of bunds, but a
looks applications use "hack" like this. For my opinion there possible next
solutions for this issue:
1) searchDict2 will return same response as searchDict1 independent of
optimizations.
2) Provide some command line option (like -fno-aggressive-loop-optimizations)
so with this new option + -fno-aggressive-loop-optimizations it work in same
way as searchDict1.
3) Produce a WARNINGS during compilation (if -Wall specify) about possible
logic corrupt during optimizations.

Best Regards
Alex

[Bug c/64185] Optimized code gives unexpected results

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
                 CC|                            |mpolacek at gcc dot gnu.org
         Resolution|---                         |INVALID

--- Comment #1 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
-fsanitize=undefined says
q.c:71:22: runtime error: index 1 out of bounds for type 'kv_t [1]'
q.c:73:20: runtime error: index 3 out of bounds for type 'kv_t [1]'

An out-of-bounds access is not just something that "looks not clean", it is a
bug, you're triggering undefined behavior so all bets are off.  Invalid.

[Bug c/64185] Optimized code gives unexpected results

[zaz at ua7 dot net]

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

--- Comment #2 from zaz at ua7 dot net ---
But this is deliberate out-of-bands. IE in this test application searchDict2
lookup all KV fields in dict structure in loop based on kv1 offset:
kv1 + 0 - this is pointer to kv1
kv1 + 1 - this is pointer to kv2
kv1 + 2 - this is pointer to kv3
etc
All other C compilers (tested on CLANG and Microsoft C compiller) and early
version of GCC (for example 4.3 in CentOS 6) work with this test as expected.
And as I know many C projects (for example asterisk) use similar code for
manage complex structures. For example asterisk have a lot structures like
this:
struct PascalString
{
  int length;
  char data[0];
};
And then just allocate necessary memory block and able access to something
like:
ps.data[100] - if allocated block have enough size.

As I told before original problem was found in opensource library sofia-sip-ua
in code which not changed many years.

In any case you able leave this issue without without any attention - but I
recommend you take a look on this problem a bit closer.

[Bug c/64185] Optimized code gives unexpected results

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to zaz from comment #2)
> But this is deliberate out-of-bands. IE in this test application searchDict2
> lookup all KV fields in dict structure in loop based on kv1 offset:
> kv1 + 0 - this is pointer to kv1
> kv1 + 1 - this is pointer to kv2
> kv1 + 2 - this is pointer to kv3
> etc
> All other C compilers (tested on CLANG and Microsoft C compiller) and early
> version of GCC (for example 4.3 in CentOS 6) work with this test as expected.
> And as I know many C projects (for example asterisk) use similar code for
> manage complex structures. For example asterisk have a lot structures like
> this:
> struct PascalString
> {
>   int length;
>   char data[0];
> };
> And then just allocate necessary memory block and able access to something
> like:
> ps.data[100] - if allocated block have enough size.

This structure will always work.  It is only the case where the array field is
not at the end of the structure which is where it is undefined.

[Bug c/64185] Optimized code gives unexpected results

[zaz at ua7 dot net]

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

--- Comment #4 from zaz at ua7 dot net ---
> > this:
> > struct PascalString
> > {
> >   int length;
> >   char data[0];
> > };
> > And then just allocate necessary memory block and able access to something
> > like:
> > ps.data[100] - if allocated block have enough size.
> 
> This structure will always work.  It is only the case where the array field
> is not at the end of the structure which is where it is undefined.

So why it not work in this case:
struct PascalString
{
  int length;
  char data[0];
  char the_padding[1024];
};

[Bug c/64185] Optimized code gives unexpected results

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to zaz from comment #4)
> > > this:
> > > struct PascalString
> > > {
> > >   int length;
> > >   char data[0];
> > > };
> > > And then just allocate necessary memory block and able access to something
> > > like:
> > > ps.data[100] - if allocated block have enough size.
> > 
> > This structure will always work.  It is only the case where the array field
> > is not at the end of the structure which is where it is undefined.
> 
> So why it not work in this case:
> struct PascalString
> {
>   int length;
>   char data[0];
>   char the_padding[1024];
> };
Because that is what the C standard say.  And also the above code is invalid C.