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

Thu Aug 13 13:50:50 PDT 2015

Please make such programs crash early and often.  They are a nightmare to
maintain.  Make them blow in the face of the original authors; not after
they are gone.

-- Gaby

On Thu, Aug 13, 2015 at 11:18 AM, Richard Smith via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> On Thu, Aug 13, 2015 at 1:52 AM, Sjoerd Meijer <Sjoerd.Meijer at arm.com>
> wrote:
>
>> Hi Richard,
>>
>> Thanks for reviewing. Agree, that was a bit confusing. More specifically,
>>
>> the warning message was confusing (i.e. wrong). This patch is for
>> compiling .c
>>
>> input in C++ mode. The new flag should be ignored for C++ **input**, and
>> indeed
>>
>> not when it is in C++ *mode* as the warning message said earlier. So I
>> have
>>
>> changed the warning message accordingly and hope that solves it, see
>> attached
>>
>> patch.
>>
>
> What is the distinction you're trying to draw here? This patch still
> doesn't make sense to me. This flag is only meaningful when compiling as
> C++. You ignore it when compiling as C but produce a warning that says it's
> ignored when compiling as C++.
>
>
>> Cheers.
>>
>>
>>
>> *From:* metafoo at gmail.com [mailto:metafoo at gmail.com] *On Behalf Of *Richard
>> Smith
>> *Sent:* 12 August 2015 23:06
>> *To:* Sjoerd Meijer
>> *Cc:* Hal Finkel; Marshall Clow; cfe-commits
>>
>> *Subject:* Re: [PATCH] RE: [cfe-dev] missing return statement for
>> non-void functions in C++
>>
>>
>>
>> This patch seems a bit confused. You warn that the flag is ignored in
>> C++, but it only has an effect in C++. You have a testcase with a .c
>> extension that is built with -x c++.
>>
>>
>>
>> On Wed, Aug 12, 2015 at 5:23 AM, Sjoerd Meijer <sjoerd.meijer at arm.com>
>> wrote:
>>
>> [ + cfe-commits at lists.llvm.org ]
>>
>>
>>
>> Hi,
>>
>> The functionality is now available under a flag, see attached patch. Note
>> that the flag is ignored in C++ mode, so it will help the use case of
>> compiling (legacy) C code with a C++ compiler.
>>
>> Cheers,
>>
>> Sjoerd.
>>
>>
>>
>> *From:* Sjoerd Meijer [mailto:sjoerd.meijer at arm.com
>> <sjoerd.meijer at arm.com>]
>> *Sent:* 03 August 2015 11:40
>> *To:* 'Richard Smith'
>> *Cc:* Hal Finkel; Marshall Clow; cfe-dev at cs.uiuc.edu Developers; cfe
>> commits
>> *Subject:* RE: [PATCH] RE: [cfe-dev] missing return statement for
>> non-void functions in C++
>>
>>
>>
>> Hi Richard,
>>
>>
>>
>> I agree with your conclusions and will start preparing a patch for option
>> 3) under a flag that is off by default; this enables folks to build/run C
>> code in C++. I actually think option 2) would be a good one too, but as it
>> is already available under a flag I also don’t see how useful it is
>> combining options 2) and 3) with another (or one more) flag that is off by
>> default.
>>
>>
>>
>> Cheers.
>>
>>
>>
>> *From:* metafoo at gmail.com [mailto:metafoo at gmail.com <metafoo at gmail.com>] *On
>> Behalf Of *Richard Smith
>> *Sent:* 31 July 2015 19:46
>> *To:* Sjoerd Meijer
>> *Cc:* Hal Finkel; Marshall Clow; cfe-dev at cs.uiuc.edu Developers; cfe
>> commits
>> *Subject:* Re: [PATCH] RE: [cfe-dev] missing return statement for
>> non-void functions in C++
>>
>>
>>
>> On Fri, Jul 31, 2015 at 7:35 AM, Sjoerd Meijer <sjoerd.meijer at arm.com>
>> wrote:
>>
>> 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?
>>
>>
>>
>> I don't think this is an improvement:
>>
>>
>>
>> This doesn't satisfy the folks who want an 'unreachable' for better code
>> size and optimization, and it doesn't satisfy the folks who want a
>> guaranteed trap for security, and it doesn't satisfy the folks who want
>> their broken code to limp along (because it'll still trap at -O0), and it
>> is at best a minor improvement for the folks who want missing returns to be
>> more easily debuggable (with -On, the code goes wrong in the caller, or
>> appears to work, rather than falling into an unrelated function, and
>> debugging this with -O0 was already easy).
>>
>>
>>
>> I think there are three options that are defensible here:
>>
>> 1) The status quo: this is UB and we treat it as such and optimize on
>> that basis, but provide a trap as a convenience at -O0
>>
>> 2) The secure approach: this is UB but we always trap
>>
>> 3) Define the behavior to return 'undef' for C types: this allows
>> questionable C code that has UB in C++ to keep working when built with a
>> C++ compiler
>>
>>
>>
>> Note that (3) can be combined with either (1) or (2). (2) is already
>> available via the 'return' sanitizer. So this really reduces to: in those
>> cases where C says it's OK so long as the caller doesn't look at the
>> returned value (and where the return type doesn't have a non-trivial copy
>> constructor or destructor, isn't a reference, and so on), should we attempt
>> to preserve the C behaviour? I would be OK with putting that behind a `-f`
>> flag (perhaps `-fstrict-return` or similar) to support those folks who want
>> to build C code in C++, but I would suggest having that flag be off by
>> default, since that is not the usual use case for a C++ compiler.
>>
>>
>>
>> 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
>>
>>
>>
>>
>>
>> -- IMPORTANT NOTICE: The contents of this email and any attachments are
>> confidential and may also be privileged. If you are not the intended
>> recipient, please notify the sender immediately and do not disclose the
>> contents to any other person, use it for any purpose, or store or copy the
>> information in any medium. Thank you.
>>
>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>> Registered in England & Wales, Company No: 2557590
>> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
>> Registered in England & Wales, Company No: 2548782
>>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150813/08c22827/attachment-0001.html>