475583 - update MMGC_GET_STACK_EXTENTS for gcc/thumb

Status: VERIFIED FIXED

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

Description

[Edwin Smith]

the only gcc/arm case that exists uses stmia which isn't thumb compatible.

Comment 1

[Edwin Smith]

from an email thread:

This is an easy change.  Can I talk you into submitting it to the open source repository?  Assuming my file is very similar to the current open source file, then around line 213 of GC.h you will find the gcc definition of of MMGC_GET_STACK_EXTENTS.  The gcc symbol you need to look for is __thumb__ and the register set you want to save is r4-r6.  So my file now has:

#ifdef __thumb__
// Store nonvolatile registers r4-r6
// Find stack pointer
#define MMGC_GET_STACK_EXTENTS(_gc, _stack, _size) \
int regs[3];\
asm("stmia %0,{r4-r6}" : : "r" (regs));\
asm("mov %0,sp" : "=r" (_stack));\
_size = (uintptr)_gc->GetStackTop() - (uintptr)_stack;
#else
// Store nonvolatile registers r4-r10
// Find stack pointer
#define MMGC_GET_STACK_EXTENTS(_gc, _stack, _size) \
int regs[7];\
asm("stmia %0,{r4-r10}" : : "r" (regs));\
asm("mov %0,sp" : "=r" (_stack));\
_size = (uintptr)_gc->GetStackTop() - (uintptr)_stack;
#endif // not _ARMCC__ (so gcc) and not thumb (so arm)

Comment 2

[Edwin Smith]

In gcc’s thumb build the implementation of setjmp is an arm (not thumb) routine and copies all of the registers into the jmp_buf.  So it seems like a perfectly good and portable definition of GET_STACK_EXTENTS would be something like:

#define MMGC_GET_STACK_EXTENTS( _gc, _stack, _size ) \

        jmp_buf jb; \

        setjmp( jb ); \

        _size = (uintpr)_gc->GetStackTop() – (uintptr)&jb;

And then you could get rid of the “_stack” parameter and probably have just a single implementation of it for all platforms.

Paul

Comment 3

[Edwin Smith]

Marking this required for the release, for __thumb__ but not for existing cpu's (more vetting required)

Comment 4

[Tommy Reilly]

I'm actually using the setjmp approach in my FlashRuntime branch and its working great on win/mac/linux so far.  Question is how do we test it on thumb?

Comment 5

[Tommy Reilly]

Attachment: new portable impl

Comment 6

[Tommy Reilly]

Attachment: new portable impl of MMGC_GET_STACK_EXTENTS

Same as last patch but includes winbuild.h which disables an assert needed for MSVC

Comment 7

[Edwin Smith]

Comment on attachment 369428 [details] [diff] [review]
new portable impl of MMGC_GET_STACK_EXTENTS

I don't understand the motivation for GCAutoExit, we need comments for that.  but i cant find any bugs, so R+ and lets unblock the release.

Comment 8

[Tommy Reilly]

MMGC_GCENTER macros uses it (see avmshell.cpp), it defines the stack entry point.  This is why all the platform specific code to find the start of the stack went away.

Comment 9

[Tommy Reilly]

pushed as 1631

Comment 10

[Tommy Reilly]

Attachment: hotfix for win32

win32 was storing callee saved regs into an area of the stack above the jmp_buf (ie lower address) in ZCT::Reap.  So the MMGC_GET_STACK_EXTENTS macro window missed this value which caused live objects to be collected.

Comment 11

[Chris Peyer]

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