[Toybox] [PATCH 1/2] xargs: fix kernel builds.

[enh]

On Mon, Oct 14, 2019 at 2:42 PM Rob Landley <rob at landley.net> wrote:
>
> On 10/14/19 3:53 PM, enh via Toybox wrote:
> > It turns out that findutils xargs almost always uses an artificially low
> > limit of 128KiB. You can observe this with --show-limits (which I
> > refrained from adding to toybox since I believe it's only useful if
> > you're debugging a broken xargs).
>
> The kernel guys' decision not to give us a way to query this is wrong, by the
> way. (I'm honestly tempted to whack-a-mole based on queried kernel version in
> portability.h at this point. It'd never work for macos and freebsd though...)
>
> > I think an alternative fix for all this would be to go back to counting
> > the cost of the (char *)s, but separate the -s limit from the "system"
> > limit --- that way we could have the same behavior as findutils xargs
> > for explicit values of -s (which we all seem to agree should *not*
> > include the cost of the (char *)s), but also not accidentally overrun
> > the actual system limits when we do count the (char *)s. That's more
> > complicated though, and findutils' "128KiB is enough for anyone"
> > behavior is demonstrably "good enough", so let's go with that for now.
>
> Are you building this on a kernel that _does_ have the 128k limit? (Which was
> lifted in 2.6.22 which came out July 8, 2007?) Otherwise a near miss of 128k
> shouldn't cause a problem...?

i'm not sure what you're talking about. the point is that the
findutils "don't count the (char *)s" algorithm is an _underestimate_
of how much space we really need, so comparing that to the actual
kernel limit is wrong. but findutils' other bug is that it doesn't
actually use the actual kernel limit. it uses 128KiB, which is much
smaller than the actual kernel limit (2090766 / 131072 == 16x), so it
"works" ("two wrongs make a right").

~$ xargs --show-limits
Your environment variables take up 2169 bytes
POSIX upper limit on argument length (this system): 2092935
POSIX smallest allowable upper limit on argument length (all systems): 4096
Maximum length of command we could actually use: 2090766
Size of command buffer we are actually using: 131072
Maximum parallelism (--max-procs must be no greater): 2147483647

Execution of xargs will continue now, and it will try to read its
input and run commands; if this is not what you wanted to happen,
please type the end-of-file keystroke.
Warning: echo will be run at least once.  If you do not want that to
happen, then press the interrupt keystroke.

~$

that 128KiB (131072 bytes) is just hard-coded. maybe because of
ancient kernels? but they don't seem to check kernel version at all.
i'm seeing their "always 128KiB" behavior on 4.19 and 5.x. (don't have
anything older easily available.)

> > Tested by building an Android common kernel with toybox xargs, which
> > failed before.
>
> Sigh, it built upstream -git. :P

repro steps if you have many gigabytes of disk and a bunch of time:

  mkdir common-android-mainline && cd common-android-mainline
  repo init -u https://android.googlesource.com/kernel/manifest -b
common-android-mainline
  repo sync

  ln -s /tmp/toybox/toybox ./build/build-tools/path/linux-x86/xargs
  BUILD_CONFIG=common/build.config.allmodconfig.x86_64 build/build.sh

the issue is a bit hidden in the logs, so repeat with an incremental build:

  SKIP_DEFCONFIG=1 SKIP_MRPROPER=1 SKIP_CP_KERNEL_HDR=1
BUILD_CONFIG=common/build.config.allmodconfig.x86_64 build/build.sh

gets you "xargs: Unknown option 'P8' (see "xargs --help")".

not skipping mrproper now triggers issue number two, so after the above:

  BUILD_CONFIG=common/build.config.allmodconfig.x86_64 build/build.sh

gets you "xargs: exec rm: Argument list too long".

> > Bug: http://b/140269206
>
> What's the translation URL to see this externally again? (I need to add that to
> the FAQ.)

it's issuetracker.something, but that's only for bugs that are
actually public bugs, so no help here. i've pasted the interesting
part above.

> (Sorry, my plane back to Austin leaves in two and a half hours, off to the
> airport. Probably just apply your stuff verbatim when I get home but I gotta go
> now.)

like i say in the commit message, we could try to do the _right_ thing
like BSD xargs, but being bug compatible with findutils is easy and
(demonstrably) "good enough", so i suspect we can get away with just
this. it upsets me a bit, but there's so many more useful things we
could fix first, and if findutils hasn't had to fix this, i doubt we
will either: 16x is one hell of a safety margin! i don't think we
_can_ hit that --- if every argument is 2 bytes, even if we're not
counting the 8 bytes for each pointer, that's still not off by a
factor of 16x.

> Rob