[Bug c/61131] New: [4.8 regression] ARM -Os: incorrect code generation

[Bug c/61131] New: [4.8 regression] ARM -Os: incorrect code generation

[swarren at nvidia dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61131

            Bug ID: 61131
           Summary: [4.8 regression] ARM -Os: incorrect code generation
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: swarren at nvidia dot com

Created attachment 32771
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=32771&action=edit
Source sample that exhibits the issue when compiled

When running gcc 4.8 for ARM with -Os, the following function:

int count_entries(struct s *p)
{
    int i = 0;

    while (p->fd[i] >= 0 && i < MAX_COUNT) {
        i++;
    }

    return i;
}

... gets compiled to essentially a no-op:

000083f8 <count_entries>:
    83f8:    e5900000     ldr    r0, [r0]
    83fc:    e1e00000     mvn    r0, r0
    8400:    e1a00fa0     lsr    r0, r0, #31
    8404:    e12fff1e     bx    lr

If I don't use -Os, then I get much larger, and working, code.

If I swap the order of the two conditions in the while expression, I get
working code:

    while (i < MAX_COUNT && p->fd[i] >= 0) {

If I use gcc-4.7 insteaad, I get working code.

A complete source sample is attached.

The bug is NOT present in
gcc-linaro-arm-linux-gnueabihf-4.7-2013.04-20130415_linux.tar.xz, which is
apparently GCC 4.7.2+svn197188.

The bug IS present in gcc-linaro-arm-linux-gnueabihf-4.8-2014.04_linux.tar.xz,
which is apparently GCC 4.8.3+svn208968.

I wasn't able to quickly track down any more recent gcc-4.8.x binaries, or
gcc-4.9.x binaries.

The bug IS present in some toolchain I received from a customer in directory
name yocto/gcc-4.8.1-glibc-2.18-hard/.

[Bug target/61131] [4.8 regression] ARM -Os: incorrect code generation

[mikpelinux at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61131

Mikael Pettersson <mikpelinux at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikpelinux at gmail dot com

--- Comment #1 from Mikael Pettersson <mikpelinux at gmail dot com> ---
With a bit more context it's easy to see the error:

==snip==
#define MAX_COUNT 2

struct s {
        int fd[MAX_COUNT];
        int status;
        unsigned int bogus;
};

int count_entries(struct s *p);
int count_entries(struct s *p)
{
        int i = 0;

        while (p->fd[i] >= 0 && i < MAX_COUNT) { // bad
        //while (i < MAX_COUNT && p->fd[i] >= 0) { // ok
                i++;
        }
==snip==

If ->fd[0] and ->fd[1] are both non-negative, then i will be incremented to 2,
the "bad" code will access ->fd[2], which is an out-of-bounds error, causing
undefined behaviour.  GCC sees this, "knows" that this cannot happen, and
changes the generated code accordingly with surprising results.  The "ok" code
is correct.

I've seen this bogus pattern in Linux kernel sources.  AFAIK it's been fixed
there now.

[Bug target/61131] [4.8 regression] ARM -Os: incorrect code generation

[rearnsha at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61131

Richard Earnshaw <rearnsha at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #2 from Richard Earnshaw <rearnsha at gcc dot gnu.org> ---
As described, the bounds check has to be applied before dereferencing the
array, not afterwards.

[Bug target/61131] [4.8 regression] ARM -Os: incorrect code generation

[swarren at nvidia dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61131

--- Comment #3 from Stephen Warren <swarren at nvidia dot com> ---
This certainly violates the principle of least-surprise...

Once i==2, "p->fd[i] >= 0" is certainly undefined. Surely "undefined && false"
is false though, since "anything && false" is false; short-circuit evaluation
should surely only apply if "anything" was known to be false, and presumably
"undefined" isn't known to be false. Or is the definition of undefined such
that it propagates through the entire expression irrespective of the
expression's logic? I suppose that could be the case, but it's certainly not
the most useful definition of undefined:-)

[Bug target/61131] [4.8 regression] ARM -Os: incorrect code generation

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61131

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Stephen Warren from comment #3)
> This certainly violates the principle of least-surprise...
> 
> Once i==2, "p->fd[i] >= 0" is certainly undefined. Surely "undefined &&
> false" is false though, since "anything && false" is false; short-circuit
> evaluation should surely only apply if "anything" was known to be false, and
> presumably "undefined" isn't known to be false. Or is the definition of
> undefined such that it propagates through the entire expression irrespective
> of the expression's logic? I suppose that could be the case, but it's
> certainly not the most useful definition of undefined:-)

The undefined basically gets turned into a continue. Also the value is not just
undefined, the effect of doing an out of bounds is undefined and could even
crash.

[Bug target/61131] [4.8 regression] ARM -Os: incorrect code generation

[mikpelinux at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61131

--- Comment #5 from Mikael Pettersson <mikpelinux at gmail dot com> ---
(In reply to Stephen Warren from comment #3)
> Or is the definition of
> undefined such that it propagates through the entire expression irrespective
> of the expression's logic?

It is.  Once execution hits undefined behaviour all bets are off.

"Undefined" is not some unspecified value you can choose to ignore.