Bug 155537

Summary: [ARM] REGRESSION(r198235): 12000 JSC stress tests started to crash on ARMv7 Thumb2 Linux platforms
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Blocker CC: cgarcia, clopez, gyuyoung.kim, jh718.park, mark.lam, oliver, ossy
Priority: P1    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 108645, 155508    

Description Csaba Osztrogonác 2016-03-16 03:49:42 PDT
GTK Linux ARM Release
----------------------
- r198228
- Failed 111 jsc tests
- https://build.webkit.org/builders/GTK%20Linux%20ARM%20Release/builds/10571

- r198253
- Failed 12070 jsc tests
- https://build.webkit.org/builders/GTK%20Linux%20ARM%20Release/builds/10570

EFL Linux ARMv7 Thumb2 Release
-------------------------------
- r198208
- Failed 18 jsc tests
- https://build.webkit.org/builders/EFL%20Linux%20ARMv7%20Thumb2%20Release/builds/17500

- r198268
- Failed 12049 jsc tests
- https://build.webkit.org/builders/EFL%20Linux%20ARMv7%20Thumb2%20Release/builds/17502

This regression occured between r198229-r198252. As far as I see, in this interval 
https://trac.webkit.org/changeset/198235 is the only one major JSC change which 
could cause this regression. I'll bisect it soon.
Comment 1 Csaba Osztrogonác 2016-03-16 06:06:39 PDT
Bisect finished, http://trac.webkit.org/changeset/198235 is the culprit.

- r198234
- https://build.webkit.org/builders/EFL%20Linux%20ARMv7%20Thumb2%20Release/builds/17504
- 22 JSC tests failed

- r198241
- https://build.webkit.org/builders/EFL%20Linux%20ARMv7%20Thumb2%20Release/builds/17505
- zillion failures

( I just reported this regression and don't have time to work 
on it. If somebody is interested in it, feel free to pick it up. )
Comment 2 Oliver Hunt 2016-03-16 07:36:18 PDT
Sorry will look at this in 30-40 minutes. Very confused :-/
Comment 3 Oliver Hunt 2016-03-16 07:37:49 PDT
Sorry will look at this in 30-40 minutes. Very confused. Are there any stack traces?
Comment 4 Oliver Hunt 2016-03-16 10:18:12 PDT
Your ARM build uses the on demand allocator, right?

I wonder if there's a bug in my control logic for this.
Comment 5 Csaba Osztrogonác 2016-03-16 10:19:41 PDT
(In reply to comment #4)
> Your ARM build uses the on demand allocator, right?
Yes.

> I wonder if there's a bug in my control logic for this.
Comment 6 Oliver Hunt 2016-03-16 10:20:53 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > Your ARM build uses the on demand allocator, right?
> Yes.
> 
> > I wonder if there's a bug in my control logic for this.

Are you able to make you test bot actually produce stack traces? This is very peculiar
Comment 7 Oliver Hunt 2016-03-16 10:22:05 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > Your ARM build uses the on demand allocator, right?
> > Yes.
> > 
> > > I wonder if there's a bug in my control logic for this.
> 
> Are you able to make you test bot actually produce stack traces? This is
> very peculiar

(Note, i am looking at this, a stack trace would just make it easier to work out where it's all going horribly wrong :-O)
Comment 8 Oliver Hunt 2016-03-16 10:26:45 PDT
Hmmm. Ossy, do you have build access for that configuration atm?

I'd be interested in knowing what happens in 
#else // ENABLE(EXECUTABLE_ALLOCATOR_FIXED)
static inline void* writeToExecutableRegion(void *dst, const void *src, size_t n)
{
    return memcpy(dst, src, n);
}
#endif

if you put an
ASSERT(((intptr_t)dst) & 1);

before the memcpy
Comment 9 Csaba Osztrogonác 2016-03-16 10:35:57 PDT
(In reply to comment #8)
> Hmmm. Ossy, do you have build access for that configuration atm?
> 
> I'd be interested in knowing what happens in 
> #else // ENABLE(EXECUTABLE_ALLOCATOR_FIXED)
> static inline void* writeToExecutableRegion(void *dst, const void *src,
> size_t n)
> {
>     return memcpy(dst, src, n);
> }
> #endif
> 
> if you put an
> ASSERT(((intptr_t)dst) & 1);
> 
> before the memcpy

Yes, I'll try it. But unfortunately the cross compiling and 
test running is quite slow, it will take at least 15-20 minutes.
Comment 10 Csaba Osztrogonác 2016-03-23 06:31:10 PDT
(In reply to comment #8)
> Hmmm. Ossy, do you have build access for that configuration atm?
> 
> I'd be interested in knowing what happens in 
> #else // ENABLE(EXECUTABLE_ALLOCATOR_FIXED)
> static inline void* writeToExecutableRegion(void *dst, const void *src,
> size_t n)
> {
>     return memcpy(dst, src, n);
> }
> #endif
> 
> if you put an
> ASSERT(((intptr_t)dst) & 1);
> 
> before the memcpy

I think you meant the negated condition. It wasn't easy, because
there are many assertions on ARMv7 due to bug154857. But I managed
to use release assert instead of, but it didn't hit on the tests
which crashed due to this bug.
Comment 11 Csaba Osztrogonác 2016-04-13 01:27:44 PDT
The original and fixed change was relanded and there is no similar issue now.