test: Fix CLI_MAX_ARG

maflcko commented at 9:39 am on August 23, 2025: member

CLI_MAX_ARG_SIZE has many edge case issues:

It seems to be lower on some systems, but it is unknown how to reproduce locally: #33079 (comment)
MAX_ARG_STRLEN is a limit per arg, but we probably want “The maximum length of [all of] the arguments”: See https://www.man7.org/linux/man-pages/man3/sysconf.3.html, section ARG_MAX - _SC_ARG_MAX.
It doesn’t account for the additional args added by the bitcoin command later on: https://github.com/bitcoin/bitcoin/blob/73220fc0f958f9b65f66cf0cf042af220b312fc6/src/bitcoin.cpp#L85-L92
It doesn’t account for unicode encoding a string to bytes before taking its length.

The issues are mostly harmless edge cases, but it would be good to fix them. So do that here, by:

Replacing max() by sum(), to correctly take into account all args, not just the largest one.
Reduce CLI_MAX_ARG_SIZE, to account for the bitcoin command additional args.

Also, there is a test. The test can be called with ulimit to hopefully limit the max args size to the hard-coded value in the test framework. For reference:

0$ ( ulimit -s 512 && python3 -c 'import os; print(os.sysconf("SC_ARG_MAX") )' ) 
1131072

On top of this pull it should pass, …

0bash -c 'ulimit -s 512 && BITCOIN_CMD="bitcoin -M" ./bld-cmake/test/functional/rpc_misc.py --usecli -l DEBUG'

… and with the test_framework changes reverted, it should fail:

0OSError: [Errno 7] Argument list too long: 'bitcoin'

Also, there is a commit to enable CI_LIMIT_STACK_SIZE=1 in the i686 task, because it should now be possible and no longer hit the hard-to-reproduce issue mentioned above.

DrahtBot commented at 9:39 am on August 23, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33243.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	cedwies, enirox001
Concept ACK	mzumsande

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

maflcko renamed this:
~~2508 test fix cli cmd size~~
test: Fix CLI_MAX_ARG_SIZE issues
on Aug 23, 2025

DrahtBot renamed this:
~~test: Fix CLI_MAX_ARG_SIZE issues~~
test: Fix CLI_MAX_ARG_SIZE issues
on Aug 23, 2025

DrahtBot added the label Tests on Aug 23, 2025

in test/functional/rpc_misc.py:33 in fabe21499c outdated

29@@ -30,6 +30,11 @@ def run_test(self):
30             lambda: node.echo(arg9='trigger_internal_bug'),
31         )
32 
33+        self.log.info("test max arg size")

cedwies commented at 3:57 pm on August 25, 2025:

This test also passes on master (MacOS environment with ARG_MAX = 1048576). It does not fail with the test_framework changes reverted. However, the following test only passes on this PR, not on my master:

0self.log.info("test many small args (sum-size trigger)")
1tiny = "a" * 120000
2args = [tiny] * 10
3assert_equal(args, node.echo(*args))

maflcko commented at 11:01 am on August 26, 2025:

This test also passes on master (MacOS environment with ARG_MAX = 1048576).

Thanks for adding a macos data point for reference. This further supports that the hard-coded value may be different on different platforms.

However, the following test only passes on this PR, not on my master:

Nice, thanks for adapting the test and checking the bugfix on macos.

in test/functional/test_framework/test_node.py:940 in fa18c82500 outdated

931@@ -928,10 +932,10 @@ def send_cli(self, clicommand=None, *args, **kwargs):
932         if clicommand is not None:
933             p_args += [clicommand]
934         p_args += pos_args + named_args
935-        max_arg_size = max(len(arg) for arg in p_args)
936+        sum_arg_size = sum(len(arg) for arg in p_args)

enirox001 commented at 4:15 pm on August 25, 2025:

02025-08-25T15:54:00.131311Z TestFramework.bitcoincli (DEBUG): Cli: Command size 131140 too large, using stdin
12025-08-25T15:54:00.148742Z TestFramework.bitcoincli (DEBUG): Cli: Command size 262212 too large, using stdin
22025-08-25T15:54:00.179312Z TestFramework.bitcoincli (DEBUG): Cli: Command size 4194372 too large, using stdin
32025-08-25T15:54:00.418554Z TestFramework.bitcoincli (DEBUG): Cli: Command size 8388676 too large, using stdin

fa18c82 — The debug logs show byte-based sizes (131140 for 65K chars). Should we use len(a.encode('utf-8')) to ensure consistency between the check and the actual command size measurement?

maflcko commented at 11:01 am on August 26, 2025:

Yeah, encoding seems like another possible source for inconsistencies. I don’t think we want to deal with encoding here, because the value is small enough for it to not matter.

enirox001 commented at 4:52 pm on August 26, 2025:

That’s reasonable. Since the test uses ASCII ‘a’ and the sizes are large, encoding won’t change the outcome in practice.

in test/functional/test_framework/test_node.py:53 in fa18c82500 outdated

