[Bug c/99797] New: accessing uninitialized automatic variables

[Bug c/99797] New: accessing uninitialized automatic variables

[muecker at gwdg dot de]

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

            Bug ID: 99797
           Summary: accessing uninitialized automatic variables
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: muecker at gwdg dot de
  Target Milestone: ---

Consider the following example which accesses an uninitialized variable:

static volatile int d = 0;
void bar(int c);
void foo(void)
{
    char c;
    //&c;
    //char *p = &c;

    if (!c)
        bar(0);

    if (d)
        c = 1;

    if (c)
        bar(1);
}


GCC produces code where 'bar' is called twice. According to the C standard, the
code is UB so this is technically OK. Still I think it is dangerous and I would
prefer a more consistent behavior. 

When taking the address and assigning it to 'p' GCC produces code which
actually checks the variable 'd'. I am not sure why this happens (it is
unnecessary). Still when taking the address of 'c' the code is not UB and the
generated code is OK.

Finally, when taking the address and not assigning it, the code is the same as
for the first case where 'bar' is called twice. This seems incorrect as the
code is not UB (according to my reading of the C standard).

Ideally, I think GCC should reject code when it is clearly UB (address not
taken). If it does not reject the code, I think it should assume an unspecified
yet consistent value is read. Producing code that assumes c == 0 and c != 0 at
different points in time (without intervening write) is really dangerous and
should be avoided.

[Bug c/99797] accessing uninitialized automatic variables

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
>Ideally, I think GCC should reject code when it is clearly UB (address not taken).

It can't because it is only undefined at runtime which means if it is never
invoked the undefined behavior will never been seen.

GCC does warn with -Wall which should be good enough for this case:
apinski@xeond:~$ ~/upstream-gcc/bin/gcc  -Wall t77.c -O2 -S
t77.c: In function ‘foo’:
t77.c:9:8: warning: ‘c’ is used uninitialized [-Wuninitialized]
    9 |     if (!c)
      |        ^


>Still when taking the address of 'c' the code is not UB
NO it is still undefined.

[Bug c/99797] accessing uninitialized automatic variables

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|WONTFIX                     |INVALID

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
.

[Bug c/99797] accessing uninitialized automatic variables

[muecker at gwdg dot de]

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

--- Comment #3 from Martin Uecker <muecker at gwdg dot de> ---

The C standard says (6.3.2.1p2)

"... If the lvalue designates an object of automatic storage duration
that could have been declared with the register storage class
(never had its address taken), and that object is uninitialized
(not declared with an initializer and no assignment to it has
been performed prior to use), the behavior is undefined."

[Bug c/99797] accessing uninitialized automatic variables

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Martin Uecker from comment #3)
> The C standard says (6.3.2.1p2)

J.2 Undefined behavior

— The value of an object with automatic storage duration is used while it is
indeterminate (6.2.4, 6.7.9, 6.8).

[Bug c/99797] accessing uninitialized automatic variables

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #4)
> (In reply to Martin Uecker from comment #3)
> > The C standard says (6.3.2.1p2)
> 
> J.2 Undefined behavior
> 
> — The value of an object with automatic storage duration is used while it is
> indeterminate (6.2.4, 6.7.9, 6.8).

6.9p10
If an object that has automatic storage duration is not initialized explicitly,
its value is indeterminate.

Is that enough?

[Bug c/99797] accessing uninitialized automatic variables

[muecker at gwdg dot de]

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

--- Comment #6 from Martin Uecker <muecker at gwdg dot de> ---


Annex J is not normative. The normative text is the one I cited and that
mentions "address taken".  Reading an indeterminate values is not UB if it is
not a trap representation and then yield an unspecified value (at least
according to the normative text of the most recent ISO C standard). But the
standard is up to interpretation and may very well change. This is not my main
point.


My main point is that the behavior is dangerous because it breaks basic logical
assumptions about the code. So even if this were UB I think it is not ideal.

[Bug c/99797] accessing uninitialized automatic variables

[muecker at gwdg dot de]

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

Martin Uecker <muecker at gwdg dot de> changed:

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

--- Comment #7 from Martin Uecker <muecker at gwdg dot de> ---
This is not resolved and 

1) still a code generation bug according to the standard text
2) really dangerous

[Bug middle-end/99797] accessing uninitialized automatic variables

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |middle-end

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I read the standard fully differently but I will let someone else make the
decision here.  

About "2) really dangerous"  GCC already treat many other undefined behavior as
undefined that being dangerous is wrong here.  For an example GCC treats signed
integer overflow as undefined and the dangerous part is similar to this case;
though GCC does have a way to detect at runtime if that happens.

[Bug middle-end/99797] accessing uninitialized automatic variables

[muecker at gwdg dot de]

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

--- Comment #9 from Martin Uecker <muecker at gwdg dot de> ---

The behavior of GCC is dangerous as the example in comment #1 show. You can not
reason at all about the generated code. It is not just that the uninitialized
value causes some random choice but it creates situation where seemingly
impossible things can happen. Assume this propagates into another security
relevant function which when analyzed independently appears completely safe,
i.e. maintains some important property by carefully checking its inputs. But
just having an uninitialized read somewhere else compromises the integrity of
the whole program.

