[parisc-linux] Looking at vfprintf.c and alloca.

[parisc-linux] Looking at vfprintf.c and alloca.

[Carlos O'Donell]

I was looking at our tst-printfsz failures in glibc, and I noticed
some very cute code in vfprintf.c. The code uses alloca to create a
specs structure, and then using certain know addresses decides if the
stack grows down or up.

Does this look right? Anyone care to review if this actually works
with a newer GCC on hppa?

   1601     size_t nspecs = 0;
   1602     size_t nspecs_max = 32;     /* A more or less arbitrary
start value.  */
   1603     struct printf_spec *specs
   1604       = alloca (nspecs_max * sizeof (struct printf_spec));

...
   1636     for (f = lead_str_end; *f != L_('\0'); f = specs[nspecs++].next_fmt)
   1637       {
   1638         if (nspecs >= nspecs_max)
   1639           {
   1640             /* Extend the array of format specifiers.  */
   1641             struct printf_spec *old = specs;
   1642
   1643             nspecs_max *= 2;
   1644             specs = alloca (nspecs_max * sizeof (struct printf_spec));
   1645
   1646             if (specs == &old[nspecs])
   1647               /* Stack grows up, OLD was the last thing allocated;
   1648                  extend it.  */
   1649               nspecs_max += nspecs_max / 2;
   1650             else
   1651               {
   1652                 /* Copy the old array's elements to the new space.  */
   1653                 memcpy (specs, old, nspecs * sizeof (struct
printf_spec));
   1654                 if (old == &specs[nspecs])
   1655                   /* Stack grows down, OLD was just below the new
   1656                      SPECS.  We can use that space when the new space
   1657                      runs out.  */
   1658                   nspecs_max += nspecs_max / 2;
   1659               }
   1660           }

I didn't expect alloca's behaviour to be so explicitly defined.

Cheers,
Carlos.
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

[parisc-linux] Re: Looking at vfprintf.c and alloca.

[Randolph Chung]

Carlos O'Donell wrote:
> I was looking at our tst-printfsz failures in glibc, and I noticed
> some very cute code in vfprintf.c. The code uses alloca to create a
> specs structure, and then using certain know addresses decides if the
> stack grows down or up.
> 
> Does this look right? Anyone care to review if this actually works
> with a newer GCC on hppa?

I tried this with both old (gcc-3.[34]) and new gcc (gcc-4.[01]) and i 
don't think this works....

tausq@riot:~$ cat alloca.c
#include <alloca.h>
#include <stdio.h>

int main(int argc, char **argv)
{
         int *old;
         int *ptr = alloca(100 * sizeof(int));

         old = ptr;

         ptr = alloca(100 * sizeof(int));

         printf("ptr = %p\nold = %p\n&old[100] = %p\n", ptr, old, 
&old[100]);
         return 0;
}
tausq@riot:~$ gcc-4.1 -Wall -o alloca alloca.c; ./alloca
ptr = 0xc04ca590
old = 0xc04ca3d0
&old[100] = 0xc04ca560

randolph
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Re: [parisc-linux] Re: Looking at vfprintf.c and alloca.

[Michael S. Zick]

On Tue July 18 2006 10:51, Randolph Chung wrote:
> Carlos O'Donell wrote:
> > I was looking at our tst-printfsz failures in glibc, and I noticed
> > some very cute code in vfprintf.c. The code uses alloca to create a
> > specs structure, and then using certain know addresses decides if the
> > stack grows down or up.
> > 
> > Does this look right? Anyone care to review if this actually works
> > with a newer GCC on hppa?
> 
> I tried this with both old (gcc-3.[34]) and new gcc (gcc-4.[01]) and i 
> don't think this works....
> 
> tausq@riot:~$ cat alloca.c
> #include <alloca.h>
> #include <stdio.h>
> 
> int main(int argc, char **argv)
> {
>          int *old;
>          int *ptr = alloca(100 * sizeof(int));
> 
>          old = ptr;
> 
>          ptr = alloca(100 * sizeof(int));
> 
>          printf("ptr = %p\nold = %p\n&old[100] = %p\n", ptr, old, 
> &old[100]);
>          return 0;
> }
> tausq@riot:~$ gcc-4.1 -Wall -o alloca alloca.c; ./alloca
> ptr = 0xc04ca590
> old = 0xc04ca3d0
> &old[100] = 0xc04ca560
> 

For the case of gcc-4.2, stack-grows down...

ptr = 0xbfea1fc0
old = 0xbfea2160
&old[100] = 0xbfea22f0

In both the cases (up/down) &old[100] has an offset of
400 bytes (correct).

There is some compiler generated, machine dependent,
optimization dependent, space between "ptr" and "old[100]"
Most likely the "guaranteed stack alignment" thingy.

Mike
> randolph
> _______________________________________________
> parisc-linux mailing list
> parisc-linux@lists.parisc-linux.org
> http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
> 
> 
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

[parisc-linux] Re: Looking at vfprintf.c and alloca.

[Carlos O'Donell]

On 7/18/06, Randolph Chung <randolph@tausq.org> wrote:
> I tried this with both old (gcc-3.[34]) and new gcc (gcc-4.[01]) and i
> don't think this works....
>
> tausq@riot:~$ gcc-4.1 -Wall -o alloca alloca.c; ./alloca
> ptr = 0xc04ca590
> old = 0xc04ca3d0
> &old[100] = 0xc04ca560

I thought so, if this code is ever execute it will be wrong.

I think our tst-printfsz failure is related to this code trying to
expand the specs and getting the wrong answer.

Michael, want to write a patch to fix this for glibc? :)
I'll support you on libc-alpha!

In glibc I don't think the code should be a runtime check at all,
there are STACK_GROWS_UP macros that should be used.

c.
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

[parisc-linux] Re: Looking at vfprintf.c and alloca.

[Michael S. Zick]

On Tue July 18 2006 14:30, Carlos O'Donell wrote:
> On 7/18/06, Randolph Chung <randolph@tausq.org> wrote:
> > I tried this with both old (gcc-3.[34]) and new gcc (gcc-4.[01]) and i
> > don't think this works....
> >
> > tausq@riot:~$ gcc-4.1 -Wall -o alloca alloca.c; ./alloca
> > ptr = 0xc04ca590
> > old = 0xc04ca3d0
> > &old[100] = 0xc04ca560
> 
> I thought so, if this code is ever execute it will be wrong.
>

I did not look at the original code - just the posted example.

If the library code is depending on an expected difference between
the old and new stack position (other than direction) 
- it will be in trouble.

> 
> I think our tst-printfsz failure is related to this code trying to
> expand the specs and getting the wrong answer.
> 
> Michael, want to write a patch to fix this for glibc? :)
> I'll support you on libc-alpha!
> 

It just so happened that I was compiling glibc and during this exchange
vfprintf.c scrolled by...
I think it set some sort of record for "number of warning messages"

So it needs work.  Probably would be better to give someone else a chance.

Getting -finstrument-functions and the related __builtin_{frame,return}_address()
to work is taking all of my time right now.

> In glibc I don't think the code should be a runtime check at all,
> there are STACK_GROWS_UP macros that should be used.
> 

Sounds reasonable to me.

> c.
Mike
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Re: [parisc-linux] Re: Looking at vfprintf.c and alloca.

[Matthew Wilcox]

On Tue, Jul 18, 2006 at 03:30:40PM -0400, Carlos O'Donell wrote:
> On 7/18/06, Randolph Chung <randolph@tausq.org> wrote:
> >tausq@riot:~$ gcc-4.1 -Wall -o alloca alloca.c; ./alloca
> >ptr = 0xc04ca590
> >old = 0xc04ca3d0
> >&old[100] = 0xc04ca560
> 
> I thought so, if this code is ever execute it will be wrong.
> 
> I think our tst-printfsz failure is related to this code trying to
> expand the specs and getting the wrong answer.
> 
> Michael, want to write a patch to fix this for glibc? :)
> I'll support you on libc-alpha!
> 
> In glibc I don't think the code should be a runtime check at all,
> there are STACK_GROWS_UP macros that should be used.

