[Bug middle-end/106080] New: Labels as values triggering -Wdangling-pointer

[Bug middle-end/106080] New: Labels as values triggering -Wdangling-pointer

[david at tarides dot com]

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

            Bug ID: 106080
           Summary: Labels as values triggering -Wdangling-pointer
           Product: gcc
           Version: 12.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: david at tarides dot com
  Target Milestone: ---

Given file1.c:

```
char * caml_instr_base;
void f (void) { return; }
```

and

file2.c:

```
extern char * caml_instr_base;
void f (void);

int main (void) {
lbl_ACC0:
  caml_instr_base = &&lbl_ACC0;
  /* Uncommenting this call suppresses the dangling-pointer warning */
  /*f();*/

  return 0;
}
```

GCC 12.1.0 (run from the published images on Docker Hub) run with `gcc -Wall -c
file1.c file2.c` gives:

file2.c:6:19: warning: storing the address of local variable 'lbl_ACC0' in
'caml_instr_base' [-Wdangling-pointer=]

The message is at least unclear (lbl_ACC0 is a label, not a local variable)
and, further, if the call to f is uncommented then the warning completely
disappears.

[Bug middle-end/106080] Labels as values triggering -Wdangling-pointer

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://github.com/ocaml/oc
                   |                            |aml/issues/11358
                URL|https://github.com/ocaml/oc |
                   |aml/issues/11358            |

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Storing local labels in globals might cause issues as you cannot use computed
gotos to jump to it without undefined results so the warning message should be
changed.

[Bug middle-end/106080] Labels as values triggering -Wdangling-pointer

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-10-22
             Status|UNCONFIRMED                 |NEW

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed, this should change the warning to be correct.
```
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 59a70530600..2c2572dc402 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -4595,6 +4595,27 @@ pass_waccess::check_dangling_stores (basic_block bb,
       if (!is_auto_decl (rhs_ref.ref))
        continue;

+      if (TREE_CODE (rhs_ref.ref) == LABEL_DECL)
+       {
+         auto_diagnostic_group d;
+         location_t loc = gimple_location (stmt);
+         if (warning_at (loc, OPT_Wdangling_pointer_,
+                         "storing the address of local label %qD in %qE",
+                         rhs_ref.ref, lhs))
+           {
+             suppress_warning (stmt, OPT_Wdangling_pointer_);
+
+             location_t loc = DECL_SOURCE_LOCATION (rhs_ref.ref);
+             inform (loc, "%qD declared here", rhs_ref.ref);
+
+             loc = DECL_SOURCE_LOCATION (lhs_ref.ref);
+
+             if (loc != UNKNOWN_LOCATION)
+             inform (loc, "%qE declared here", lhs_ref.ref);
+           }
+         continue;
+       }
+
       auto_diagnostic_group d;
       location_t loc = gimple_location (stmt);
       if (warning_at (loc, OPT_Wdangling_pointer_,


```
The documentation even warns about the use of labels as addresses too:
https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Labels-as-Values.html#Labels-as-Values
`
You may not use this mechanism to jump to code in a different function. If you
do that, totally unpredictable things happen. The best way to avoid this is to
store the label address only in automatic variables and never pass it as an
argument.
`

[Bug middle-end/106080] Labels as values triggering -Wdangling-pointer

