[Bug c++/66590] New: switch statement: incorrect warning "reaches end of non-void function"

[Bug c++/66590] New: switch statement: incorrect warning "reaches end of non-void function"

[brutus at free dot fr]

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

            Bug ID: 66590
           Summary: switch statement: incorrect warning "reaches end of
                    non-void function"
           Product: gcc
           Version: 4.9.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: brutus at free dot fr
  Target Milestone: ---

Created attachment 35803
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35803&action=edit
code snippet generated erroneous "end of non-void function" warning

If the attached code is compiled with -Wall, gcc versions 4.9.2, 5.1 and others
incorrectly warn about reaching end of non-void function.

this is typically reproduced with:
 /opt/install/gcc-4.9.2/bin/g++ -Wall -c switch.cpp


warning disappears if:
  -optimisation is at least -O1
  -OR declaration of ~A() line 1 is removed
  -OR declaration of variable "a" line 6 is removed
  -OR declaration of variable "tmp" line 9 is removed
  -OR break on line 9 is removed

[Bug c++/66590] switch statement: incorrect warning "reaches end of non-void function"

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Related to bug 20681.  Most likely block_may_fallthru is returning true for the
{} block.

[Bug c++/66590] switch statement: incorrect warning "reaches end of non-void function"

[manu at gcc dot gnu.org]

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

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-06-19
                 CC|                            |manu at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #2 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
The gimple we build is very strange:

            switch (xD.2336) <default: <D.2342>, case 1: <D.2340>>
            {
              <D.2340>:
              {
                struct AD.2324 tmpD.2341;

                try
                  {
                    try
                      {
                        D.2345 = 1;
                        return D.2345;
                      }
                    finally
                      {
                        # USE = anything
                        # CLB = anything
                        __comp_dtor D.2329 (&tmpD.2341);
                      }
                  }
                finally
                  {
                    tmpD.2341 = {CLOBBER};
                  }
              }
              goto <D.2343>;
              <D.2342>:
              D.2345 = 0;
              return D.2345;
            }
            <D.2343>:
          }
        }

That goto doesn't make much sense.
>From gcc-bugs-return-489374-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Jun 19 07:47:32 2015
Return-Path: <gcc-bugs-return-489374-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 116808 invoked by alias); 19 Jun 2015 07:47:31 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 116753 invoked by uid 48); 19 Jun 2015 07:47:25 -0000
From: "ubizjak at gmail dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug target/66593] driver-i386.c: -mtune=native unavailable with non-bootstrapped build of libgccjit with gcc < 5
Date: Fri, 19 Jun 2015 07:47:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: target
X-Bugzilla-Version: 6.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: ubizjak at gmail dot com
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Resolution:
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-66593-4-2OQ2JiFFlw@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-66593-4@http.gcc.gnu.org/bugzilla/>
References: <bug-66593-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-06/txt/msg01706.txt.bz2
Content-length: 2927

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

--- Comment #2 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to David Malcolm from comment #0)
> Currently libgccjit uses -mtune=generic; I'm working on enabling
> -mtune=native for libgccjit.
> 
> However, on i386/x86_64 with a non-bootstrap build using gcc < 5,
> host_detect_local_cpu does nothing.
> 
> This is due to:
> 
> commit b587c12551143c14f023860a1dbdf7316ae71f27
> Author: uros <uros@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date:   Fri Oct 17 06:00:58 2014 +0000
> 
>         * config/i386/cpuid.h (__cpuid): Remove definitions that handle %ebx
>         register in a special way.
>         (__cpuid_count): Ditto.
>         * config/i386/driver-i386.h: Protect with
>         "#if defined(__GNUC__) && (__GNUC__ >= 5 || !defined(__PIC__))".
>         (host_detect_local_cpu): Mention that GCC with non-fixed %ebx
>         is required to compile the function.
>  
> Uros said (in https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01516.html):
> > The only drawback would be that non-bootstrapped build with gcc < 5.0
> > will ignore -march=native, but I think this should be acceptable."
> 
> Jakub said (in https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01520.html):
> > At least on Linux, driver-i386.c should not be built with PIC normally,
> > so at least changing
> > #if __GNUC__ >= 5
> > to
> > #if defined(__GNUC__) && (__GNUC__ >= 5 || !defined(__PIC__))
> > would limit the -march=native change for non-bootstrapped compilers to
> > Darwin only (or what other targets use PIC by default?).
> 
> The jit builds with PIC, and -mtune=native would be highly appropriate for
> jitted-code; so this is a somewhat bigger deal.
> 
> Currently the instructions for building libgccjit recommend
> --disable-bootstrap, so everyone following those on i386/x86_64 with gcc < 5
> as the system compiler is going to get a hobbled -mtune=native.
> 
> That said, the above code runs inside the driver, and libgccjit doesn't yet
> run that part of the driver, so some extra work will be needed by me on top
> of this before we can get it to do better than -mtune=generic.

