[Bug c/98055] New: __builtin_alloca should not have warn_unused_result attribute

[Bug c/98055] New: __builtin_alloca should not have warn_unused_result attribute

[psmith at gnu dot org]

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

            Bug ID: 98055
           Summary: __builtin_alloca should not have warn_unused_result
                    attribute
           Product: gcc
           Version: 10.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: psmith at gnu dot org
  Target Milestone: ---

Code that wants to use alloca() and still be portable will often include
replacements where memory is allocated on the heap, and the user is expected to
invoke alloca(0) periodically to free up this memory.  See for example the GNU
gnulib replacement alloca().

(Please no comments about the usefulness or not of alloca()--I'm not interested
in that discussion.  alloca() has problems but it's strictly more powerful than
VLAs, while providing the possibility of portability to compilers that don't
support them).

In this situation (invoking alloca(0)) there's no point in assigning the return
value, and older versions of GCC this works fine but when I try to compile the
same code with GCC 10.2.0 compilation fails because apparently
__builtin_alloca() now has the warn_unused_result attribute applied to it (this
isn't documented anywhere that I can find, but appears to be the case):

In file included from src/makeint.h:31,
                 from src/read.c:17:
src/read.c: In function 'eval_makefile':
lib/alloca.h:46:18: error: ignoring return value of '__builtin_alloca' declared
with attribute 'warn_unused_result' [-Werror=unused-result]
   46 | #  define alloca __builtin_alloca
src/read.c:435:3: note: in expansion of macro 'alloca'
  435 |   alloca (0);
      |   ^~~~~~

Because (void) doesn't work around this attribute there's no easy way to
resolve this; I don't think it's appropriate to add this attribute to alloca()
and it should be removed.

[Bug c/98055] __builtin_alloca should not have warn_unused_result attribute

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
See the thread starting at:
https://gcc.gnu.org/pipermail/gcc-patches/2019-June/523700.html

[Bug middle-end/98055] __builtin_alloca should not have warn_unused_result attribute

[psmith at gnu dot org]

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

--- Comment #2 from Paul Smith <psmith at gnu dot org> ---
I see no resolution to that thread, but the current behavior of GCC in this
respect is not right and Martin Sebor's arguments are missing the point: the
thinking there is too GCC-centric.  Whether or not builtin-alloca is used is
irrelevant: the issue is _portability_ to _other compilers_.

[Bug middle-end/98055] __builtin_alloca should not have warn_unused_result attribute

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |WONTFIX
                 CC|                            |msebor at gcc dot gnu.org

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
The -Wunused-result warning applies to calls to the GCC alloca built-in
function that allocate space on the stack.  To use replacement implementations
of the function the built-in must be disabled by -fno-builtin-alloca.  That
also suppresses the warning.  This behavior applies in general to other
built-in functions as well, including malloc (and including calls to malloc(0)
which need not allocate any memory and returns null on some implementations).

Diagnosing unportable code, either to other implementations or even to
different configurations of GCC, could be a useful feature, but it's not the
purpose of existing warnings.  Their goal is to detect potential problems given
the current target, under the options in effect.  As an example, take
-Wpointer-to-int-cast which only diagnoses slicing conversion under the
selected data model (e.g., pointer to int in LP64 but not in ILP32).

Broadening the scope of existing warnings to portability would need to be done
under the control of some sort of a -Wportability option that, to be useful and
not overly noisy, would also need to specify the target(s) to which the code is
intended to be portable.  Otherwise, to continue with the example of
-Wpointer-to-int-cast, the warning would have to diagnose every conversion of a
pointer to an integer except intptr_t (which might be useful to some but not
others).

I don't think it would be appropriate to make these implementation choices on a
case-by-case basis and have GCC behave differently for one set of constructs or
functions than another.  Doing so would only result in inconsistencies and make
the warnings less useful, not more.

[Bug middle-end/98055] __builtin_alloca should not have warn_unused_result attribute

[egallager at gcc dot gnu.org]

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

Eric Gallager <egallager at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=78014
                 CC|                            |egallager at gcc dot gnu.org

--- Comment #4 from Eric Gallager <egallager at gcc dot gnu.org> ---
(In reply to Martin Sebor from comment #3)
> 
> Broadening the scope of existing warnings to portability would need to be
> done under the control of some sort of a -Wportability option that, to be
> useful and not overly noisy, would also need to specify the target(s) to
> which the code is intended to be portable.

Hm, I thought there was a separate bug for this, but it turns out it was just a
tangent to bug 78014...

[Bug middle-end/98055] __builtin_alloca should not have warn_unused_result attribute

[psmith at gnu dot org]

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

--- Comment #5 from Paul Smith <psmith at gnu dot org> ---
IMO that response is missing the point.  This bug should be reopened and
resolved by removing this attribute from the __builtin_alloca function in GCC. 
That's all that's needed: there's no need for more complexity.

First, there's no need to add this attribute to alloca(): it's has virtually no
useful effect.  The chance that it actually catches a noticeable bug is almost
nil; any incorrect result that was more severe than reserving more stack than
was strictly necessary (actually failing to assign to a variable where it was
needed) would be obvious.

Second, alloca() is not just GCC's __builtin_alloca().  There are other
implementations, that behave differently than __builtin_alloca() and in those
implementations calling alloca() without using the return value is a valid and
useful thing to do.  I agree that it should not be up to GCC to try to suggest
portability options and am not suggesting it should do that.  However by adding
this attribute GCC is actively and affirmatively working _AGAINST_ portability.

The GCC docs say:

> warn_unused_result
> The warn_unused_result attribute causes a warning to be emitted if a caller
> of the function with this attribute does not use its return value. This is
> useful for functions where not checking the result is either a security
> problem or always a bug, such as realloc.

Using alloca() without checking the result is not a security problem, and it is
not always a bug as I've explained.  If it were the case that a commonly-used
malloc() replacement required calling malloc(0) (and not using the return
value) periodically for proper functioning, then absolutely GCC should clearly
not emit a warning for that usage even though GCC's malloc() doesn't work that
way, because otherwise it's hard to write code that doesn't depend on a
particular compiler.

Put more simply, I have this code in my program:

    alloca (0);

I need that line there so things work properly on compilers that don't provide
alloca().

How do you suggest I compile with GCC without either (a) forgoing this warning
everywhere or (b) adding a lot of pragma boilerplate to allow me to disable the
warning for this line or (c) creating a hack such as:

    { void *__p = alloca (0); (void) __p; }

Is there some preprocessor magic that lets me know that I'm using GCC's
__builtin_alloc so I can avoid calling alloca(0) in that situation?  I can't
think of one myself.

[Bug middle-end/98055] __builtin_alloca should not have warn_unused_result attribute

[msebor at gcc dot gnu.org]

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

--- Comment #6 from Martin Sebor <msebor at gcc dot gnu.org> ---
(In reply to Paul Smith from comment #5)
> Is there some preprocessor magic that lets me know that I'm using GCC's
> __builtin_alloc so I can avoid calling alloca(0) in that situation?

The following detects support for __builtin_alloca in GCC.  __has_builtin is
available in GCC 10 and later and all recent versions of Clang.

#ifdef __has_builtin
#  if __has_builtin (__builtin_alloca)
#    warning have __builtin_alloca
#  else
#   warning no __builtin_alloca
#  endif
#else
# warning no __has_builtin
#endif