[PATCH] RE: [cfe-dev] missing return statement for non-void functions in C++

Fri Jul 31 07:35:54 PDT 2015

Hi, I am not sure if we came to a conclusion. Please find attached a patch. It simply removes the two lines that insert an unreachable statement (which cause removal of the return statement). Please note that at -O0 the trap instruction is still generated. Is this something we could live with?

Cheers,

Sjoerd.

From: cfe-dev-bounces at cs.uiuc.edu [mailto:cfe-dev-bounces at cs.uiuc.edu] On Behalf Of Richard Smith
Sent: 29 July 2015 18:07
To: Hal Finkel
Cc: Marshall Clow; cfe-dev at cs.uiuc.edu Developers
Subject: Re: [cfe-dev] missing return statement for non-void functions in C++

On Jul 29, 2015 7:43 AM, "Hal Finkel" <hfinkel at anl.gov> wrote:
>
> ----- Original Message -----
> > From: "David Blaikie" <dblaikie at gmail.com>
> > To: "James Molloy" <james at jamesmolloy.co.uk>
> > Cc: "Marshall Clow" <mclow.lists at gmail.com>, "cfe-dev Developers" <cfe-dev at cs.uiuc.edu>
> > Sent: Wednesday, July 29, 2015 9:15:09 AM
> > Subject: Re: [cfe-dev] missing return statement for non-void functions in C++
> >
> >
> > On Jul 29, 2015 7:06 AM, "James Molloy" < james at jamesmolloy.co.uk >
> > wrote:
> > >
> > > Hi,
> > >
> > > If we're going to emit a trap instruction (and thus create a broken
> > > binary), why don't we error instead?
> >
> > We warn, can't error, because it may be dynamically unreached, in
> > which case the program is valid and we can't reject it.
>
> I think this also explains why this is useful for optimization.
>
>  1. It is a code-size optimization
>  2. By eliminating unreachable control flow, we can remove branches and tests that are not actual necessary
>
> int foo(int x) {
>   if (x > 5) return 2*x;
>   else if (x < 2) return 3 - x;
> }
>
> That having been said, there are other ways to express these things, and the situation often represents an error. I'd be fine with requiring a special flag (-fallow-nonreturning-functions or whatever) in order to put the compiler is a truly confirming mode (similar to the situation with sized delete).

Note that we already have a flag to trap on this: -fsanitize-trap=return. (You may also need -fsanitize=return, I don't remember.) That seems consistent with how we treat most other forms of UB.

>  -Hal
>
> >
> > >
> > > James
> > >
> > > On Wed, 29 Jul 2015 at 15:05 David Blaikie < dblaikie at gmail.com >
> > > wrote:
> > >>
> > >>
> > >> On Jul 29, 2015 2:10 AM, "mats petersson" < mats at planetcatfish.com
> > >> > wrote:
> > >> >
> > >> >
> > >> >
> > >> > On 28 July 2015 at 23:40, Marshall Clow < mclow.lists at gmail.com
> > >> > > wrote:
> > >> >>
> > >> >>
> > >> >>
> > >> >> On Tue, Jul 28, 2015 at 6:14 AM, Sjoerd Meijer <
> > >> >> sjoerd.meijer at arm.com > wrote:
> > >> >>>
> > >> >>> Hi,
> > >> >>>
> > >> >>>
> > >> >>>
> > >> >>> In C++, the undefined behaviour of a missing return statements
> > >> >>> for a non-void function results in not generating the
> > >> >>> function epilogue (unreachable statement is inserted and the
> > >> >>> return statement is optimised away). Consequently, the
> > >> >>> runtime behaviour is that control is never properly returned
> > >> >>> from this function and thus it starts executing “garbage
> > >> >>> instructions”. As this is undefined behaviour, this is
> > >> >>> perfectly fine and according to the spec, and a compile
> > >> >>> warning for this missing return statement is issued. However,
> > >> >>> in C, the behaviour is that a function epilogue is generated,
> > >> >>> i.e. basically by returning uninitialised local variable.
> > >> >>> Codes that rely on this are not beautiful pieces of code, i.e
> > >> >>> are buggy, but it might just be okay if you for example have
> > >> >>> a function that just initialises stuff (and the return value
> > >> >>> is not checked, directly or indirectly); some one might argue
> > >> >>> that not returning from that function might be a bit harsh.
> > >> >>
> > >> >>
> > >> >> I would not be one of those people.
> > >> >
> > >> >
> > >> > Nor me.
> > >> >>
> > >> >>
> > >> >>>
> > >> >>> So this email is to probe if there would be strong resistance
> > >> >>> to follow the C behaviour? I am not yet sure how, but would
> > >> >>> perhaps a compromise be possible/acceptable to make the
> > >> >>> undefined behaviour explicit and also generate the function
> > >> >>> epilogue?
> > >> >>
> > >> >>
> > >> >> "undefined behavior" is exactly that.
> > >> >>
> > >> >> You have no idea what is going to happen; there are no
> > >> >> restrictions on what the code being executed can do.
> > >> >>
> > >> >> "it just might be ok" means on a particular version of a
> > >> >> particular compiler, on a particular architecture and OS, at a
> > >> >> particular optimization level. Change any of those things, and
> > >> >> you can change the behavior.
> > >> >
> > >> >
> > >> > In fact, the "it works kind of as you expected" is the worst
> > >> > kind of UB in my mind. UB that causes a crash, stops or other
> > >> > "directly obvious that this is wrong" are MUCH easier to debug.
> > >> >
> > >> > So make this particular kind of UB explicit by crashing or
> > >> > stopping would be a good thing. Making it explicit by
> > >> > "returning kind of nicely, but not correct return value" is
> > >> > about the worst possible result.
> > >>
> > >> At -O0 clang emits a trap instruction, making it more explicit as
> > >> you suggest. At higher optimization levels it just falls
> > >> through/off.
> > >>
> > >> >
> > >> > --
> > >> > Mats
> > >> >>
> > >> >>
> > >> >> -- Marshall
> > >> >>
> > >> >>
> > >> >> _______________________________________________
> > >> >> cfe-dev mailing list
> > >> >> cfe-dev at cs.uiuc.edu
> > >> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
> > >> >>
> > >> >
> > >> >
> > >> > _______________________________________________
> > >> > cfe-dev mailing list
> > >> > cfe-dev at cs.uiuc.edu
> > >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
> > >> >
> > >>
> > >> _______________________________________________
> > >> cfe-dev mailing list
> > >> cfe-dev at cs.uiuc.edu
> > >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
> >
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
> >
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150731/c1e2b561/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: missingreturn.diff
Type: application/octet-stream
Size: 509 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150731/c1e2b561/attachment.obj>