jeroen / V8

Embedded JavaScript Engine for R
https://cran.r-project.org/package=V8
Other
201 stars 29 forks source link

tests fail with v8 8.1.63 #78

Closed JanMarvin closed 4 years ago

JanMarvin commented 4 years ago

Just to inform you, recent changes cause a crash in v8.

==> devtools::test()

Loading V8
Using V8 engine 8.1.63
Testing V8
✓ |  OK F W S | Context
⠏ |   0       | JS ArrayBuffers
 *** caught segfault ***
address (nil), cause 'unknown'

Traceback:
 1: .Call("_V8_context_eval", PACKAGE = "V8", src, ctx, serialize)
 2: context_eval(join(src), private$context, serialize)
 3: identical(str, "undefined")
 4: get_str_output(context_eval(join(src), private$context, serialize))
 5: evaluate_js(name, serialize = TRUE)
 6: get_json_output(evaluate_js(name, serialize = TRUE), ...)
 7: ctx$get("data")
 8: eval_bare(expr, quo_get_env(quo))
 9: quasi_label(enquo(object), label, arg = "object")
10: expect_equal(ctx$get("data"), 1:3)
11: eval(code, test_env)
12: eval(code, test_env)
13: withCallingHandlers({    eval(code, test_env)    if (!handled && !is.null(test)) {        skip_empty()    }}, expectation = handle_expectation, skip = handle_skip, warning = handle_warning,     message = handle_message, error = handle_error)
14: doTryCatch(return(expr), name, parentenv, handler)
15: tryCatchOne(expr, names, parentenv, handlers[[1L]])
16: tryCatchList(expr, names[-nh], parentenv, handlers[-nh])
17: doTryCatch(return(expr), name, parentenv, handler)
18: tryCatchOne(tryCatchList(expr, names[-nh], parentenv, handlers[-nh]),     names[nh], parentenv, handlers[[nh]])
19: tryCatchList(expr, classes, parentenv, handlers)
20: tryCatch(withCallingHandlers({    eval(code, test_env)    if (!handled && !is.null(test)) {        skip_empty()    }}, expectation = handle_expectation, skip = handle_skip, warning = handle_warning,     message = handle_message, error = handle_error), error = handle_fatal,     skip = function(e) {    })
21: test_code(desc, code, env = parent.frame())
22: test_that("Reading and writing raw buffers", {    ctx <- V8::v8()    ctx$eval("\nvar data = [1,2,3]\nvar intArray = new Uint8Array( data )\nvar dataBuffer = new ArrayBuffer( 3 )\nvar floatArray = new Float32Array( [0, Math.PI])")    expect_equal(ctx$get("data"), 1:3)    expect_equal(ctx$get("dataBuffer"), raw(3))    expect_equal(ctx$get("floatArray"), as.raw(c(0, 0, 0, 0,         219, 15, 73, 64)))    expect_equal(ctx$get("intArray"), as.raw(1:3))    expect_equal(ctx$eval("dataBuffer"), "[object ArrayBuffer]")    expect_equal(ctx$eval("floatArray.buffer"), "[object ArrayBuffer]")    expect_equal(ctx$eval("intArray.buffer"), "[object ArrayBuffer]")})
23: eval(code, test_env)
24: eval(code, test_env)
25: withCallingHandlers({    eval(code, test_env)    if (!handled && !is.null(test)) {        skip_empty()    }}, expectation = handle_expectation, skip = handle_skip, warning = handle_warning,     message = handle_message, error = handle_error)
26: doTryCatch(return(expr), name, parentenv, handler)
27: tryCatchOne(expr, names, parentenv, handlers[[1L]])
28: tryCatchList(expr, names[-nh], parentenv, handlers[-nh])
29: doTryCatch(return(expr), name, parentenv, handler)
30: tryCatchOne(tryCatchList(expr, names[-nh], parentenv, handlers[-nh]),     names[nh], parentenv, handlers[[nh]])
31: tryCatchList(expr, classes, parentenv, handlers)
32: tryCatch(withCallingHandlers({    eval(code, test_env)    if (!handled && !is.null(test)) {        skip_empty()    }}, expectation = handle_expectation, skip = handle_skip, warning = handle_warning,     message = handle_message, error = handle_error), error = handle_fatal,     skip = function(e) {    })
33: test_code(NULL, exprs, env)
34: source_file(path, new.env(parent = env), chdir = TRUE, wrap = wrap)
35: force(code)
36: doWithOneRestart(return(expr), restart)
37: withOneRestart(expr, restarts[[1L]])
38: withRestarts(testthat_abort_reporter = function() NULL, force(code))
39: with_reporter(reporter = reporter, start_end_reporter = start_end_reporter,     {        reporter$start_file(basename(path))        lister$start_file(basename(path))        source_file(path, new.env(parent = env), chdir = TRUE,             wrap = wrap)        reporter$.end_context()        reporter$end_file()    })
40: FUN(X[[i]], ...)
41: lapply(paths, test_file, env = env, reporter = current_reporter,     start_end_reporter = FALSE, load_helpers = FALSE, wrap = wrap)
42: force(code)
43: doWithOneRestart(return(expr), restart)
44: withOneRestart(expr, restarts[[1L]])
45: withRestarts(testthat_abort_reporter = function() NULL, force(code))
46: with_reporter(reporter = current_reporter, results <- lapply(paths,     test_file, env = env, reporter = current_reporter, start_end_reporter = FALSE,     load_helpers = FALSE, wrap = wrap))
47: test_files(paths, reporter = reporter, env = env, stop_on_failure = stop_on_failure,     stop_on_warning = stop_on_warning, wrap = wrap)
48: (function (path, filter = NULL, reporter = default_reporter(),     env = test_env(), ..., encoding = "unknown", load_helpers = TRUE,     stop_on_failure = FALSE, stop_on_warning = FALSE, wrap = TRUE) {    if (!missing(encoding) && !identical(encoding, "UTF-8")) {        warning("`encoding` is deprecated; all files now assumed to be UTF-8",             call. = FALSE)    }    testthat_dir <- maybe_root_dir(path)    withr::local_envvar(list(R_TESTS = "", TESTTHAT = "true",         TESTTHAT_DIR = testthat_dir))    if (load_helpers) {        source_test_helpers(path, env)    }    source_test_setup(path, env)    on.exit(source_test_teardown(path, env), add = TRUE)    if (identical(Sys.getenv("NOT_CRAN"), "true")) {        withr::local_options(list(lifecycle_verbose_retirement = TRUE))    }    paths <- find_test_scripts(path, filter, ...)    test_files(paths, reporter = reporter, env = env, stop_on_failure = stop_on_failure,         stop_on_warning = stop_on_warning, wrap = wrap)})("/home/jmg/Source/V8/tests/testthat", filter = NULL, env = <environment>,     stop_on_failure = FALSE, load_helpers = FALSE)
49: do.call(testthat::test_dir, testthat_args)
50: force(code)
51: withr::with_envvar(c(r_env_vars(), TESTTHAT_PKG = pkg$package),     do.call(testthat::test_dir, testthat_args))
52: force(code)
53: withr::with_options(c(useFancyQuotes = FALSE), withr::with_envvar(c(r_env_vars(),     TESTTHAT_PKG = pkg$package), do.call(testthat::test_dir,     testthat_args)))
54: devtools::test()
An irrecoverable exception occurred. R is aborting now ...
jeroen commented 4 years ago

