ARM assembly code created by JIT use same stack so it cause crash

RESOLVED DUPLICATE of bug 533280

Status

Core Graveyard
Nanojit
RESOLVED DUPLICATE of bug 533280
8 years ago
4 years ago

People

(Reporter: Michael Song, Unassigned)

Tracking

unspecified
Future
Other
Linux

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US) AppleWebKit/532.3 (KHTML, like Gecko) Chrome/4.0.223.16 Safari/532.3
Build Identifier: flash10.1 

assembly codes made for ARM platform overwrite  stack memory which used for argument passed to  avmglue_v2a_od_thunk.
third parameter value is set to 1 by assembly codes.
please refer to 20925 line of attached a-1.txt file which include log and assembly codes.
r2 point [fp-180] and r6  saved to [fp-180] so it's value is changed to 1 from object point value.

it need to modify nativeARM.cpp

Reproducible: Always

Steps to Reproduce:
1.build  ARM/Linux plugin
2.install libflashplayer.so
3.browsing the following site :
http://www.current.tv  (also http://www.current.com)
Actual Results:  
segment fault happened in "SObject* sobj = this->GetSObject();" of  DisplayObject::setNumberProperty.
this have a value of 1 which is invalid pointer.

Expected Results:  
this in DisplayObject::setNumberProperty should have valid point value 

    stqi newActivation1[24] = ldq1
