[Webkit-unassigned] [Bug 146440] Crash on xLarge memory allocation using bmalloc on 32bit systems

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=146440

--- Comment #17 from Mario Sanchez Prada <mario at webkit.org> ---
(In reply to comment #14)
> Comment on attachment 256038 [details]
> Patch proposal
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=256038&action=review
> 
> > Source/WebCore/CMakeLists.txt:3581
> > +if (CMAKE_COMPILER_IS_GNUCXX AND "${LOWERCASE_CMAKE_HOST_SYSTEM_PROCESSOR}" MATCHES "(i[3-6]86|x86)")
> 
> I think this should be: ... MATCHES "(i[3-6]86|x86)$")
> 
> Otherwise this becomes true also for AMD64 with GCC (where
> LOWERCASE_CMAKE_HOST_SYSTEM_PROCESSOR = x86_64 ). (yes, there is a bug on
> the previous code introduced on 128855 <http://trac.webkit.org/r128855>)

Good catch, I'll change that.

(In reply to comment #15)
> [...]
> > Otherwise this becomes true also for AMD64 with GCC (where
> > LOWERCASE_CMAKE_HOST_SYSTEM_PROCESSOR = x86_64 ). (yes, there is a bug on
> > the previous code introduced on 128855 <http://trac.webkit.org/r128855>)
> 
> Also I don't think this will work for ARM 32-bit machines or $RANDOMARCH
> 32-bit machines.

No, it won't work with ARM or other 32bit machines, but that's intentional: in the tests we did we saw this crash happening in Intel 32bit archs only but not on ARM, at least not in the case we tried. Thus, I'd rather keep this change for Intel 32bit only, because that's the only case where I'm sure the crash is happening.

(In reply to comment #16)
> Comment on attachment 256038 [details]
> Patch proposal
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=256038&action=review
> 
> Do you know how important this optimization is? My main concern, like with
> all other patches that disable optimizations to work around compiler bugs,
> is that it's very difficult to know when we can stop disabling it, and we
> end up with the optimization disabled forever. Also, I think we should
> report these bugs to GCC as well.

Other than the fact that it reliably gets WebKit to crash if enabled, I don't know how important this optimization exactly is, which why I always said this is not a proper fix, but a workaround. That said, I agree on that doing this poses the problems you mentioned and that GCC devs should be reported about that, which is what Michael did on https://bugzilla.redhat.com/show_bug.cgi?id=1225733 already.  

> [...]
> > Also I don't think this will work for ARM 32-bit machines or $RANDOMARCH 32-bit machines.
> 
> Yes, we do a similar match to add the --no-keep-memory linker option in 32
> bits. I think we could move it to a common place, detecting also non intel
> 32 bits processors and exposing a variable to CMake.

As I said above, doing this for Intel 32bit only was intentional, as that's the only configuration I'm sure the bug is happening, so I'm not sure about merging this with the --no-keep-memory flag.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150703/1a2a1fee/attachment-0001.html>