Remove wrong paranoia (and add correct one (and shorten in general (and fix a bug in))) fpc_dynarray_setlength.
Patch: dynarr.patch.
(Will conflict with #40179 (closed) but I’m tired to wait, I’ll redo one if you accept the other.)
Two arguments why the recheck of refcount = 1
cannot serve any purpose:
(1) This (non-)issue. At that time, I thought ansistring
assignments are atomic, and a dude on the local forum convinced me it was fundamentally impossible, and you knew it from the start... Arrays are the same.
(2) If truthness of refcount = 1
can change after the first check but before the second, it can change after the second just as well, so your unsynchronized code will just have different crash patterns. Synchronous change is out of question: no user code (like initialization/finalization; not even memory manager callbacks; okay, maybe runtime error handler :D) is performed between these two checks of refcount
.
Also, multiplication overflow check is just wrong. Counterexample on i386
:
var
x: array of array[0 .. 65535] of byte;
begin
SetLength(x, 10);
writeln('Allocated for 10 x 65536: ', MemSize(pointer(x) - 2 * sizeof(pointer))); // -> 655380 — ok
x := nil;
SetLength(x, 65536);
writeln('Allocated for 65536 x 65536: ', MemSize(pointer(x) - 2 * sizeof(pointer))); // -> 12 — wrong
end.
I changed it to {$push} {$q+,r+} r := a * b; {$pop}
. The only correct alternative I know about is verifying the multiplication result with div
.
Also, the only scenario when you don’t updatep
is SetLength(a, length(a))
for already unique a
, in which doing it anyway is harmless. Also, never taking newp
address and using it instead of realp
whenever possible should be beneficial for code generation.
Finally, this fixes a bug slightly similar to #40179 (closed): operator Initialize
is skipped for new members of the array that had several references and was simultaneously uniquified and grown by SetLength
, test included.