Pfff incredible how they manage to change the API every time.

JanMarvin commented 4 years ago

https://github.com/jeroen/V8/commit/6e6f59369c14dbef978d60882d026431676ed1c3 is causing the error. The change in bindings.cpp lines 148

jeroen commented 4 years ago

Thanks! IsUndefined() again 🤔 Can you see which of these tests causes the segfault?

  ctx <- V8::v8()
  expect_equal(ctx$eval('function I(x){return x}'), 'undefined')
  expect_null(ctx$eval("undefined", TRUE))
  expect_null(ctx$eval("null", TRUE))
  expect_equal(ctx$eval("[]", TRUE), "[]")
  expect_equal(ctx$eval('I({})', TRUE), "{}")
  expect_equal(ctx$eval("1", TRUE), "1")
  expect_equal(ctx$eval(1, TRUE), "1")
  expect_equal(ctx$eval('new Uint8Array( [1,2] )', TRUE), as.raw(1:2))
  expect_equal(ctx$eval('new Uint8Array( [1,2] ).buffer', TRUE), as.raw(1:2))
  expect_error(ctx$eval('doesnotexist', TRUE), 'doesnotexist', class = "std::runtime_error")
JanMarvin commented 4 years ago

commented the crashing ones

ctx <- V8::v8()
expect_equal(ctx$eval('function I(x){return x}'), 'undefined')
# expect_null(ctx$eval("undefined", TRUE))
# expect_null(ctx$eval("null", TRUE))
# expect_equal(ctx$eval("[]", TRUE), "[]")
# expect_equal(ctx$eval('I({})', TRUE), "{}")
expect_equal(ctx$eval("1", TRUE), "1")
expect_equal(ctx$eval(1, TRUE), "1")
# expect_equal(ctx$eval('new Uint8Array( [1,2] )', TRUE), as.raw(1:2))
# expect_equal(ctx$eval('new Uint8Array( [1,2] ).buffer', TRUE), as.raw(1:2))
expect_error(ctx$eval('doesnotexist', TRUE), 'doesnotexist', class = "std::runtime_error")

edit: but other tests fail as well :man_shrugging: edit2: if I keep either IsNull() or IsUndefined() R crashes, if I remove the if-clause all but 2 tests succeed.

jeroen commented 4 years ago

I'm trying to build V8 8.1 on homebrew but the build fails. Do you happen to know which of the external resources that need an update to build the latest V8? https://github.com/homebrew/homebrew-core/blob/master/Formula/v8.rb

JanMarvin commented 4 years ago

No sorry, I assume if it builds any V8 7.x release it should be fine.

jeroen commented 4 years ago

Can you test if the latest commit solves anything?

JanMarvin commented 4 years ago

No, it does not. The call ->IsNull() or ->IsUndefined() cause the crash.

jeroen commented 4 years ago

Yeah I was hoping we would not get to that call in case of value.IsEmpty()

jeroen commented 4 years ago

Looks like I'm going to first have to figure out how to fix the libv8 build 8.1 on macos to debug this... the problem is a new requirement on zlib which you have addressed here: https://github.com/JanMarvin/v8-R/commit/526cf82f09633911b4fb999721763f31fde9aeec

JanMarvin commented 4 years ago

Ah yes, completely forgot about that. Somehow I assumed that it's Arch Linux specific

jeroen commented 4 years ago

I have committed a workaround. But still haven't figured out the real problem. Looks like a bug in v8...

JanMarvin commented 4 years ago

Thanks will test it tomorrow. Wish you a nice Christmas! :gift:

jeroen commented 4 years ago

Posted the issue in the v8-dev forum: https://groups.google.com/forum/#!topic/v8-dev/N9BpiGxPaQs

Happy xmas to you as well 🎄

jeroen commented 4 years ago

Were you able to confirm if compiling with V8_ENABLE_CHECKS resolves the crash for you?

Also could you maybe test if the test program that I attached to this email in the v8 forums also crashes on arch? https://groups.google.com/forum/#!topic/v8-dev/N9BpiGxPaQs

JanMarvin commented 4 years ago

Sorry for the late reply: V8_ENABLE_CHECKS resolves the issue for me and the hello-crash program crashes for me too (had to change v8 includes). I assume the issue is somehow related to this bug: https://bugs.chromium.org/p/v8/issues/detail?id=10041&q=&colspec=ID%20Type%20Status%20Priority%20Owner%20Summary%20HW%20OS%20Component%20Stars

GNU gdb (GDB) 8.3.1
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./a.out...
(gdb) run
Starting program: /tmp/a.out 
/usr/lib/../share/gcc-9.2.0/python/libstdcxx/v6/xmethods.py:731: SyntaxWarning: list indices must be integers or slices, not str; perhaps you missed a comma?
  refcounts = ['_M_refcount']['_M_pi']
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".
[New Thread 0x7ffff4b23700 (LWP 2241)]
[New Thread 0x7ffff4322700 (LWP 2242)]
[New Thread 0x7ffff3b21700 (LWP 2243)]
[New Thread 0x7ffff3320700 (LWP 2244)]
[New Thread 0x7ffff2b1f700 (LWP 2245)]
[New Thread 0x7ffff231e700 (LWP 2246)]
[New Thread 0x7ffff1b1d700 (LWP 2247)]
[New Thread 0x7ffff131c700 (LWP 2248)]

Thread 1 "a.out" received signal SIGSEGV, Segmentation fault.
v8::internal::Internals::GetInstanceType (obj=15221498847309) at /usr/include/v8-internal.h:233
233     return ReadRawField<uint16_t>(map, kMapInstanceTypeOffset);
(gdb) backtrace
#0  v8::internal::Internals::GetInstanceType (obj=15221498847309) at /usr/include/v8-internal.h:233
#1  v8::Value::QuickIsUndefined (this=0x5555555f5db0) at /usr/include/v8.h:11310
#2  v8::Value::IsUndefined (this=0x5555555f5db0) at /usr/include/v8.h:11301
#3  main (argc=1, argv=0x7fffffffe8a8) at hello-crash.cc:53

Edit: If I compile hello-crash with -DV8_ENABLE_CHECKS that works as well. Checked on V8 8.1.80

jeroen commented 4 years ago

I'm considering reverting https://github.com/jeroen/V8/pull/76 because the real problem is probably fixed by V8_ENABLE_CHECKS...

JanMarvin commented 4 years ago

No worries, I'm waiting with updates to the aur package until the issue is resolved. Happy new year!

jeroen commented 4 years ago

Some upstream progress: https://bugs.chromium.org/p/v8/issues/detail?id=10041

jeroen commented 4 years ago

FYI, I have pushed out a new CRAN release to make sure users don't get hit by this issue.

JanMarvin commented 4 years ago

AUR package is updated, thanks!

JanMarvin commented 4 years ago

Upstream has made further progress, this breaks building with -DV8_ENABLE_CHECKS and requires -DV8_COMPRESS_POINTERS or it will fail with the pointer compression mismatch error.

https://bugs.chromium.org/p/v8/issues/detail?id=10041#c18

JanMarvin commented 4 years ago

For clarification: v8 defaults to pointer compression which caused crashes. to avoid this pointer compression was disabled on the R side (#78). This does not work anymore with v8 after https://github.com/v8/v8/commit/2db93c0233790b2cfb9e80fa1fa89dee18c7109f. Now v8 compares it's internal handling of pointer compression to the embedders pointer compression. If they don't match the embedding program crashes.