Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Interrealm take 2 #4028

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

[WIP] Interrealm take 2 #4028

wants to merge 10 commits into from

Conversation

jaekwon
Copy link
Contributor

@jaekwon jaekwon commented Mar 28, 2025

This replaces the previous interrealm spec.
Docs coming and this README will improve after more impl.

(TODO improve)

1. Keep the check on DidUpdate.
   (storage realm of real objects must be the same as the current realm to edit).
   This not only helps prevent global vars (and its subvalues) from being edited,
   but also values referred to by pointer, by slice (esp []byte), and so on.
2. When code takes a reference (pointer, slice) from a real object persisted
   in an external realm, it is considered "readonly".
   This prevents exploits where such references are passed by argument or even
   as field values to trick the VM into modifying something it doesn't want to.
   (e.g. x := extrealm.Notebook{Counter:&extrealm.Global.Foo[0]}.IncrementCounter())
   Wrapping does not occur by code defined in the same realm as the storage-realm
   of the base. "readonly" is unwrapped for receiver method calls, so methods
   can still modify a value.
3. Make realm switches explicit with 'withrealm(fn)(...)' and corresponding
   'switchrealm()'. 'switchrealm()' can only be used in realms. A function
   or method without 'switchrealm()' does not switch the realm, so /p/ code
   can be copied over to /r/ realms without change in behavior. This allows
   utility methods to exist also in realms.
   'switchrealm()' calls can be stacked, just like master currently.
4. "Soft-switch" to the receiver's storage realm when calling a method
   unless there is a switchrealm() in the method, in which case switch
   to the realm in which it is declared. Soft-switching does not affect
   std.CurrentRealm() and std.PreviousRealm().
   This allows new values saved by method to reside in the same realm
   as the receiver, prevents storage fragmentation, and prevents exploits
   related to 1,2 above.
5. Later, introduce attach() uverse function which recursively attaches
   all reachable unreal objects to the caller's realm.
   This is for when you want to pass an unreal object to a caller
   which will hold a reference to the unreal object(s) but you want the
   object to reside in your realm, not the callee's.

This PR is for 1,2 above.
For 3,4 see #4037.
5 implementation will come later, probably after launch.

@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label Mar 28, 2025
@Gno2D2
Copy link
Collaborator

Gno2D2 commented Mar 28, 2025

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):
  • IGNORE the bot requirements for this PR (force green CI check)
Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

No automated checks match this pull request.

☑️ Contributor Actions:
  1. Fix any issues flagged by automated checks.
  2. Follow the Contributor Checklist to ensure your PR is ready for review.
    • Add new tests, or document why they are unnecessary.
    • Provide clear examples/screenshots, if necessary.
    • Update documentation, if required.
    • Ensure no breaking changes, or include BREAKING CHANGE notes.
    • Link related issues/PRs, where applicable.
☑️ Reviewer Actions:
  1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.
📚 Resources:
Debug
Manual Checks
**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

  • Any user with comment edit permission

Copy link

codecov bot commented Mar 28, 2025

@@ -1970,15 +1970,44 @@ func (m *Machine) PushForPointer(lx Expr) {
}

func (m *Machine) PopAsPointer(lx Expr) PointerValue {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment should say, pop a pointer for writing. All calls to this will end up writing to it.

@@ -152,17 +159,13 @@ func (m *Machine) doOpStar() {
tv.SetUint8(dbv.GetByte())
m.PushValue(tv)
} else {
if pv.TV.IsUndefined() && bt.Elt.Kind() != InterfaceKind {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is vestigial; unless interface value, TV is never undefined, it at least has the concrete value set by defaultValue().

lvu := lv.IsUndefined()
rvu := rv.IsUndefined()
lvu := lv.IsUndefined2()
rvu := rv.IsUndefined2()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

after this in separate PR will convert all IsDefined() to IsDefined2().

if xvdt, ok := xv.T.(*DeclaredType); ok &&
xvdt.PkgPath == m.Realm.Path {
// Except allow if xv.T is m.Realm.
// XXX do we need/want this?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What could go wrong?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: No status
Status: In Review
Development

Successfully merging this pull request may close these issues.

2 participants