644951 - Fix for hang in AOT apps.

Status: RESOLVED INVALID

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Damanjit Singh]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.16) Gecko/20110319 Firefox/3.6.16
Build Identifier: 

Seems that AOT (Device Release ipa-test, ipa-debug) builds are very sensitive to AbcParser structure. Changes to this structure (like adding new fields at the beginning, changing the order of first two fields etc) results in a hang for some apps (specifically in one's calling overridden methods, at the point of call).
Moving the added bitfields to the end of the structure fixes the issue. It could be that some place in the code has made hardcoded assumptions about the Abcparser structure. But we haven't been able to find the exact cause.
But this needs to be fixed ASAP as is blocking QE certification. We have raised another bug investigate the root cause.

Reproducible: Always

Steps to Reproduce:
1.Happens every time with apps having overridden methods. hang occurs while calling an overridden method.
2. Happens on ios / ARM platform only.
Actual Results:  
App hangs

Expected Results:  
No hang

Comment 1

[Damanjit Singh]

Attachment: proposed patch

Comment 2

[Edwin Smith]

Comment on attachment 521774 [details] [diff] [review]
proposed patch

This is probably an improvement anyway -- any class that has DATA SECTION BEGIN/END comments should not have any fields outside that section; this patch moves the floatSupport flags into the data section.

Comment 3

[Steven Johnson]

Comment on attachment 521774 [details] [diff] [review]
proposed patch

given the urgency, I'll hold my nose and R+, but this is a major hack festival -- we really need to figure out how this change could possibly be affecting anything, as it smells like another latent bug somewhere that we're just getting lucky with...

Comment 4

[Lars T Hansen]

(In reply to comment #3)
> Comment on attachment 521774 [details] [diff] [review]
> proposed patch
> 
> given the urgency, I'll hold my nose and R+, but this is a major hack festival
> -- we really need to figure out how this change could possibly be affecting
> anything, as it smells like another latent bug somewhere that we're just
> getting lucky with...

IMO: A patch should never, ever be approved because it is simply "urgent".  If it needs to ship because it masks an important bug then so be it, but then it needs to be either on the ship branch and not in mainline, or it needs to be under the control of a suitable ifdefs and comments that say what's going on.

Comment 5

[Steven Johnson]

This patch isn't going to go into TR; it's for the Wasabi branch.

Comment 6

[Damanjit Singh]

Taking the bug / patch back as this is not supposed to go into Main or TR.
Marking the bug resolved, invalid.