How about just using min(ptr, old)?  ;-)
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Re: [parisc-linux] Re: Looking at vfprintf.c and alloca.

[Carlos O'Donell]

On 7/18/06, Matthew Wilcox <matthew@wil.cx> wrote:
> > In glibc I don't think the code should be a runtime check at all,
> > there are STACK_GROWS_UP macros that should be used.
>
> How about just using min(ptr, old)?  ;-)

You're almost there Matthew, just a little more, and I'll be able to
list your name in the libc-ports ChangeLogs :-)

c.
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

[parisc-linux] Re: Looking at vfprintf.c and alloca.

[John David Anglin]

>    1646             if (specs == &old[nspecs])

>    1654                 if (old == &specs[nspecs])

These two tests seem totally bogus to me.  They don't take
into account the padding needed to maintain the preferred
stack boundary.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

[parisc-linux] Re: Looking at vfprintf.c and alloca.

[John David Anglin]

> I tried this with both old (gcc-3.[34]) and new gcc (gcc-4.[01]) and i 
> don't think this works....
> 
> tausq@riot:~$ cat alloca.c
> #include <alloca.h>
> #include <stdio.h>
> 
> int main(int argc, char **argv)
> {
>          int *old;
>          int *ptr = alloca(100 * sizeof(int));
> 
>          old = ptr;
> 
>          ptr = alloca(100 * sizeof(int));
> 
>          printf("ptr = %p\nold = %p\n&old[100] = %p\n", ptr, old, 
> &old[100]);
>          return 0;
> }
> tausq@riot:~$ gcc-4.1 -Wall -o alloca alloca.c; ./alloca
> ptr = 0xc04ca590
> old = 0xc04ca3d0
> &old[100] = 0xc04ca560

