[Bug target/107606] New: rs6000: Option not to use parameter save area in variadic function implementations

[Bug target/107606] New: rs6000: Option not to use parameter save area in variadic function implementations

[fw at gcc dot gnu.org]

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

            Bug ID: 107606
           Summary: rs6000: Option not to use parameter save area in
                    variadic function implementations
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: fw at gcc dot gnu.org
                CC: bergner at gcc dot gnu.org
  Target Milestone: ---
            Target: powerpc64le-*-linux-gnu

Programmers tend to call functions like open, openat and prctl through
non-variadic prototypes. As a result, the caller does not set up the parameter
save area. Use of the parameter save area in the glibc implementation then
leads to difficult-to-diagnose (non-localized) memory corruption.

For glibc, we could use either a command-line option or a function attribute.

[Bug target/107606] rs6000: Option not to use parameter save area in variadic function implementations

[segher at gcc dot gnu.org]

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

--- Comment #1 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Hi Florian,

What do you want such an option to do?  The PSA is used only when it is needed,
do you want to have the compiler error out in such cases?  This is very
unpredictable to the user, so it should be clearly marked as just a debug aid
(in documentation and such).

Or what else is the intention?  Do you have an example of something that was
hard to debug, maybe?

[Bug target/107606] rs6000: Option not to use parameter save area in variadic function implementations

[fw at gcc dot gnu.org]

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

--- Comment #2 from Florian Weimer <fw at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #1)
> Or what else is the intention?  Do you have an example of something that was
> hard to debug, maybe?

The prctl(2) manual page documents the prctl function as:

       int prctl(int option, unsigned long arg2, unsigned long arg3,
                 unsigned long arg4, unsigned long arg5);

<https://man7.org/linux/man-pages/man2/prctl.2.html>

But if you use this prototype to call prctl (potentially through a function
pointer), then the glibc implementation will corrupt the stack in the caller
because it is defined as:

int
prctl (int option, ...)
{
…
}

The prototype in <sys/prctl.h> matches this definition. We use to have an
assembler implementation that did not clobber the caller stack even if called
as a non-variadic function. But that got replaced in glibc 2.32, breaking some
programs. We had similar portability issues with open/openat.

Technically these things are all undefined, and the glibc headers and
implementations are consistent. Users can even declare their own matching
(variadic) prototypes, as required by POSIX for such functions. On the other
hand, the prctl prototype is misdocumented, so it's easy to get that wrong, and
the variadic nature of open/openat is not that widely realized, either.
Basically this is an avoidable portability trap for powerpc64le-linux-gnu, and
maybe we can make life for programmers a bit easier.

[Bug target/107606] rs6000: Option not to use parameter save area in variadic function implementations

[segher at gcc dot gnu.org]

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

Segher Boessenkool <segher at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-11-10

--- Comment #3 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to Florian Weimer from comment #2)
> (In reply to Segher Boessenkool from comment #1)
> > Or what else is the intention?  Do you have an example of something that was
> > hard to debug, maybe?
> 
> The prctl(2) manual page documents the prctl function as:
> 
>        int prctl(int option, unsigned long arg2, unsigned long arg3,
>                  unsigned long arg4, unsigned long arg5);

And it also says you have to include <sys/prctl.h> to use this, and that
header has a correct declaration.  Could be worse ;-)

> But if you use this prototype to call prctl

You are never supposed to do that, you always should use the header file.
But people cut corners (which then ends up as being way more wasted work) :-(

> The prototype in <sys/prctl.h> matches this definition. We use to have an
> assembler implementation that did not clobber the caller stack even if
> called as a non-variadic function. But that got replaced in glibc 2.32,
> breaking some programs. We had similar portability issues with open/openat.

Ah, thanks for the background!

> Technically these things are all undefined, and the glibc headers and
> implementations are consistent.

And there is no standard that describe prctl(), which would be the last thing
justifying declaring it manually (like abort() is often declared manually
for example.  Bad practice really, but not wrong).

> Users can even declare their own matching
> (variadic) prototypes, as required by POSIX for such functions. On the other
> hand, the prctl prototype is misdocumented, so it's easy to get that wrong,
> and the variadic nature of open/openat is not that widely realized, either.

That is misdocumented as well, yes.

> Basically this is an avoidable portability trap for powerpc64le-linux-gnu,
> and maybe we can make life for programmers a bit easier.

Many PowerPC ABIs, not just 64-bit even -- and probably many other archs
will have the same problem even.  But it is more tractable to detect this
for rs6000 only, yup.

Confirmed.