Fix dangling btPChar values: the slot now owns its content#316
Open
TetzkatLipHoka wants to merge 1 commit into
Open
Fix dangling btPChar values: the slot now owns its content#316TetzkatLipHoka wants to merge 1 commit into
TetzkatLipHoka wants to merge 1 commit into
Conversation
Assigning to a script PChar stored the payload pointer of a TEMPORARY string (SetVariantValue: pansichar(dest^) := pansichar(PSGetAnsiString(...))) which is freed right after the statement - every 'p := <expression>' left a dangling pointer, and any later use read freed memory (e.g. a kernel32 lstrlenA import returned wrong lengths depending on heap reuse). The readers already treated the slot as a string: the comparison operators, Assigned() and the concat operator all cast the slot to tbtString. Make the ownership model explicit and consistent: - SetVariantValue stores a managed copy (the payload pointer IS the p-char value); CopyArrayContents copies btPChar record/array fields as string references; btPChar joins NeedFinalization and FinalizeVariant. - Function results of imported functions are copied into the slot on all paths (Rtti InvokeCall, x86 all four conventions, x64). - var-param p-chars are protected around external calls with PSBorrowPCharOwnership/PSReownPCharOwnership: the reference is borrowed before the call (recursing into records/static arrays); afterwards an untouched pointer silently keeps its reference, a replaced pointer's data is copied into an owned string. External code may therefore overwrite the pointer or write through it without corrupting the slot. - The class-property helper abused a btPChar heap variant as a raw pointer carrier (@DaTa); it is cleared before destruction now that finalization is real. Verified with DCC32/DCC64 on both invoke paths: pchar-from-literal survives heap churn, aliases stay isolated, var-param keep/replace round trips, and 30 compile/run cycles leak 0 bytes. The consolidated test suite goes from 70 to 71/74 passes (Rtti/classic) with the byte-p-char DLL probe green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Assigning to a script PChar stored the payload pointer of a TEMPORARY
string (SetVariantValue: pansichar(dest^) := pansichar(PSGetAnsiString(...)))
which is freed right after the statement - every 'p := ' left a
dangling pointer, and any later use read freed memory (e.g. a kernel32
lstrlenA import returned wrong lengths depending on heap reuse). The
readers already treated the slot as a string: the comparison operators,
Assigned() and the concat operator all cast the slot to tbtString.
Make the ownership model explicit and consistent:
value); CopyArrayContents copies btPChar record/array fields as string
references; btPChar joins NeedFinalization and FinalizeVariant.
paths (Rtti InvokeCall, x86 all four conventions, x64).
PSBorrowPCharOwnership/PSReownPCharOwnership: the reference is borrowed
before the call (recursing into records/static arrays); afterwards an
untouched pointer silently keeps its reference, a replaced pointer's data
is copied into an owned string. External code may therefore overwrite the
pointer or write through it without corrupting the slot.
carrier (@DaTa); it is cleared before destruction now that finalization
is real.
Verified with DCC32/DCC64 on both invoke paths: pchar-from-literal survives
heap churn, aliases stay isolated, var-param keep/replace round trips, and
30 compile/run cycles leak 0 bytes. The consolidated test suite goes from
70 to 71/74 passes (Rtti/classic) with the byte-p-char DLL probe green.
Found by the consolidated test suite (see the test-suite PR): its DLL probe showed lstrlenA returning heap-dependent wrong lengths for script PChar values.
🤖 Generated with Claude Code