What doesn't work?  ptr - old = 0x1c0 = 7 * 64.  The preferred
stack boundary is 64 bytes.  6 * 64 = 384 bytes.  That's too
small for the requested allocation.  &old[100] - old = 0x190 = 400.
So, the results printed appear correct.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

[parisc-linux] Re: Looking at vfprintf.c and alloca.

[Randolph Chung]

> What doesn't work?  ptr - old = 0x1c0 = 7 * 64.  The preferred
> stack boundary is 64 bytes.  6 * 64 = 384 bytes.  That's too
> small for the requested allocation.  &old[100] - old = 0x190 = 400.
> So, the results printed appear correct.

gcc is completely correct, I meant to agree with your other email, that
is glibc is totally bogus ;P

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Re: [parisc-linux] Re: Looking at vfprintf.c and alloca.

[John David Anglin]

> For the case of gcc-4.2, stack-grows down...

Is this for hppa?  The stack should never grow downward.

> ptr = 0xbfea1fc0
> old = 0xbfea2160
> &old[100] = 0xbfea22f0
> 
> In both the cases (up/down) &old[100] has an offset of
> 400 bytes (correct).

You have to take into account the preferred stack boundary
into account when determining the delta between alloca allocations.

The HP runtime specifies 64 bytes (i.e., cache aligned) for
the preferred stack boundary in the 32-bit runtime.  The stack
boundary is 8 bytes.  This is the minimum alignment that the
hardware needs except for the infamous ldcw instruction.  The
preferred stack boundary could have been 8 bytes.  However,
changing it now would be an ABI change...

HP probably recognized that 64-byte alignment didn't help performance
much, so they relaxed the 64-bit alignment to 16 bytes.  In the 64-bit
runtime, some types are 16-byte aligned.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Re: [parisc-linux] Re: Looking at vfprintf.c and alloca.

[Michael S. Zick]

On Tue July 18 2006 22:04, Randolph Chung wrote:
> > What doesn't work?  ptr - old = 0x1c0 = 7 * 64.  The preferred
> > stack boundary is 64 bytes.  6 * 64 = 384 bytes.  That's too
> > small for the requested allocation.  &old[100] - old = 0x190 = 400.
> > So, the results printed appear correct.
> 
> gcc is completely correct, I meant to agree with your other email, that
> is glibc is totally bogus ;P
>

Since I have exchanged off-list bits and pieces that are related to the
series 4 compilers, let me try to explain the observations to date.
 
Joel has invested months of time, looking for the cause of 64-bit-smp
hangs under heavy load.

He will post when he is ready, I say nothing here other than he is
still working hard on the problem.

I have an interest in getting -finstrument-functions to work to my
own standard of satisfaction.  I will post to the appropriate lists
when I am certain I have all of the "cockpit error" out of my findings.

</end disclaimers>

I will be very careful to avoid the word: "mis-compiled" since it does
not apply to the following;

Given any series 3 compiler...
Given any C source code...
Define the assembler code generated as "expected output"...
Define the logical function(s) performed by that code 
as the "expected result".

The test standard of "expected output" is only an indicator;
not an error in itself.

We have found, Joel in the functions using spinlock macros;
Myself in the functions of the gcc provided crt* files;

That not only is the series 4 compiler not generating the
expected output (only an indicator) but it is not generating
the expected result (functionally equivalent code).

To date, neither of us have found a "wrong code" error in
the compilers.

What we have found are "source based" problems, in general,
the series 4 compilers require additional annotations in the
source to generate functionally equivalent code.

Without sufficient, proper, annotations; the series 4 compiler
will (correctly) throw away things the author of the source
expected to always be part of the "expected result".

The above is not our "single point of failure".  We have both
discovered good, old fashioned, programming errors in addition
to the above.

Which brings me to the subject of this thread, the vfprintf.c
is a "good, old fashioned, programming error" if the code expects
the stack space generated by alloca to be contiguous with any
other stack object.  That is true for either series compiler,
on any machine, regardless of stack direction growth.
(The man page for alloca could also use some help.  Its text
ignores the effect of compiler stack padding.)

> randolph
Mike
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Re: [parisc-linux] Re: Looking at vfprintf.c and alloca.

[John David Anglin]

> Which brings me to the subject of this thread, the vfprintf.c
> is a "good, old fashioned, programming error" if the code expects
> the stack space generated by alloca to be contiguous with any
> other stack object.  That is true for either series compiler,
> on any machine, regardless of stack direction growth.
> (The man page for alloca could also use some help.  Its text
> ignores the effect of compiler stack padding.)

I agree that the linux man page doesn't accurately describe the
behavior of alloca on hppa when it is inlined by gcc:

	The  alloca() function allocates size bytes of space in
	the stack frame of the caller.

The HP man page is more accurate:

	Allocates space from the stack of the caller for
	a block of at least size bytes, ...

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux