phpv8 / v8js

V8 Javascript Engine for PHP — This PHP extension embeds the Google V8 Javascript Engine
http://pecl.php.net/package/v8js
MIT License
1.84k stars 200 forks source link

V8 crashing on toLocaleString() #306

Closed temuri416 closed 7 years ago

temuri416 commented 7 years ago

The following expression crashes V8JS:

var date = new Date("10/11/2009"),
    locale = "en-us",
    month = date.toLocaleString(locale, { month: "long" });

Results in:

 kernel: [10936.602251] traps: php-fpm[8516] trap invalid opcode ip:7f9239f2526f sp:7ffff8f825f8 error:0 in libv8_libbase.so[7f9239f16000+14000]

The problem is toLocaleString() call.

I realize it's not the fault of V8JS... But maybe @stesie has an idea of what's happening? It was fine with 5.5.x versions of V8.

Thanks!

temuri416 commented 7 years ago

just as I suspected - v8_enable_i18n_support = false should also be supplied during configuration...

now everything works fine.

EDIT. Nope, .toLocaleString() no longer crashes V8, but does not return month name :(

temuri416 commented 7 years ago

Decided to reopen... V8 itself handles toLocaleString() correctly:

Running:

/tmp/v8/out.gn/x64.release/d8

image

After compiling V8JS running the following:

$v8 = new V8Js;
$expr = 'new Date("10/11/2009").toLocaleString("en-us", { month: "long" });';
echo $v8->executeString($expr, null, V8Js::FLAG_FORCE_ARRAY);

results in:

#
# Fatal error in , line 0
# Failed to create ICU date format, are ICU data files missing?
#

==== C stack trace ===============================

    /opt/v8/lib/libv8_libbase.so(v8::base::debug::StackTrace::StackTrace()+0xe) [0x7f7874e1b32e]
    /opt/v8/lib/libv8_libplatform.so(+0x61b5) [0x7f7874e281b5]
    /opt/v8/lib/libv8_libbase.so(V8_Fatal+0xdd) [0x7f7874e191ed]
    /opt/v8/lib/libv8.so(+0x5a3ed0) [0x7f786fac4ed0]
    /opt/v8/lib/libv8.so(+0x7537bd) [0x7f786fc747bd]
    [0x39243ffbda18]
Received signal 4 ILL_ILLOPN 7f7874e1a26f
Illegal instruction (core dumped)

V8 compiles icudtl.dat and places it alongside libv8.so. Before compiling V8JS I copied icudtl.dat to /opt/v8/lib.

Could this be helpful?

https://groups.google.com/forum/embed/#!topic/v8-users/4DOD6z57xfU

Do you call V8::InitializeICU() or V8::InitializeICUDefaultLocation() before V8::Initialize()?

https://github.com/phpv8/v8js/blob/php7/v8js_v8.cc#L80

Any idea?

Thanks!

duxet commented 7 years ago

@temuri416 Same issue with the newest build of my Docker image:

#
# Fatal error in , line 0
# Failed to create ICU date format, are ICU data files missing?
#

==== C stack trace ===============================

    /usr/v8/lib/libv8_libbase.so(+0x1044e) [0x7f32fc53844e]
    /usr/v8/lib/libv8_libbase.so(V8_Fatal+0xdf) [0x7f32fc5363af]
    /usr/v8/lib/libv8.so(+0x5903b0) [0x7f32ef89c3b0]
    /usr/v8/lib/libv8.so(+0x75310d) [0x7f32efa5f10d]
    [0x1f5b578b5fc2]
Illegal instruction
stesie commented 7 years ago

I've got this to work on MacOS and Linux so far, see #307.

Yet I wonder how to fix it on Windows. @Jan-E could you please check whether the build of V8 on Windows also yields a icudtl.dat file? Your ZIP archives seem to not ship it, but I'd suppose it to be there

Jan-E commented 7 years ago

There was an icudtl.dll in the V8 versions 4.9, 5.1 and 5.4 and probably the versions in between as well. But it was not there anymore in 5.5.372.19 and more recent.

https://travis-ci.org/phpv8/v8js/builds/222213690 tells me you are using old versions of V8. You may have a task ahead; make it work in V8 5.5+ on Linux and MacOS

stesie commented 7 years ago

@Jan-E well yes, on Travis (Linux) I'm still using pretty old versions of V8. #307 actually fixes the bug there, yet Travis doesn't show it (but locally). For V8 5.3+ you need to point V8 to a icudtl.dat file (dat for data, not the dll file)

I'm asking regarding the windows build (https://ci.appveyor.com/project/stesie/v8js/build/1.0.180/job/pnyny5416tqnc9ko in particular), ... there I'm using the 5.8.301.0 build you've once provided. That one has V8 + icu ... yet the zip archive is missing a icudtl.dat file. However the error message pretty clearly says that it's missing a data file (also on Windows)

Jan-E commented 7 years ago

Aha, confused the dat and the dll. Did you try adding the Linux or macOS dat file to my zip? The dat file might be OS-independent...

Anyway, I did not save the build tree, so I am recompiling 5.8-lkgr at the moment. V8 5.8.283.31 that is.

Jan-E commented 7 years ago

Found it:

 Directory of D:\chromium\v8\v5.8\x64.release

05/04/17  06:50        10.166.816 icudtl.dat

It will take quite some time before I've got new builds. It is a lengthy process. However, it is the same as in

 Directory of D:\chromium\v8\third_party\icu\common

05/04/17  06:50        10.166.816 icudtl.dat

which seems to confirm that it is OS-independent.

Jan-E commented 7 years ago

https://phpdev.toolsforresearch.com/V8-5.8.283.31-x64.zip The zip also contains the exe's for the unittests.

x86 is on its way.

Jan-E commented 7 years ago

https://phpdev.toolsforresearch.com/V8-5.8.283.31-x86.zip

Jan-E commented 7 years ago

I did the test myself: https://github.com/phpv8/v8js/pull/309 https://ci.appveyor.com/project/stesie/v8js/build/1.0.181 Hooray!

ghost commented 6 years ago

Hi @Jan-E This seems to be the same issue with your build for php7.1.14 Can you please check it out? Much appreciated :-)

ghost commented 6 years ago

Oh I got it now... We have to specify the file on the php.ini

v8js.icudtl_dat_path = "path/to/icudtl.dat"

gempir commented 6 years ago

I've got this exact same issue, but specifying the path did not help at all.

dmytro-y-dev commented 6 years ago

I followed this instruction to build v8: https://github.com/phpv8/v8js/blob/master/README.Linux.md

Also had same issue with toLocaleString during make test.

I copied icudtl.dat to /opt/v8/lib folder and now make test is green and passes.