<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Aug 13, 2015 at 1:52 AM, Sjoerd Meijer <span dir="ltr"><<a href="mailto:Sjoerd.Meijer@arm.com" target="_blank">Sjoerd.Meijer@arm.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div lang="EN-GB" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Richard,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thanks for reviewing. Agree, that was a bit confusing. More specifically,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">the warning message was confusing (i.e. wrong). This patch is for compiling .c
<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">input in C++ mode. The new flag should be ignored for C++ *<b>input</b>*, and indeed
<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">not when it is in C++ *mode* as the warning message said earlier. So I have
<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">changed the warning message accordingly and hope that solves it, see attached<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">patch.</span></p></div></div></blockquote><div><br></div><div>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++.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-GB" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Cheers.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> <a href="mailto:metafoo@gmail.com" target="_blank">metafoo@gmail.com</a> [mailto:<a href="mailto:metafoo@gmail.com" target="_blank">metafoo@gmail.com</a>]
<b>On Behalf Of </b>Richard Smith<br>
<b>Sent:</b> 12 August 2015 23:06<br>
<b>To:</b> Sjoerd Meijer<br>
<b>Cc:</b> Hal Finkel; Marshall Clow; cfe-commits</span></p><div><div class="h5"><br>
<b>Subject:</b> Re: [PATCH] RE: [cfe-dev] missing return statement for non-void functions in C++<u></u><u></u></div></div><p></p><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">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++.<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On Wed, Aug 12, 2015 at 5:23 AM, Sjoerd Meijer <<a href="mailto:sjoerd.meijer@arm.com" target="_blank">sjoerd.meijer@arm.com</a>> wrote:<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">[ +
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a> ]</span><u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">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.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Cheers,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Sjoerd.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<div>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Sjoerd
Meijer [<a href="mailto:sjoerd.meijer@arm.com" target="_blank">mailto:sjoerd.meijer@arm.com</a>]
<br>
<b>Sent:</b> 03 August 2015 11:40<br>
<b>To:</b> 'Richard Smith'<br>
<b>Cc:</b> Hal Finkel; Marshall Clow; <a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">
cfe-dev@cs.uiuc.edu</a> Developers; cfe commits<br>
<b>Subject:</b> RE: [PATCH] RE: [cfe-dev] missing return statement for non-void functions in C++</span><u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Richard,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">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.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Cheers.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">
<a href="mailto:metafoo@gmail.com" target="_blank">metafoo@gmail.com</a> [<a href="mailto:metafoo@gmail.com" target="_blank">mailto:metafoo@gmail.com</a>]
<b>On Behalf Of </b>Richard Smith<br>
<b>Sent:</b> 31 July 2015 19:46<br>
<b>To:</b> Sjoerd Meijer<br>
<b>Cc:</b> Hal Finkel; Marshall Clow; <a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">
cfe-dev@cs.uiuc.edu</a> Developers; cfe commits<br>
<b>Subject:</b> Re: [PATCH] RE: [cfe-dev] missing return statement for non-void functions in C++</span><u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<div>
<p class="MsoNormal">On Fri, Jul 31, 2015 at 7:35 AM, Sjoerd Meijer <<a href="mailto:sjoerd.meijer@arm.com" target="_blank">sjoerd.meijer@arm.com</a>> wrote:<u></u><u></u></p>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">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?</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">I don't think this is an improvement:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">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).<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">I think there are three options that are defensible here:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">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<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">2) The secure approach: this is UB but we always trap<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">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<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">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.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0cm;margin-bottom:5.0pt">
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Cheers,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Sjoerd.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">
<a href="mailto:cfe-dev-bounces@cs.uiuc.edu" target="_blank">cfe-dev-bounces@cs.uiuc.edu</a> [mailto:<a href="mailto:cfe-dev-bounces@cs.uiuc.edu" target="_blank">cfe-dev-bounces@cs.uiuc.edu</a>]
<b>On Behalf Of </b>Richard Smith<br>
<b>Sent:</b> 29 July 2015 18:07<br>
<b>To:</b> Hal Finkel<br>
<b>Cc:</b> Marshall Clow; <a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a> Developers</span><u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><br>
<b>Subject:</b> Re: [cfe-dev] missing return statement for non-void functions in C++<u></u><u></u></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<p>On Jul 29, 2015 7:43 AM, "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>> wrote:<br>
><br>
> ----- Original Message -----<br>
> > From: "David Blaikie" <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>><br>
> > To: "James Molloy" <<a href="mailto:james@jamesmolloy.co.uk" target="_blank">james@jamesmolloy.co.uk</a>><br>
> > Cc: "Marshall Clow" <<a href="mailto:mclow.lists@gmail.com" target="_blank">mclow.lists@gmail.com</a>>, "cfe-dev Developers" <<a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a>><br>
> > Sent: Wednesday, July 29, 2015 9:15:09 AM<br>
> > Subject: Re: [cfe-dev] missing return statement for non-void functions in C++<br>
> ><br>
> ><br>
> > On Jul 29, 2015 7:06 AM, "James Molloy" < <a href="mailto:james@jamesmolloy.co.uk" target="_blank">
james@jamesmolloy.co.uk</a> ><br>
> > wrote:<br>
> > ><br>
> > > Hi,<br>
> > ><br>
> > > If we're going to emit a trap instruction (and thus create a broken<br>
> > > binary), why don't we error instead?<br>
> ><br>
> > We warn, can't error, because it may be dynamically unreached, in<br>
> > which case the program is valid and we can't reject it.<br>
><br>
> I think this also explains why this is useful for optimization.<br>
><br>
> 1. It is a code-size optimization<br>
> 2. By eliminating unreachable control flow, we can remove branches and tests that are not actual necessary<br>
><br>
> int foo(int x) {<br>
> if (x > 5) return 2*x;<br>
> else if (x < 2) return 3 - x;<br>
> }<br>
><br>
> 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).<u></u><u></u></p>
<p>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.<u></u><u></u></p>
<p>> -Hal<br>
><br>
> ><br>
> > ><br>
> > > James<br>
> > ><br>
> > > On Wed, 29 Jul 2015 at 15:05 David Blaikie < <a href="mailto:dblaikie@gmail.com" target="_blank">
dblaikie@gmail.com</a> ><br>
> > > wrote:<br>
> > >><br>
> > >><br>
> > >> On Jul 29, 2015 2:10 AM, "mats petersson" < <a href="mailto:mats@planetcatfish.com" target="_blank">
mats@planetcatfish.com</a><br>
> > >> > wrote:<br>
> > >> ><br>
> > >> ><br>
> > >> ><br>
> > >> > On 28 July 2015 at 23:40, Marshall Clow < <a href="mailto:mclow.lists@gmail.com" target="_blank">
mclow.lists@gmail.com</a><br>
> > >> > > wrote:<br>
> > >> >><br>
> > >> >><br>
> > >> >><br>
> > >> >> On Tue, Jul 28, 2015 at 6:14 AM, Sjoerd Meijer <<br>
> > >> >> <a href="mailto:sjoerd.meijer@arm.com" target="_blank">sjoerd.meijer@arm.com</a> > wrote:<br>
> > >> >>><br>
> > >> >>> Hi,<br>
> > >> >>><br>
> > >> >>><br>
> > >> >>><br>
> > >> >>> In C++, the undefined behaviour of a missing return statements<br>
> > >> >>> for a non-void function results in not generating the<br>
> > >> >>> function epilogue (unreachable statement is inserted and the<br>
> > >> >>> return statement is optimised away). Consequently, the<br>
> > >> >>> runtime behaviour is that control is never properly returned<br>
> > >> >>> from this function and thus it starts executing “garbage<br>
> > >> >>> instructions”. As this is undefined behaviour, this is<br>
> > >> >>> perfectly fine and according to the spec, and a compile<br>
> > >> >>> warning for this missing return statement is issued. However,<br>
> > >> >>> in C, the behaviour is that a function epilogue is generated,<br>
> > >> >>> i.e. basically by returning uninitialised local variable.<br>
> > >> >>> Codes that rely on this are not beautiful pieces of code, i.e<br>
> > >> >>> are buggy, but it might just be okay if you for example have<br>
> > >> >>> a function that just initialises stuff (and the return value<br>
> > >> >>> is not checked, directly or indirectly); some one might argue<br>
> > >> >>> that not returning from that function might be a bit harsh.<br>
> > >> >><br>
> > >> >><br>
> > >> >> I would not be one of those people.<br>
> > >> ><br>
> > >> ><br>
> > >> > Nor me.<br>
> > >> >><br>
> > >> >><br>
> > >> >>><br>
> > >> >>> So this email is to probe if there would be strong resistance<br>
> > >> >>> to follow the C behaviour? I am not yet sure how, but would<br>
> > >> >>> perhaps a compromise be possible/acceptable to make the<br>
> > >> >>> undefined behaviour explicit and also generate the function<br>
> > >> >>> epilogue?<br>
> > >> >><br>
> > >> >><br>
> > >> >> "undefined behavior" is exactly that.<br>
> > >> >><br>
> > >> >> You have no idea what is going to happen; there are no<br>
> > >> >> restrictions on what the code being executed can do.<br>
> > >> >><br>
> > >> >> "it just might be ok" means on a particular version of a<br>
> > >> >> particular compiler, on a particular architecture and OS, at a<br>
> > >> >> particular optimization level. Change any of those things, and<br>
> > >> >> you can change the behavior.<br>
> > >> ><br>
> > >> ><br>
> > >> > In fact, the "it works kind of as you expected" is the worst<br>
> > >> > kind of UB in my mind. UB that causes a crash, stops or other<br>
> > >> > "directly obvious that this is wrong" are MUCH easier to debug.<br>
> > >> ><br>
> > >> > So make this particular kind of UB explicit by crashing or<br>
> > >> > stopping would be a good thing. Making it explicit by<br>
> > >> > "returning kind of nicely, but not correct return value" is<br>
> > >> > about the worst possible result.<br>
> > >><br>
> > >> At -O0 clang emits a trap instruction, making it more explicit as<br>
> > >> you suggest. At higher optimization levels it just falls<br>
> > >> through/off.<br>
> > >><br>
> > >> ><br>
> > >> > --<br>
> > >> > Mats<br>
> > >> >><br>
> > >> >><br>
> > >> >> -- Marshall<br>
> > >> >><br>
> > >> >><br>
> > >> >> _______________________________________________<br>
> > >> >> cfe-dev mailing list<br>
> > >> >> <a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a><br>
> > >> >> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
> > >> >><br>
> > >> ><br>
> > >> ><br>
> > >> > _______________________________________________<br>
> > >> > cfe-dev mailing list<br>
> > >> > <a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a><br>
> > >> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
> > >> ><br>
> > >><br>
> > >> _______________________________________________<br>
> > >> cfe-dev mailing list<br>
> > >> <a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a><br>
> > >> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
> ><br>
> > _______________________________________________<br>
> > cfe-dev mailing list<br>
> > <a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a><br>
> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
> ><br>
><br>
> --<br>
> Hal Finkel<br>
> Assistant Computational Scientist<br>
> Leadership Computing Facility<br>
> Argonne National Laboratory<br>
><br>
> _______________________________________________<br>
> cfe-dev mailing list<br>
> <a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><u></u><u></u></p>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div></div></div>
<br>
<font face="Arial" color="Black" size="2">-- 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.<br>
<br>
ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590<br>
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782<br>
</font>
</div>
</blockquote></div><br></div></div>