This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] avoid calling alloca(0)


On 11/17/2016 03:52 PM, Jeff Law wrote:
On 11/17/2016 03:03 PM, Jakub Jelinek wrote:
On Thu, Nov 17, 2016 at 01:56:03PM -0700, Jeff Law wrote:
On 11/17/2016 11:24 AM, Jakub Jelinek wrote:
On Thu, Nov 17, 2016 at 11:14:26AM -0700, Martin Sebor wrote:
--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -2821,7 +2821,8 @@ compare_actual_formal (gfc_actual_arglist
**ap, gfc_formal_arglist *formal,
  for (f = formal; f; f = f->next)
    n++;

-  new_arg = XALLOCAVEC (gfc_actual_arglist *, n);
+  /* Take care not to call alloca with a zero argument.  */
+  new_arg = XALLOCAVEC (gfc_actual_arglist *, n + !n);

  for (i = 0; i < n; i++)
    new_arg[i] = NULL;

Ugh, that is just too ugly.  I don't see anything wrong on alloca (0),
and we don't rely on those pointers being distinct from other pointers.
On systems where alloca was implemented on top of malloc, alloca (0)
would
cause collection of alloca'd objects that had gone out of scope.

Ouch.  Do we support any such systems as hosts?  If yes, can't we just
define XALLOCAVEC etc. to alloca (len + 1) or alloca (len ? len : 1)
on those systems and leave the sane hosts as is?
I would guess they're all dead as hosts for building GCC.  I was most
familiar with hpux, but I'm pretty sure there were others as emacs
(IIRC) had a replacement alloca for systems without it as a builtin.
They probably all fall into the "retro-computing" bucket these days.

Essentially those systems worked by recording all the allocations as
well as the frame depth at which they occurred.  The next time alloca
was called, anything at a deeper depth than the current frame was released.

So even if we called alloca (0) unexpectedly, it's not going to cause
anything to break.  Failing to call alloca (0) could run the system out
of heap memory.  It's left as an exercise to the reader to ponder how
that might happen -- it can and did happen building GCC "in the old days".

The point is warning on an alloca (0) may not be as clear cut as it
might seem.  It's probably a reasonable thing to do on the host, but on
a target, which might be embedded and explicitly overriding the builtin
alloca, warning on alloca (0) is less of a slam dunk.

I confess I haven't heard of such an implementation before but after
some searching I have managed to find a few examples of it online,
such as in QNX or in the BlackBerry SDK, or in the GCC shipped by
Texas Instruments.  For instance:

http://www.qnx.com/developers/docs/6.5.0SP1.update/com.qnx.doc.neutrino_lib_ref/a/alloca.html

Their GCC-derived compilers apparently enable it with -fno-builtins.

Other references include source code derived from an implementation
with Doug Gwyn's name on it, such as this one:

https://opensource.apple.com/source/gnudiff/gnudiff-10/diffutils/alloca.c

A comment at the top the file mentions the alloca(0) use case:

	As a special case, alloca(0) reclaims storage without
	allocating any.  It is a good idea to use alloca(0) in
	your main control loop, etc. to force garbage collection.

That said, I don't view this as reason to exclude the warning from
-Wextra.  The vendor-provided compilers are obviously customized
versions of GCC with their own features, including their own set
of options and warnings.  It shouldn't stop us from enabling those
we expect to be useful to the typical GCC user on typical targets,
and especially those that can help expose sources of common bugs.
Recognizing I'm preaching to choir here: Calling alloca with any
argument is considered dangerous enough to be discouraged in most
man pages.  Alloca(0) is a potentially dangerous corner case of
an already dangerous API.  It seems at least as problematic as
any of the warnings already in -Wextra, and I'd say as many in
-Wall.

Even on systems with this unusual alloca implementation, since
alloca(0) is special (and expensive), warning on such calls would
likely be useful.  In fact, since calling alloca(0) in these
environments is actually important, warning on them coukd help
detect their unintended absence.  The few such calls that are
intended can be easily tweaked to suppress the warning.

Martin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]