Skip to content

objc: create new blocks as stack blocks so Copy moves them - #518

Merged
hajimehoshi merged 2 commits into
ebitengine:mainfrom
kumagi:fix/block-stack-isa
Sep 20, 2026
Merged

hajimehoshi merged 2 commits into
ebitengine:mainfrom
kumagi:fix/block-stack-isa

Conversation

@kumagi

@kumagi kumagi commented Sep 5, 2026 •

Copy link
Copy Markdown
Contributor

What issue is this addressing?

Closes #523

What type of issue is this addressing?

bug

What this PR does | solves

NewBlock built its template with an NSMallocBlock isa, for which _Block_copy is a no-op retain of the Go-side allocation. The Block then points at Go memory invisible to the collector. Start from NSStackBlock so Copy relocates the block to the heap as the ABI requires.

@hajimehoshi

Copy link
Copy Markdown
Member

Let's continue the discussion at #523

@kumagi

kumagi commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Sure, keeping the discussion in #523.

Outcome there: this is not a use-after-free. Block_copy chooses its path from the flags word (BLOCK_NEEDS_FREE / BLOCK_IS_GLOBAL) rather than from the isa, and it gives the relocated block its __NSMallocBlock__ isa itself, so NewBlock was already returning heap memory (verified on macOS arm64 via malloc_size / malloc_zone_from_ptr).

I left the change as a naming/clarity one only: the constant stays __NSStackBlock__ because that describes a not-yet-copied template, and I dropped the comment claim that Copy would be a no-op retain. I'll retitle the PR accordingly, and I'm happy to close it if you would rather not touch this at all.

NewBlock built its template with an __NSMallocBlock__ isa, for which
_Block_copy is a no-op retain of the Go-side allocation. The Block
then points at Go memory invisible to the collector. Start from
__NSStackBlock__ so Copy relocates the block to the heap as the ABI
requires.
Block_copy chooses its path from the flags word and gives the relocated
block its __NSMallocBlock__ isa itself, so the isa of the template is a
naming matter: __NSStackBlock__ describes a block that has not been
copied to the heap yet, as the Block ABI prescribes.
@kumagi
kumagi force-pushed the fix/block-stack-isa branch from d08147b to 346e3c8 Compare September 17, 2026 18:06

@hajimehoshi hajimehoshi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, assuming this is just a code clarification, not a bug fix

@hajimehoshi

Copy link
Copy Markdown
Member

@TotallyGamerJet PTAL

@TotallyGamerJet TotallyGamerJet left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hajimehoshi
hajimehoshi merged commit 656af01 into ebitengine:main Sep 20, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NewBlock uses __NSMallocBlock__ isa so Copy never moves the block off Go memory

3 participants