Re: [PATCH] printf_fp: Get rid of alloca.

[Joe Simmons-Talbott <josimmon@redhat.com>]

On Wed, Sep 06, 2023 at 01:55:55PM -0300, Adhemerval Zanella Netto wrote:
> 
> 
> On 01/09/23 09:35, Adhemerval Zanella Netto wrote:
> > 
> > 
> > On 01/09/23 09:20, Florian Weimer wrote:
> >> * Adhemerval Zanella Netto via Libc-alpha:
> >>
> >>> The maximum size of frac/tmp/scale is bound by the largest size of floating
> >>> supported by the ABI.  The p.exponent will at maximum LDBL_MAX_EXP, so the
> >>> required size will depend of the maximum support floating point support
> >>> (2080 for most architectures, otherwise 160).
> >>>
> >>> Using scratch_buffer will already increase the minimum stack usage for ~3k,
> >>> so I wonder if would be better to just define the maximum mp_limb_t size
> >>> direct on hack_digit_param:
> >>
> >> The issue is that this increases the stack size for functions like
> >> dprintf that might be used in signal handlers.  While it's conditional
> >> and only for floating point formatting, the old approach restricts the
> >> most excess to certain subcases, especially those involving long double.
> >> That's why I left in the alloca when I refactored __printf_fp to avoid
> >> stdio interfaces for internal use.
> > 
> > It already calls malloc in __printf_fp_buffer_1, so this is not fully 
> > AS-signal safe (it is unlikely, but possible).  Another option would to 
> > decompose __printf_fp_buffer_1 and if long double / float128 is use it 
> > routes to another function that allocates a large stack (the default for 
> > double should be ok, about ~680 bytes of extra stack).
> 
> I think we still avoid the pessimization of using one scratch_buffer for
> each temporary variable (~3KB of stack requirement) with some code
> refactoring.  For double we need at maximum ~160 bytes for each
> temporary, so we can allocate all the required buffer on the stack.
> The main issue is long double / _Float128, which may require up to
> ~6Kb of extra memory.  In this case, I think a single scratch_buffer
> should be suffice.  
> 
> Joe, I have implemented these on a personal branch [1].  It does not
> avoid the use o malloc (so printf double is still not fully AS-safe),
> but it should use a feasible amount of stack.
> 
> [1] https://sourceware.org/git/?p=glibc.git;a=blobdiff;f=stdio-common/printf_fp.c;h=431602caed85ee35c341a3565346406ff0736d45;hp=6f22985ba136da2ccb2bb5b9d397ce386c78ee91;hb=3c0bc3ed09ff7ce13524bab5ddb760b363640a1e;hpb=742b35228f3efa25d41d14f27c8911f308514b28
> 

Your branch looks okay to me and passed 'make check' without any
regressions.  Could you post the patch so that others more knowledgable
than myself can comment and review please?

Thanks,
Joe