617250 - Static Mops/Alchemy storage

Status: VERIFIED WONTFIX

(Tamarin Graveyard :: Virtual Machine, enhancement)

Description

[Alex Macdonald]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_5; en-us) AppleWebKit/533.19.4 (KHTML, like Gecko) Version/5.0.3 Safari/533.19.4
Build Identifier: 

In  current shipping flavours of the VM the alchemy/mops opcodes have access to one dynamically allocated growable buffer per domain env. This patch adds support for a using a global fixed size buffer shared across domains.

We don't expect to use this in any shipping configuration of the player/AIR, with one exception. We want to offer this as an opt-in feature for native applications compiled for iOS. Since the apps produced for iOS are standalone apps with a captive runtime which the user can already inject arbitrary native code into there is no additional risk introduced by exposing this functionality.



Reproducible: Always

Comment 1

[Alex Macdonald]

Attachment: Static Mops

Comment 2

[Steven Johnson]

Comment on attachment 495757 [details] [diff] [review]
Static Mops

style nit: prevsailing style in Tamarin is for constants to begin with "k" or be all-caps so globalMemoryIsStatic -> kGlobalMemoryIsStatic or GLOBAL_MEMORY_IS_STATIC; ditto globalMemoryNeedsRangeCheck.

style nit: "if()" -> "if ()"

globalMemoryBase() and globalMemorySize() will generated unused-argument warnings on some compilers, this should be fixed.

globalMemoryNeedsRangeCheck is entirely unused and should therefore be removed.

Comment 3

[Steven Johnson]

Comment on attachment 495757 [details] [diff] [review]
Static Mops

Actually, one more nit that must be fixed: VMCFG_ macros are expected to use the existing avmfeatures.as setup, but this patch doesn't include any modifications to those. Please follow that protocol as it makes managing suites of features easier to track in the long run.

Comment 4

[Alex Macdonald]

Attachment: Static Mops

Fixed the stylistic bugs + added the avmfeatures changes

Comment 5

[Alex Macdonald]

Attachment: Static mops

here's the correct patch

Comment 6

[Steven Johnson]

Comment on attachment 495890 [details] [diff] [review]
Static mops

Looks good; adding Edwin as he's expressed a question about this...

Comment 7

[Edwin Smith]

Comment on attachment 495890 [details] [diff] [review]
Static mops

Over email, I've been convinced that the patch is safe from bad aliasing -- when used in an AOT context there will only be 1 DomainEnv and thus code can't tell if its really in the Domain or DomainEnv.

SR- for these reasons:
* needs comments, in avmfeatures.as, DomainMgr, or both, that fully explain the topology of domains in AOT, and where mops memory lives.  Or, a document in the docs directory that spells it all out.
* If there are other AOT-specific restrictions (is the size of the mops memory also fixed) then they need to be commented & explained.
* needs test cases that probe the failure modes (fixed-sized mops memory, single domains, etc).

Comment 8

[Alex Macdonald]

I didn't have the time to address the feedback for this release so I'm marking this bug as won't fix for now. Maybe next release we can do this.

Comment 9

[Trevor Baker]

bulk verifying resolved !fixed issues