The core of the problem lies in cpuid.h. The definitions of __cpuid and
__cpuid_count now use "b" register constraint which is not usable with GCC < 5
when -fPIC is used (these versions just declare %ebx as fixed, unallocable PIC
register).

We *really* don't want to litter cpuid.h with a compatibility stuff (there were
horrible bugs when 64bit code used %xchgl with 32bit registers), since cpuid.h
is intended to be used with the just built compiler.

To avoid the troubles, I'd suggest to just document that gcc-5 is needed for
x86 -mtune=native jit and cut away the past. Otherwise, we could introduce some
kind of local cpuid-compat.h to house all the compatibility stuff and hide the
file in some dark corner.
>From gcc-bugs-return-489376-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Jun 19 08:02:09 2015
Return-Path: <gcc-bugs-return-489376-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 27604 invoked by alias); 19 Jun 2015 08:02:09 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 27559 invoked by uid 48); 19 Jun 2015 08:02:04 -0000
From: "glisse at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug tree-optimization/64130] vrp: handle non zero constant divided by range cannot be zero.
Date: Fri, 19 Jun 2015 08:02:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: tree-optimization
X-Bugzilla-Version: 5.0
X-Bugzilla-Keywords: missed-optimization
X-Bugzilla-Severity: enhancement
X-Bugzilla-Who: glisse at gcc dot gnu.org
X-Bugzilla-Status: NEW
X-Bugzilla-Resolution:
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-64130-4-UddanzZaae@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-64130-4@http.gcc.gnu.org/bugzilla/>
References: <bug-64130-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-06/txt/msg01708.txt.bz2
Content-length: 586

https://gcc.gnu.org/bugzilla/show_bug.cgi?idd130

--- Comment #8 from Marc Glisse <glisse at gcc dot gnu.org> ---
(for cmp1 (eq ne)
     cmp2 (lt ge)
 (simplify
  (cmp1 (trunc_div @0 @1) integer_zerop)
  (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
       || VECTOR_INTEGER_TYPE_P (TREE_TYPE (@0)))
   (cmp2 (abs @0) (abs @1)))))

(I assume that abs simplifies to the identity for unsigned)
But if we are going to use A/B anyway, computing two ABS may waste time, so
maybe restrict it to TYPE_UNSIGNED (TREE_TYPE (@0)) || single_use (@2) (where
@2 is the trunc_div) or something like that.

[Bug c++/66590] switch statement: incorrect warning "reaches end of non-void function"

[miyuki at gcc dot gnu.org]

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

Mikhail Maltsev <miyuki at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |larsch at belunktum dot dk

--- Comment #3 from Mikhail Maltsev <miyuki at gcc dot gnu.org> ---
*** Bug 67693 has been marked as a duplicate of this bug. ***

[Bug c++/66590] switch statement: incorrect warning "reaches end of non-void function"

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This is what we have now:

;; Function f (_Z1fi, funcdef_no=0, decl_uid=2362, cgraph_uid=1,
symbol_order=0)

Removing basic block 6
Removing basic block 8
Merging blocks 3 and 4
Removing basic block 11
Merging blocks 12 and 14
Merging blocks 3 and 5
;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2 3 4 5 6 7 8 9
;; 2 succs { 4 3 }
;; 3 succs { 5 }
;; 4 succs { 5 }
;; 5 succs { 8 6 }
;; 6 succs { 7 }
;; 7 succs { 1 }
;; 8 succs { 9 }
;; 9 succs { 1 }
int f (int x)
{
  int finally_tmp.0;
  struct A tmp;
  struct A a;
  int D.2384;

  <bb 2> :
  if (x == 1)
    goto <bb 3>; [INV]
  else
    goto <bb 4>; [INV]

  <bb 3> :
<L0>:
  D.2384 = 1;
  A::~A (&tmp);
  tmp = {CLOBBER};
  finally_tmp.0 = 0;
  goto <bb 5>; [INV]

  <bb 4> :
<L3>:
  D.2384 = 0;
  finally_tmp.0 = 0;

  <bb 5> :
  A::~A (&a);
  if (finally_tmp.0 == 1)
    goto <bb 6>; [INV]
  else
    goto <bb 8>; [INV]

  <bb 6> :
<L6>:

  <bb 7> :
  a = {CLOBBER};
  return;

  <bb 8> :
  a = {CLOBBER};

  <bb 9> :
<L11>:
  return D.2384;

}


Looks like there was a case where we thought finally_tmp.0 could be equal to 1.
 Note this is all internal.

[Bug c++/66590] switch statement: incorrect warning "reaches end of non-void function"

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Oh yes this issue:
case 1: { A tmp; return 1; } break;

[Bug c++/66590] switch statement: incorrect warning "reaches end of non-void function"

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #5)
> Oh yes this issue:
> case 1: { A tmp; return 1; } break;

Which was PR 20681 but that didn't handle this case.

[Bug c++/66590] switch statement: incorrect warning "reaches end of non-void function"

[pinskia at gcc dot gnu.org]

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

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

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

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I have a patch.
diff --git a/gcc/cp/cp-objcp-common.c b/gcc/cp/cp-objcp-common.c
index beef0123b04..43888507b85 100644
--- a/gcc/cp/cp-objcp-common.c
+++ b/gcc/cp/cp-objcp-common.c
@@ -317,6 +317,15 @@ cxx_block_may_fallthru (const_tree stmt)
        return true;
       return block_may_fallthru (ELSE_CLAUSE (stmt));

+    case CLEANUP_STMT:
+      /* Just handle the try/finally cases.  */
+      if (!CLEANUP_EH_ONLY (stmt))
+       {
+         return (block_may_fallthru (CLEANUP_BODY (stmt))
+                 && block_may_fallthru (CLEANUP_EXPR (stmt)));
+       }
+      return true;
+
     default:
       return c_block_may_fallthru (stmt);
     }

[Bug c++/66590] switch statement: incorrect warning "reaches end of non-void function"

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:971df602e0a798fe9c805c3105f4ac80d638a12b

commit r12-3145-g971df602e0a798fe9c805c3105f4ac80d638a12b
Author: Andrew Pinski <apinski@marvell.com>
Date:   Mon Aug 9 18:33:17 2021 -0700

    Fix PR c++/66590: incorrect warning "reaches end of non-void function" for
switch

    So the problem here is there is code in the C++ front-end not to add a
    break statement (to the IR) if the previous block does not fall through.
    The problem is the code which does the check to see if the block
    may fallthrough does not check a CLEANUP_STMT; it assumes it is always
    fall through.  Anyways this adds the code for the case of a CLEANUP_STMT
    that is only for !CLEANUP_EH_ONLY (the try/finally case).

    OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

    gcc/cp/ChangeLog:

            PR c++/66590
            * cp-objcp-common.c (cxx_block_may_fallthru): Handle
            CLEANUP_STMT for the case which will be try/finally.

    gcc/testsuite/ChangeLog:

            PR c++/66590
            * g++.dg/warn/Wreturn-5.C: New test.

[Bug c++/66590] switch statement: incorrect warning "reaches end of non-void function"

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

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