45@@ -46,7 +46,11 @@
46 # The size of the blocks xor key
47 # from InitBlocksdirXorKey::xor_key.size()
48 NUM_XOR_BYTES = 8
49-CLI_MAX_ARG_SIZE = 131071 # many systems have a 128kb limit per arg (MAX_ARG_STRLEN)
50+# Many systems have a 128kB limit for a command size. However, when using the
51+# 'bitcoin' command, it may internally insert more args, which must be
52+# accounted for. There is no need to pick the largest possible value here
53+# anyway and it should be fine to set it to 1kB.
54+CLI_MAX_ARG_SIZE = 1024

enirox001 commented at 4:17 pm on August 25, 2025:

fa18c82 — Does this test use the system ARG_MAX at runtime or intentionally use a conservative value? Could we add a short comment that 1024 is a test-only, conservative value to ensure the stdin fallback is exercised, and that production behavior still relies on the system ARG_MAX

maflcko commented at 11:01 am on August 26, 2025:

thx, reworded the comment a bit

cedwies commented at 4:18 pm on August 25, 2025: none

ACK fabe214

This PR fixes how the functional test framework estimates command-line argument size. Previously it checked only the longest argument, but the OS limit is on the total argv/env size, so it now sums all args. It also adds a functional test using very large echo args, which exercises the -stdin fallback. The test passes for me.

I appreciate lowering CLI_MAX_ARG_SIZE to 1024. It’s a very safe, conservative threshold that avoids platform-dependent ARG_MAX issues, and since larger payloads automatically go through -stdin anyway, I see no real downside. A nice fail-safe change IMO.

in test/functional/rpc_misc.py:34 in fa18c82500 outdated

29@@ -30,6 +30,11 @@ def run_test(self):
30             lambda: node.echo(arg9='trigger_internal_bug'),
31         )
32 
33+        self.log.info("test max arg size")
34+        for arg_sz in [0, 1, 100, 2**16 - 1, 2**17 - 1, 2**21 - 1, 2**22 - 1]:

enirox001 commented at 4:21 pm on August 25, 2025:

fa18c82 — Perhaps add a short note that these sizes target historical/modern ARG_MAX boundaries (64KiB, ~128KiB, ~2MiB, ~4MiB) and that -1 tests just-below the power-of-two to catch off-by-ones? Took me a while to understand the intent.

maflcko commented at 11:01 am on August 26, 2025:

It is mostly a regression test, checking that on master a single arg with size of CLI_MAX_ARG_SIZE = 131071 fails the test (with the reproducer shared in the pull description).

Force pushed to reduce to that (and added a comment).

enirox001 commented at 4:39 pm on August 25, 2025: contributor

Concept ACK fa18c82 — The fix is a welcome improvement: summing arg sizes better reflects kernel ARG_MAX behavior. Good use of >= to match inclusive boundary semantics. Left a few non-blocking clarifier nits inline.

maflcko force-pushed on Aug 26, 2025

DrahtBot added the label CI failed on Aug 26, 2025

test: Fix CLI_MAX_ARG_SIZE issues facfde2cdc

ci: Enable CI_LIMIT_STACK_SIZE=1 in i686_no_ipc task fa96a4afea

maflcko force-pushed on Aug 26, 2025

DrahtBot removed the label CI failed on Aug 26, 2025

cedwies commented at 1:09 pm on August 26, 2025: none

ACK fa96a4a

DrahtBot requested review from enirox001 on Aug 26, 2025

enirox001 commented at 4:58 pm on August 26, 2025: contributor

ACK fa96a4a — thanks for addressing the nits and clarifying the test; LGTM.

mzumsande commented at 8:53 pm on August 26, 2025: contributor

It claims to be a “limit per arg”, but it is a “The maximum length of the arguments”: See https://www.man7.org/linux/man-pages/man3/sysconf.3.html, section ARG_MAX - _SC_ARG_MAX.

the relevant bottleneck for me (tested on Ubuntu and Debian) was not ARG_MAX, but MAX_ARG_STRLEN, which accounts for the maximum per arg, not the sum. e.g. the folllowing passes

0arg1=$(printf "%0*d" $((131072 - 1)) 0)
1arg2=$(printf "%0*d" $((131072 - 1)) 0)
2$(which printf) "%s %s" "$arg1" "$arg2" >/dev/null

whereas

0arg3=$(printf "%0*d" $((131072)) 0)
1$(which printf) "%s" "$arg3" >/dev/null

fails with bash: /usr/bin/printf: Argument list too long

But seeing how much this apparently differs between systems, Concept ACK to using a smaller limit.

maflcko commented at 5:48 am on August 27, 2025: member

the relevant bottleneck for me (tested on Ubuntu and Debian) was not ARG_MAX, but MAX_ARG_STRLEN, which accounts for the maximum per arg, not the sum.

Thanks for clarifying that this was intentionally picked and that MAX_ARG_STRLEN is a limit per arg, separate from the command size limit SC_ARG_MAX. SC_ARG_MAX is larger on many modern systems, but not all. Also, it reduces to MAX_ARG_STRLEN, or even lower on some systems. I’ve edited the pull description, to better reflect this.

test: Fix CLI_MAX_ARG_SIZE issues #33243

Code Coverage & Benchmarks

Reviews