004839d774   ldr r6, [fp, #-176]    r0(env) r5(ldc2) r7(newActivation1)
004839d778   ldr ip, [fp, #-172]    r0(env) r5(ldc2) r7(newActivation1)
004839d77c   str r6, [r7, #24]      r0(env) r5(ldc2) r7(newActivation1)
004839d780   str ip, [r7, #28]      r0(env) r5(ldc2) r7(newActivation1)
    ld3 = ld vars[16]
004839d784   ldr r6, [fp, #-112]    r0(env) r5(ldc2) r7(newActivation1)
    sti newActivation1[16] = ld3
004839d788   str r6, [r7, #16]      r0(env) r5(ldc2) r6(ld3) r7(newActivation1)
    ldq2 = ldq newActivation1[24]
004839d78c   ldr r6, [r7, #24]      r0(env) r5(ldc2) r7(newActivation1)
004839d790   ldr ip, [r7, #28]      r0(env) r5(ldc2) r7(newActivation1)
004839d794   str r6, [fp, #-176]    r0(env) r5(ldc2) r7(newActivation1)
004839d798   str ip, [fp, #-172]    r0(env) r5(ldc2) r7(newActivation1)
    stqi ldc2[192] = ldq2
004839d79c   ldr r6, [fp, #-176]    r0(env) r5(ldc2) r7(newActivation1)
004839d7a0   ldr ip, [fp, #-172]    r0(env) r5(ldc2) r7(newActivation1)
004839d7a4   str r6, [r5, #192]     r0(env) r5(ldc2) r7(newActivation1)
004839d7a8   str ip, [r5, #196]     r0(env) r5(ldc2) r7(newActivation1)
    ld4 = ld ldc2[152]
004839d7ac   ldr r6, [r5, #152]     r0(env) r5(ldc2) r7(newActivation1)
    ldq3 = ldq ldc2[192]
004839d7b0   ldr r4, [r5, #192]     r0(env) r5(ldc2) r6(ld4) r7(newActivation1)
004839d7b4   ldr ip, [r5, #196]     r0(env) r5(ldc2) r6(ld4) r7(newActivation1)
004839d7b8   str r4, [fp, #-192]    r0(env) r5(ldc2) r6(ld4) r7(newActivation1)
004839d7bc   str ip, [fp, #-188]    r0(env) r5(ldc2) r6(ld4) r7(newActivation1)
    jt eq1 -> npe
004839d7c0   tst  r6, r6            r0(env) r5(ldc2) r6(ld4) r7(newActivation1)
004839d7c4   beq 0x48385390         r0(env) r5(ldc2) r6(ld4) r7(newActivation1)
    ldc3 = ldc ld4[8]
004839d7c8   ldr r4, [r6, #8]       r5(ldc2) r6(ld4) r7(newActivation1)
    ldc4 = ldc ldc3[248]
004839d7cc   ldr r0, [r4, #248]     r4(ldc3) r5(ldc2) r6(ld4) r7(newActivation1)
    ialloc1 = ialloc 16
004839d7d0       11111    restore i r0(ldc4) r5(ldc2) r6(ld4) r7(newActivation1)
004839d7d0   sub r2, fp, #0xB8      r0(ldc4) r5(ldc2) r6(ld4) r7(newActivation1)
    sti ialloc1[4] = ld4
004839d7d4   str r6, [fp, #-180]    r0(ldc4) r2(ialloc1) r5(ldc2) r6(ld4) r7(newActivation1)
    stqi ialloc1[8] = ldq3
004839d7d8   ldr r6, [fp, #-192]    r0(ldc4) r2(ialloc1) r5(ldc2) r7(newActivation1)
004839d7dc   ldr ip, [fp, #-188]    r0(ldc4) r2(ialloc1) r5(ldc2) r7(newActivation1)
004839d7e0   str r6, [r2, #8]       r0(ldc4) r2(ialloc1) r5(ldc2) r7(newActivation1)
004839d7e4   str ip, [r2, #12]      r0(ldc4) r2(ialloc1) r5(ldc2) r7(newActivation1)
    ld5 = ld ldc4[0]
004839d7e8   ldr lr, [r0, #0]       r0(ldc4) r2(ialloc1) r5(ldc2) r7(newActivation1)
    iaddp1 = iaddp ialloc1, 4
004839d7ec   add r2, r2, #0x4       r0(ldc4) r2(ialloc1) r5(ldc2) r7(newActivation1) lr(ld5)
004839d7f0   sub r6, fp, #0xB4      r0(ldc4) r2(ialloc1) r5(ldc2) r7(newActivation1) lr(ld5)
    1 = int 1
004839d7f4   mov r6, #0x1           r0(ldc4) r2(iaddp1) r5(ldc2) r7(newActivation1) lr(ld5)
004839d7f8   str r6, [fp, #-180]    r0(ldc4) r2(iaddp1) r5(ldc2) r6(1) r7(newActivation1) lr(ld5)
    acalli1 = icall [ld5] ( ldc4 1 iaddp1 )
        
004839d7fc   mov r1, #0x1           r0(ldc4) r5(ldc2) r7(newActivation1) lr(ld5)
004839d800   blx ip                 r5(ldc2) r7(newActivation1)
004839d804       11111    restore e r5(ldc2) r7(newActivation1)
004839d804   ldr r0, [fp, #-152]    r5(ldc2) r7(newActivation1)
    ld6 = ld ldc2[152]
004839d808   ldr r6, [r5, #152]     r0(env) r5(ldc2) r7(newActivation1)
    jt eq2 -> npe


above are assembly codes made from JIT on ARM
in 004839d7f8  r6 is saved in [fp - 180] which is used by r2 that is third parameter of avmglue_v2a_od_thunk
(Reporter)

Comment 1

8 years ago
Created attachment 415751 [details]
this is assembly codes created on i386 platform by JIT
(Reporter)

Comment 2

8 years ago
Created attachment 415752 [details]
this is ARM assembly codes which cause crash

see assembly codes for "1 = int 1" and "findproperty1 = icall #findproperty
"
that was commented by //check
(Reporter)

Comment 3

8 years ago
Created attachment 415754 [details]
this is assemby codes created after patch is applied

see assembly codes for "1 = int 1" and "findproperty1 = icall #findproperty
"
that was commented by //check

now it don't use stack save 1 as in i386 platform
(Reporter)

Comment 4

8 years ago
on I386 platform, assembly codes for "1 = int 1" is not created.
but on ARM platform, assembly codes for "1 = int 1" is created because "1" is referred as a parameter of "findproperty1 = icall #findproperty"

in patch, immediate constant is used so stack memory is not used for "1 =  int 1" as in I386 platform.
(Reporter)

Comment 5

8 years ago
Created attachment 415759 [details]
diff image between original and patch in NativeARM.cpp

Comment 6

8 years ago
In Araxis you can use File->Report->Unix diff to generate a diff (patch) file.  And when you upload it be sure to select the patch checkbox.
(Reporter)

Comment 7

8 years ago
Created attachment 415773 [details] [diff] [review]
a diff patch file generated in Araxis
(Reporter)

Comment 8

8 years ago
Created attachment 415775 [details] [diff] [review]
patch diff using context format

Comment 9

8 years ago
Nice catch...this bug is quite subtle.  It likely means that all our back-ends need to have explicit logic for handling consts in asm_stkarg.

The patch still seems a bit buggered, I'll update it tomorrow but adding Nick to the review list for his feedback.

Comment 10

8 years ago
Comment on attachment 415775 [details] [diff] [review]
patch diff using context format

adding nick for early feedback.  The patch format is incorrect but you can get the idea.
Attachment #415775 - Flags: review?(nnethercote)
Sorry, I don't understand the bug.  Also, I'm taking Friday off and then travelling on the weekend to get to MV for the Mozilla all-hands on Monday, so I won't have much chance to look at this for a few days.

Comment 12

8 years ago
Micheal can you confirm if this is still an issue with the latest tamarin-redux.

I believe we have fixed it already.

Comment 13

8 years ago
Comment on attachment 415775 [details] [diff] [review]
patch diff using context format

Removing Nick from review.
Attachment #415775 - Flags: review?(nnethercote)

Updated

8 years ago
Component: JIT Compiler (NanoJIT) → Nanojit
Product: Tamarin → Core
QA Contact: nanojit → nanojit
Target Milestone: --- → Future

Updated

8 years ago
Status: UNCONFIRMED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 533280
(Assignee)

Updated

4 years ago
Component: Nanojit → Nanojit
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.