Of course, if this is UB than this is technically allowed from the standard's
point of view.  But what the standard allows is one question. What a good
compiler should do in case of undefined behavior is a completely different one.

The "optimize based on the assumption that UB can not happen" philosophy
amplifies even minor programming errors into something dangerous. This, of
course, also applies to other UB (in varying degrees). For signed overflow we
have -fsanitize=signed-integer-overflow which can help detect and mitigate such
errors, e.g. by trapping at run-time. And also this is allowed by UB. 

In case of UB the choice of what to do lies with the compiler, but I think it
is a bug if this choice is unreasonable and does not serve its users well.

[Bug middle-end/99797] accessing uninitialized automatic variables

[vanyacpp at gmail dot com]

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

Ivan Sorokin <vanyacpp at gmail dot com> changed:

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

--- Comment #10 from Ivan Sorokin <vanyacpp at gmail dot com> ---
(Disclaimer: I'm not a GCC developer, I'm just a random guy who reads bugzilla
and
tried making some simple changes to GCC a few times)

(In reply to Martin Uecker from comment #9)
> The behavior of GCC is dangerous as the example in comment #1 show. You can
> not reason at all about the generated code.

My reasoning normally boils down to this: As the program invokes UB therefore
the exact behavior depends on the compiler, the compiler version, the OS and
other factors.

I would like to note that the optimization performed by compiler are not
designed to break user's code. They were designed to optimize some typical
redundancies in programs. It just happened that their combination breaks
unpredictably the code invoking UB.

Normally it is difficult/impossible not to break the code invoking UB
without regressing some optimizations.

Also optimizations performed by compiler change over time, so the exact
result of the breakage inevitably depends on the specific compiler
version.

In theory GCC already has an option that limits the effects of UB: -O0. I
believe this is the only forward-compatible option for that. If we want to
be more precise we can disable only -fno-tree-ccp, but these fine-grained
optimization options changes from one compiler version to another.

> The "optimize based on the assumption that UB can not happen" philosophy
> amplifies even minor programming errors into something dangerous.

Unfortunately this is easier said than done. I far as I know all major
compilers do optimization based on UB. Consider this:

const int PI = 3;

int tau()
{
   return 2 * PI; // can this be folded into 6?
}

GCC folds 2 * PI into 6 even with -O0. This optimization is based on UB.
Because in some other function one can write:

void evil()
{
    const_cast<int&>(PI) = 4;
}

As some usages of PI can be folded and some can be not. The ones that were
folded would see PI = 3, the ones that were not folded would see PI = 4.

One can argue that the constant folding is fundamentally an optimization
based on UB. I believe few optimizations will be left, if we disable all
that rely on UB.

> This, of  course, also applies to other UB (in varying degrees). For signed
> overflow we have -fsanitize=signed-integer-overflow which can help detect and
> mitigate such errors, e.g. by trapping at run-time. And also this is allowed
> by UB. 

> In case of UB the choice of what to do lies with the compiler, but I think it
> is a bug if this choice is unreasonable and does not serve its users well.

Do you have some specific proposal in mind?

Currently a user has these 5 options:
1. Using -O0 suppressing optimizations.
2. Using -fno-tree-ccp suppressing this specific optimization.
3. Using -Wall and relying on warnings.
4. (in theory) Using static analyzer -fanalyzer. It doesn't detect this error
   at the moment, but I believe can be taught detecting this.
5. Using dynamic analyzer like valgrind.

It seems that you find existing options insufficient and want another one.

[Bug middle-end/99797] accessing uninitialized automatic variables

[muecker at gwdg dot de]

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

--- Comment #11 from Martin Uecker <muecker at gwdg dot de> ---
(In reply to Ivan Sorokin from comment #10)

...
> > is a bug if this choice is unreasonable and does not serve its users well.
> 
> Do you have some specific proposal in mind?
> 
> Currently a user has these 5 options:
> 1. Using -O0 suppressing optimizations.
> 2. Using -fno-tree-ccp suppressing this specific optimization.

Optimizations are important, so this is not really an option.

> 3. Using -Wall and relying on warnings.

It is not clear to me that this fully addresses the problem. GCC does not warn
about all possible accesses to uninitialized variables.

> 4. (in theory) Using static analyzer -fanalyzer. It doesn't detect this error
>    at the moment, but I believe can be taught detecting this.

This may be helpful.

> 5. Using dynamic analyzer like valgrind.

This is too expensive for production and also only useful for limited testing.

> It seems that you find existing options insufficient and want another one.

I want the optimizer to assume that uninitialized variables have an unknown but
fixed value. Then one could still optimize almost as well *and* get analyzable
and more benign behavior even when uninitialized variables are accessed.
Optimizers already know how to deal with variables of unknown content, so this
should be fairly easy to implement (maybe I will try).

I would also like something such as -fsanitize=undefined which detects for
uninitialized variables at run-time.

Best,
Martin

[Bug middle-end/99797] accessing uninitialized automatic variables

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

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

--- Comment #12 from Martin Sebor <msebor at gcc dot gnu.org> ---
Sounds like this is effectively the same request as pr70069 for which there's a
POC prototype:
https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565514.html
Let's track the request there and resolve this as a duplicate.

*** This bug has been marked as a duplicate of bug 70069 ***