Skip to content

Added decoding for static references#1091

Merged
automergerpr-permission-manager[bot] merged 10 commits into
masterfrom
dc/decode-statics
Jun 23, 2026
Merged

Added decoding for static references#1091
automergerpr-permission-manager[bot] merged 10 commits into
masterfrom
dc/decode-statics

Conversation

@dkcumming

Copy link
Copy Markdown
Collaborator

Added decoding for static references:

  • Changed reduce_to to keep the static items
  • Decoding of statics added to decoding.py
  • Added passing test for static references, and failing test for static pointers (to be followed up on in another PR)

@dkcumming dkcumming self-assigned this Jun 21, 2026

@mariaKt mariaKt 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.

Because statics have a symbol name, due to reduce_to now maintaining statics, they end up into the functions map as well.

Here is the path:

  • reduce_to keeps the static item, so it appears in SMIRInfo.items (smir.py:94, indexed by symbol_name).
  • function_symbols (smir.py:128) builds missing from ALL items.keys() not already in the functions table. The static's symbol isn't in that table, so it gets added and assigned a fake negative Ty (fake_ty = -2, -3, …).
    Now, every step that is followed for the functions (function_symbols_reverse, _functions), is similarly followed for the statics because they have a symbol, so they end up in the function map.

This is probably OK for correctness, because the negative Tys are never used as a call target, so the keys associated with statics are never looked up that way. But:

  1. the function map is already large; there's no need to bloat it further with entries that are never used.
  2. it's conceptually wrong: a static is not a function and shouldn't be in the functions map. While I see that they are both global objects and a common data structure with both can be useful (e.g., for addressing), I don't think that that data structure should be the functions map.

This can be addressed by restricting the missing computation to function items only, so only genuine functions get synthesized Tys, or another way if you prefer. Either in this PR or a quick follow-up, whichever is easier, no need to block this. It shouldn't affect the static decoding added by this PR, since it reads from the separate new statics property, not from the function map.

Comment thread kmir/src/kmir/decoding.py Outdated
@dkcumming

Copy link
Copy Markdown
Collaborator Author

@mariaKt good catch with the function symbols and statics mix up, thank you.

@automergerpr-permission-manager automergerpr-permission-manager Bot merged commit dcc97b0 into master Jun 23, 2026
12 checks passed
@automergerpr-permission-manager automergerpr-permission-manager Bot deleted the dc/decode-statics branch June 23, 2026 11:43
mariaKt added a commit that referenced this pull request Jun 26, 2026
40 new passing tests (this PR and PR #1091: static refs)
mariaKt added a commit that referenced this pull request Jun 26, 2026
40 new passing tests (this PR and PR #1091: static refs)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants