This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 0/6] Some vfprintf refactoring
- From: Florian Weimer <fweimer at redhat dot com>
- To: "Carlos O'Donell" <carlos at redhat dot com>, libc-alpha at sourceware dot org
- Date: Thu, 05 Mar 2015 21:53:48 +0100
- Subject: Re: [PATCH 0/6] Some vfprintf refactoring
- Authentication-results: sourceware.org; auth=none
- References: <cover dot 1425246936 dot git dot fweimer at redhat dot com> <54F8AFE7 dot 4040603 at redhat dot com>
On 03/05/2015 08:35 PM, Carlos O'Donell wrote:
> On 03/01/2015 04:55 PM, Florian Weimer wrote:
>> These patches move around some code in vfprintf, and finally split off
>> the positional argument handling code from the main vfprintf function.
>
> Why? :-)
The goal is to remove extend_alloca. Define a local struct
scratch_buffer variable involves a large stack allocation. My initial
version just put a struct scratch_buffer variable definition roughly in
the same place as the work_buffer array. But that would increase stack
usage in all cases, even with positional parameters. I expected that
this would raise a red flag during review.
So I tried to eliminate the unconditional stack allocation of the
scratch buffer. First, I tried to move the scratch buffer definition
after the do_positional label. While doing that, I noticed the business
with the jump tables. It turns out that both the non-positional and
positional copy of the tables jump to the all_done label. In the
positional case, the scratch buffer is out of scope and cannot be freed.
So this does not work with the current scratch buffer functions. I
started adding a parameter to the jump table code so that I could inject
cleanup code specific to the positional case, but I stopped when I
realized what I was doing. Rather than writing yet another version of
the append-to-buffer code, I then realized that separating out the
positional case into a separate function would allow me to customize the
action when the all_done label is reached, which eventually led to the
current patch series.
GCC will inline the new printf_positional function because it is called
in just one place, so any differences should be just random noise by
slightly different optimizer choices. I will try to verify that and
report back the results. But due to the huge size of the function, it
could be very difficult to get a complete picture. Benchmarking is
difficult because I don't know what a representative use case of
positional arguments would be.
--
Florian Weimer / Red Hat Product Security