ARM avmshell getstack does not generate correct ARM instructions

VERIFIED FIXED

Status

Tamarin
Baseline JIT (CodegenLIR)
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: Tom Donovan, Assigned: Edwin Smith)

Tracking

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US) AppleWebKit/525.19 (KHTML, like Gecko) Chrome/1.0.154.65 Safari/525.19
Build Identifier: 1,4 2009-04-24

UnixPlatform::getStackBase() in avmshellUnix.cpp has no provision for ARM asm instructions to get the stack base.  The Intel instructions are not accepted by gcc.

Reproducible: Always

Steps to Reproduce:
1. Compile Tamarin on ARM/Linux  with gcc 4.3.2
2.
3.
Actual Results:  
 Error: bad instruction `movl %esp,r0'

Expected Results:  
no error
(Reporter)

Comment 1

9 years ago
Created attachment 382729 [details] [diff] [review]
avmshellUnix ARM patch
(Reporter)

Comment 2

9 years ago
From Dave Estes - Qualcomm.
Hardware: Other → ARM

Updated

9 years ago
Attachment #382729 - Flags: review?(edwsmith)
(Assignee)

Comment 3

9 years ago
Comment on attachment 382729 [details] [diff] [review]
avmshellUnix ARM patch

this should use VMCFG_ARM, AVMPLUS_ARM is being phased out.  R+ with this change.

also, it will only work on compilers with asm() support, (gcc + armcc), which presumably isnt a problem in avmshellUnix.cpp?
Attachment #382729 - Flags: review?(edwsmith) → review+
(Assignee)

Comment 4

9 years ago
It looks like this bug is no longer a problem with the latest code in tamarin-redux, which uses getrlimit().  Anyone want to confirm or deny?  either way this bug patches code which is now gone, so please rebase & resubmit a new patch if something more is needed.

Comment 5

9 years ago
We had a report from the field (tamarin-discuss) that this very patch fixes compilation on a Maemo IIRC, but it's not clear to me if that was tamarin-redux or tamarin-central.  Investigating.
Assignee: nobody → edwsmith
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Comment 6

9 years ago
The correspondent reports that he's working from tamarin-central (rev 710).

Comment 7

9 years ago
Created attachment 384839 [details] [diff] [review]
__clear_cache patch for unix
Attachment #384839 - Flags: review?(edwsmith)
(Assignee)

Comment 8

9 years ago
Comment on attachment 384839 [details] [diff] [review]
__clear_cache patch for unix

i moved the patch to bug 499914, I will push it to tamarin-redux
Attachment #384839 - Flags: review?(edwsmith) → review+
(Assignee)

Comment 9

9 years ago
Comment on attachment 382729 [details] [diff] [review]
avmshellUnix ARM patch

I'll push this to tamarin-central but there's nothing to merge with tamarin-redux since the code changed.
(Assignee)

Updated

9 years ago
Attachment #384839 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 11

9 years ago
(In reply to comment #9)
> (From update of attachment 382729 [details] [diff] [review])
> I'll push this to tamarin-central but there's nothing to merge with
> tamarin-redux since the code changed.

I have tried to use __clear_cache similar way as tamarin-redux, it still hangs with unmapped PC on ARMV7. I guess there are lot of ARMJIT changes which need to be merged into TC. Is there any easier way to get the ARMJIT related diff and apply it to TC?
(Assignee)

Comment 12

9 years ago
you can search through the TR history to find the change that corresponds to the TC tip, and do hg diff.  But, it might be easier to just use TR; its a deveopment branch but its pretty stable.

Comment 13

9 years ago
Resolved fixed engineering / work item that has been pushed.  Setting status to verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.