481208 - Move portable alloca infrastructure to MMgc

Status: VERIFIED FIXED

(Tamarin Graveyard :: Garbage Collection (mmGC), defect)

Description

[Tommy Reilly]

Attachment: patch created from p4 changelist

1) all memory management should be in MMgc
2) the player needs this when no AvmCore is around (symbian)

Comment 1

[Lars T Hansen]

Comment on attachment 365215 [details] [diff] [review]
patch created from p4 changelist

Two comments:

- some of us have already discussed that the name VMPI_alloca is the wrong name for this function, as the API is not like that of alloca and the semantics are those of a macro.  So that name will have to change, and the question is whether we should change it now before somebody starts using this code.

- The comments in the block above AllocaStackSegment refer to the avmplus exception handling system, which is not relevant if the code is in mmgc; on the other hand, that code needs to state very clearly that if any kind of longjump exception handling is used then that mechanism must incorporate some sort of cleanup that calls allocaPopTo(), or the world may come to an end.

Comment 2

[Steven Johnson]

(In reply to comment #1)
> (From update of attachment 365215 [details] [diff] [review])
> So that name will have to change, and the question is
> whether we should change it now before somebody starts using this code.

Let's change it now, while use is still minimal.

Comment 3

[Lars T Hansen]

Making sure Rishit sees it.

Comment 4

[Tommy Reilly]

I was gonna wait until Rishit's patch landed before landing this.

Comment 5

[Lars T Hansen]

AVM_STACK_ALLOC ? MMGC_STACK_ALLOC ? The code is not actually not platform dependent at all, so "VMPI_" is not warranted, and it must be a macro because it will expand to code that might contain a call to alloca(), where that's supported (it's assumed to be a bit faster than our own code, for several reasons.)  I favor ALL UPPER CASE for macro names :-)

MMGC_STACK_ALLOC looks like a configuration parameter because MMGC is controlled by names of the form MMGC_X rather than, say, MMGC_FEATURE_X (the general style adopted for the rest of the vm) or AVMPLUS_FEATURE_X (the actual sytle for the rest of the vm).  I see little reason to keep MMGC and AVMPLUS features separated, frankly, and would probably prefer MMGC to be controlled by AVMPLUS_FEATURE_X names too.

MMGC_StackAlloc ?

Add your suggestions here, please, but soon :-)

Comment 6

[Tommy Reilly]

MMGC_alloca?

Comment 7

[Lars T Hansen]

(In reply to comment #6)
> MMGC_alloca?

I can live with that.  (The name 'alloca' suggests semantics that simply aren't there... but the required three arguments to the macro will make the point well enough to the wayward developer, I suppose.)

Comment 8

[Tommy Reilly]

pushed as 1633

Comment 9

[Chris Peyer]

Resolved fixed engineering / work item that has been pushed.  Setting status to verified.