Re: [TCLCORE] CFV warning: TIP #626: Command arguments > 2^31 elements

[<apn...@ya...>]

I think I have gotten about as far as I will with TIP 626 review. Some 100+ files are affected and I have been only been through a few but have enough concerns as to not proceed further until these are resolved.

Most important, is that as of now passing even the smallest 33-bit number of arguments is unusable in practice. Second, it is not clear to me based on a review of the source (keeping in mind my limited understanding of the byte code engine) that all necessary changes have been made.

With regards to the first, consider the following basic tests run on Stefan's 128GB Debian box (ulimit shows unlimited so I presume I can use as much memory as available).

% lassign [lseq 0x100000000] {*}[lseq 0x100000000]; set 1
killed

% lassign {} {*}[lseq 0x100000000]
killed

% dict set d {*}[lseq 0x100000000]; dict size $d
killed

% dict set d {*}[lrepeat 0x100000000 x]; dict size $d
killed

Or defining a proc that can take that many parameters. Note the proc is not even called.

% proc xx [lrepeat 0x100000000 x] {}
killed

Or explicitly defining the proc in script

% set l [lrepeat 0x100000000 x]; llength $l
4294967296
% writeFile manyargs.tcl [list proc xx $l {return $x}]
% exit
[apn@tcltk build]$ LD_LIBRARY_PATH=`pwd` ./tclsh manyargs.tcl  
killed

Or using args,

% proc xx args {}
% xx {*}[lseq 0x100000000]
killed

It is not clear to me if the above processes are killed because of memory exhaustion or *possible* implementation bugs (see below) that can be corrected. If the former, then I'm afraid one has to ask whether the extensive changes across more than a hundred files are warranted by a feature that will be very rarely used and is not usable even on a 128GB system.

My other concern is that the current implementation may be incomplete in at least some respects. This is purely based on a source review as confirmation through test cases is not possible because of the failures illustrated above. (Again, for CYA purposes, the byte code / compiler is a complicated beast so caveats about my reading of the code and understanding apply in all cases!)

Afaik, the byte code engine was incapable of handling 64-bit immediate values, jump offsets etc. in 9.0 and that has not changed with TIP 626. In 9.0, a global check ensured invocations with more than 2**31 (32?) arguments raised an error as it was explicitly not supported. TIP 626 removes this global check on argument count since that is its purpose. Instead, it adds a check to each compile command so that argument counts greater than INT_MAX are not byte compiled but take the slow path instead. It appears this check is missing at least in some cases, for example

TclCompileLindexCmd - the INST_LIST_INDEX_MULTI instruction emit

TclCompileListCmd - the build variable is an int but tracks number of arguments which is Tcl_Size and therefore can overflow. Changing it to Tcl_Size will not suffice either because it is then emitted via TclEmitInstInt4 which will truncate.

TclCompileReturnCmd - numOptionWords is typed Tcl_Size, derived from number of arguments but emitted via TclEmitInstInt4 with no check for overflow.

Those are the ones I found but I only went through a sampling of the compiled commands. 

Let me again stress the above are possibly correctable bugs but illustrate the need to have a test suite covering > 2**32 arguments for all commands that take a variable number of arguments.

The other category of potential errors is functions returning ints have changed to returning Tcl_Size. These include (at least)
 
TclCreateExceptRange
AnonymousLocal
LocalScalarFromToken
LocalScalar
ExceptionRangeStarts
TclCreateAuxData
IndexTailVarIfKnown
PushVarNameWord

These are used by multiple compilation functions. The potential problem here is the Tcl_Size value is then emitted by their callers with Emit14Inst / TclEmitInt4 etc. resulting in truncation. 

Similarly, jump offsets generated by if, switch, subst, exception range etc. compilers are now Tcl_Size but truncated to int (TclFixupForwardJumpToHere, OP4, IssueSwitchJumpTable). For example, TclFixupForwardJump and TclFinalizeLoopExceptionRange "break" targets issue INST_JUMP4 with 64-bit values.

Likewise, code offsets seem to be truncated before comparing in StartExpanding (afaik code offsets can be Tcl_Size with TIP 626 as per the type definitions)

In all these cases, I cannot see where the Tcl_Size values are restricted to INT_MAX making them eligible for emiting as 32 bit values. If in fact they are restricted, why were the functions changed to Tcl_Size in the first place? As an aside, the compiler does not generate truncation warnings because of casts.

That's all from me on TIP 626.

/Ashok



-----Original Message-----
From: Jan Nijtmans <jan...@gm...> 
Sent: Saturday, April 5, 2025 2:07 AM
To: apn...@ya...
Cc: tcl...@li...
Subject: Re: [TCLCORE] CFV warning: TIP #626: Command arguments > 2^31 elements

Op vr 4 apr 2025 om 12:21 schreef apnmbx-public:
> I haven't been reviewing it since you have been continuing with your
> commits. Since I can only afford the time for one full review, I wanted to
> wait till you are done. *If* you feel the implementation and test suite is
> more or less complete, I will review over the weekend. Let me know
> accordingly

Then, go ahead.

I'm sure some "bigdata" testcases need to be adapted: They
no longer give an error-message but will work as expected
now. I don't have a machine with enough memory to
do this. And those testcases cannot run in GITHUB
CI anyway.

       Jan Nijtmans