[mpolacek at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |mpolacek at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |mpolacek at gcc dot gnu.org

--- Comment #3 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
This came up in kernel: https://bugzilla.kernel.org/show_bug.cgi?id=215851
which I've reduced to 

void
foo (void **failaddr)
{
  *failaddr = ({ __label__ __here; __here: &&__here; });
}

I don't think we should be warning here at all and I have a patch to that
effect.

[Bug middle-end/106080] Labels as values triggering -Wdangling-pointer

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Marek Polacek from comment #3)
> This came up in kernel: https://bugzilla.kernel.org/show_bug.cgi?id=215851
> which I've reduced to 
> 
> void
> foo (void **failaddr)
> {
>   *failaddr = ({ __label__ __here; __here: &&__here; });
> }
> 
> I don't think we should be warning here at all and I have a patch to that
> effect.

Actaully in the above case, there should be a warning as you should not be able
to jump into a statement expression.

[Bug middle-end/106080] Labels as values triggering -Wdangling-pointer

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Marek Polacek from comment #3)
> I don't think we should be warning here at all and I have a patch to that
> effect.

I also disagree with the saying we should not be warning here. Address to
lables are undefined once they leave the function scope (or in the case of
statement expressions; out of the statement expression scope). We even document
that.

https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Labels-as-Values.html#Labels-as-Values

> You may not use this mechanism to jump to code in a different function. If you do that, totally unpredictable things happen. The best way to avoid this is to store the label address only in automatic variables and never pass it as an argument.

We should be warning about this case just because of the above wording in the
documentation.

[Bug middle-end/106080] Labels as values triggering -Wdangling-pointer

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
On the statement expression part, it is documented that jumping into statement
expression is not allowed too:
https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Statement-Exprs.html#Statement-Exprs

>Jumping into a statement expression with goto or using a switch statement outside the statement expression with a case or default label inside the statement expression is not permitted. Jumping into a statement expression with a computed goto (see Labels as Values) has undefined behavior.

[Bug middle-end/106080] Labels as values triggering -Wdangling-pointer

[mpolacek at gcc dot gnu.org]

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

--- Comment #7 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
I know you can't jump into a statement expression but that's not what's
happening here.  The kernel uses this trick to get the current address for
error messages:

#define __this_address ({ __label__ __here; __here: barrier(); &&__here; })

and they will have to disable the warning completely because of it.  The manual
for -Wdangling-pointer also talks about pointers to *objects* and labels aren't
objects.

[Bug middle-end/106080] Labels as values triggering -Wdangling-pointer

[pinskia at gcc dot gnu.org]

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

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Marek Polacek from comment #7)
> I know you can't jump into a statement expression but that's not what's
> happening here.  The kernel uses this trick to get the current address for
> error messages:
> 
> #define __this_address ({ __label__ __here; __here: barrier(); &&__here; })
> 
> and they will have to disable the warning completely because of it.  The
> manual for -Wdangling-pointer also talks about pointers to *objects* and
> labels aren't objects.

But that is undefined at what label or even where label would be located; it
could be located anywhere. The kernel use of this extension should be removed
as it is not using it to get an useful value at all. Maybe we should add
another builtin to get the current PC.

Also can't they turn off the warning inside the statement expression/around the
macro?

[Bug middle-end/106080] Labels as values triggering -Wdangling-pointer

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
It is true that if there is no edge to the label there is no guarantee where
exactly it will wind up, but we make quite a lot of effort to emit it as a
reasonable spot if possible (see NOTE_INSN_DELETED_LABEL etc.).

That said, -Wdangling-pointer* description is quite clear:
Warn about uses of pointers (or C++ references) to objects with automatic
storage duration after their lifetime has ended.

Labels aren't automatic storage duration objects, they are code addresses but
if anything, they could be more compared to static function-scope variables,
-Wdangling-pointer* doesn't warn about those either.

A lot of programs in the wild actually do use &&label for just printing
addresses, not just the kernel, and we shouldn't warn on it in any way,
especially not with completely unrelated warning.  We can warn if we can prove
somebody is actually trying to jump through them into statement expressions and
the like, but that is not what happens here.

The bug is clearly in
/* Return true of X is a DECL with automatic storage duration.  */

static inline bool
is_auto_decl (tree x)
{
  return DECL_P (x) && !DECL_EXTERNAL (x) && !TREE_STATIC (x);
}

That function should be killed and auto_var_p function should be used instead.

[Bug middle-end/106080] Labels as values triggering -Wdangling-pointer

[mpolacek at gcc dot gnu.org]

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

--- Comment #10 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #8)
> (In reply to Marek Polacek from comment #7)
> > I know you can't jump into a statement expression but that's not what's
> > happening here.  The kernel uses this trick to get the current address for
> > error messages:
> > 
> > #define __this_address ({ __label__ __here; __here: barrier(); &&__here; })
> > 
> > and they will have to disable the warning completely because of it.  The
> > manual for -Wdangling-pointer also talks about pointers to *objects* and
> > labels aren't objects.
> 
> But that is undefined at what label or even where label would be located; it
> could be located anywhere. The kernel use of this extension should be
> removed as it is not using it to get an useful value at all. Maybe we should
> add another builtin to get the current PC.

Apparently it's worked for them.  XFS uses __this_address as defined above, the
rest of the kernel uses
#define _RET_IP_                (unsigned long)__builtin_return_address(0)
#define _THIS_IP_  ({ __label__ __here; __here: (unsigned long)&&__here; })

Maybe we should have __builtin_current_pc but we don't.

> Also can't they turn off the warning inside the statement expression/around
> the macro?

That's a question for them.  I continue to think that warning about non-objects
is a flaw and not useful.

[Bug middle-end/106080] Labels as values triggering -Wdangling-pointer

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Marek Polacek <mpolacek@gcc.gnu.org>:

https://gcc.gnu.org/g:d482b20fd346482635a770281a164a09d608b058

commit r13-6060-gd482b20fd346482635a770281a164a09d608b058
Author: Marek Polacek <polacek@redhat.com>
Date:   Tue Feb 14 17:05:44 2023 -0500

    warn-access: wrong -Wdangling-pointer with labels [PR106080]

    -Wdangling-pointer warns when the address of a label escapes.  This
    causes grief in OCaml (<https://github.com/ocaml/ocaml/issues/11358>) as
    well as in the kernel:
    <https://bugzilla.kernel.org/show_bug.cgi?id=215851> because it uses

      #define _THIS_IP_  ({ __label__ __here; __here: (unsigned long)&&__here;
})

    to get the PC.  -Wdangling-pointer is documented to warn about pointers
    to objects.  However, it uses is_auto_decl which checks DECL_P, but DECL_P
    is also true for a label/enumerator/function declaration, none of which is
    an object.  Rather, it should use auto_var_p which correctly checks VAR_P
    and PARM_DECL.

            PR middle-end/106080

    gcc/ChangeLog:

            * gimple-ssa-warn-access.cc (is_auto_decl): Remove.  Use auto_var_p
            instead.

    gcc/testsuite/ChangeLog:

            * c-c++-common/Wdangling-pointer-10.c: New test.
            * c-c++-common/Wdangling-pointer-9.c: New test.

[Bug middle-end/106080] Labels as values triggering -Wdangling-pointer

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Marek Polacek
<mpolacek@gcc.gnu.org>:

https://gcc.gnu.org/g:825a47f1bb9a42df65157d4dc0a11b2c054e97cc

commit r12-9176-g825a47f1bb9a42df65157d4dc0a11b2c054e97cc
Author: Marek Polacek <polacek@redhat.com>
Date:   Tue Feb 14 17:05:44 2023 -0500

    warn-access: wrong -Wdangling-pointer with labels [PR106080]

    -Wdangling-pointer warns when the address of a label escapes.  This
    causes grief in OCaml (<https://github.com/ocaml/ocaml/issues/11358>) as
    well as in the kernel:
    <https://bugzilla.kernel.org/show_bug.cgi?id=215851> because it uses

      #define _THIS_IP_  ({ __label__ __here; __here: (unsigned long)&&__here;
})

    to get the PC.  -Wdangling-pointer is documented to warn about pointers
    to objects.  However, it uses is_auto_decl which checks DECL_P, but DECL_P
    is also true for a label/enumerator/function declaration, none of which is
    an object.  Rather, it should use auto_var_p which correctly checks VAR_P
    and PARM_DECL.

            PR middle-end/106080

    gcc/ChangeLog:

            * gimple-ssa-warn-access.cc (is_auto_decl): Remove.  Use auto_var_p
            instead.

    gcc/testsuite/ChangeLog:

            * c-c++-common/Wdangling-pointer-10.c: New test.
            * c-c++-common/Wdangling-pointer-9.c: New test.

    (cherry picked from commit d482b20fd346482635a770281a164a09d608b058)

[Bug middle-end/106080] Labels as values triggering -Wdangling-pointer

[mpolacek at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #13 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Fixed.