Merge "sysconf(_SC_ARG_MAX): go back to imitating the kernel."
diff --git a/libc/bionic/sysconf.cpp b/libc/bionic/sysconf.cpp
index 2b3200c..dd6b129 100644
--- a/libc/bionic/sysconf.cpp
+++ b/libc/bionic/sysconf.cpp
@@ -52,16 +52,31 @@
// Things we actually have to calculate...
//
case _SC_ARG_MAX:
- // https://lkml.org/lkml/2017/11/15/813...
+ // You might think that just returning a constant 128KiB (ARG_MAX) would
+ // make sense, as this guy did:
//
- // I suspect a 128kB sysconf(_SC_ARG_MAX) is the sanest bet, simply
- // because of that "conservative is better than aggressive".
+ // https://lkml.org/lkml/2017/11/15/813...
//
- // Especially since _technically_ we're still limiting things to that
- // 128kB due to the single-string limit.
+ // I suspect a 128kB sysconf(_SC_ARG_MAX) is the sanest bet, simply
+ // because of that "conservative is better than aggressive".
//
- // Linus
- return ARG_MAX;
+ // Especially since _technically_ we're still limiting things to that
+ // 128kB due to the single-string limit.
+ //
+ // Linus
+ //
+ // In practice that caused us trouble with toybox tests for xargs edge
+ // cases. The tests assume that they can at least reach the kernel's
+ // "minimum maximum" of 128KiB, but if we report 128KiB for _SC_ARG_MAX
+ // and xargs starts subtracting the environment space and so on from that,
+ // then xargs will think it's run out of space when given 128KiB of data,
+ // which should always work. See this thread for more:
+ //
+ // http://lists.landley.net/pipermail/toybox-landley.net/2019-November/011229.html
+ //
+ // So let's resign ourselves to tracking what the kernel actually does.
+ // Right now (2019, Linux 5.3) that amounts to:
+ return MAX(MIN(__sysconf_rlimit(RLIMIT_STACK) / 4, 3 * _STK_LIM / 4), ARG_MAX);
case _SC_AVPHYS_PAGES: return get_avphys_pages();
case _SC_CHILD_MAX: return __sysconf_rlimit(RLIMIT_NPROC);
diff --git a/tests/unistd_test.cpp b/tests/unistd_test.cpp
index 99d92e6..f3b08c3 100644
--- a/tests/unistd_test.cpp
+++ b/tests/unistd_test.cpp
@@ -27,6 +27,7 @@
#include <stdint.h>
#include <sys/capability.h>
#include <sys/param.h>
+#include <sys/resource.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <sys/utsname.h>
@@ -1065,11 +1066,38 @@
}
TEST(UNISTD_TEST, sysconf_SC_ARG_MAX) {
- // https://lkml.org/lkml/2017/11/15/813.
-#if !defined(ARG_MAX)
-#define ARG_MAX 131072
-#endif
- ASSERT_EQ(ARG_MAX, sysconf(_SC_ARG_MAX));
+ // Since Linux 2.6.23, ARG_MAX isn't a constant and depends on RLIMIT_STACK.
+ // See prepare_arg_pages() in the kernel for the gory details:
+ // https://elixir.bootlin.com/linux/v5.3.11/source/fs/exec.c#L451
+
+ // Get our current limit, and set things up so we restore the limit.
+ rlimit rl;
+ ASSERT_EQ(0, getrlimit(RLIMIT_STACK, &rl));
+ uint64_t original_rlim_cur = rl.rlim_cur;
+ if (rl.rlim_cur == RLIM_INFINITY) {
+ rl.rlim_cur = 8 * 1024 * 1024; // Bionic reports unlimited stacks as 8MiB.
+ }
+ auto guard = android::base::make_scope_guard([&rl, original_rlim_cur]() {
+ rl.rlim_cur = original_rlim_cur;
+ ASSERT_EQ(0, setrlimit(RLIMIT_STACK, &rl));
+ });
+
+ // _SC_ARG_MAX should be 1/4 the stack size.
+ EXPECT_EQ(static_cast<long>(rl.rlim_cur / 4), sysconf(_SC_ARG_MAX));
+
+ // If you have a really small stack, the kernel still guarantees "32 pages" (see fs/exec.c).
+ rl.rlim_cur = 1024;
+ rl.rlim_max = RLIM_INFINITY;
+ ASSERT_EQ(0, setrlimit(RLIMIT_STACK, &rl));
+
+ EXPECT_EQ(static_cast<long>(32 * sysconf(_SC_PAGE_SIZE)), sysconf(_SC_ARG_MAX));
+
+ // With a 128-page stack limit, we know exactly what _SC_ARG_MAX should be...
+ rl.rlim_cur = 128 * sysconf(_SC_PAGE_SIZE);
+ rl.rlim_max = RLIM_INFINITY;
+ ASSERT_EQ(0, setrlimit(RLIMIT_STACK, &rl));
+
+ EXPECT_EQ(static_cast<long>((128 * sysconf(_SC_PAGE_SIZE)) / 4), sysconf(_SC_ARG_MAX));
}
TEST(UNISTD_TEST, sysconf_unknown) {