Incomplete testing on CRAN

[rstub]

While everything works on rhub's Solaris machine, some of the new tests fail on CRAN's Solaris machine:

Version: 0.1.0
Check: tests
Result: ERROR
     Running ‘testthat.R’ [25s/48s]
    Running the tests in ‘tests/testthat.R’ failed.
    Complete output:
     > library(testthat)
     > library(dqrng)
     >
     > test_check("dqrng")
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 49: Warning: Shift count is too large.
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 83: Where: While instantiating "unsigned short dqrng::convert_seed_internal<unsigned short, int, 32>(const int*, unsigned)".
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 83: Where: Instantiated from unsigned short dqrng::convert_seed<unsigned short>(const int*, unsigned).
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 88: Where: Instantiated from unsigned short dqrng::convert_seed<unsigned short>(Rcpp::Vector<13, PreserveStorage>).
     "convert.cpp", line 12: Where: Instantiated from std::string convert_base<unsigned short>(Rcpp::Vector<13, PreserveStorage>).
     "convert.cpp", line 20: Where: Instantiated from non-template code.
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 58: Warning: Shift count is too large.
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 83: Where: While instantiating "unsigned short dqrng::convert_seed_internal<unsigned short, int, 32>(const int*, unsigned)".
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 83: Where: Instantiated from unsigned short dqrng::convert_seed<unsigned short>(const int*, unsigned).
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 88: Where: Instantiated from unsigned short dqrng::convert_seed<unsigned short>(Rcpp::Vector<13, PreserveStorage>).
     "convert.cpp", line 12: Where: Instantiated from std::string convert_base<unsigned short>(Rcpp::Vector<13, PreserveStorage>).
     "convert.cpp", line 20: Where: Instantiated from non-template code.
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 49: Warning: Shift count is too large.
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 83: Where: While instantiating "unsigned dqrng::convert_seed_internal<unsigned, int, 32>(const int*, unsigned)".
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 83: Where: Instantiated from unsigned dqrng::convert_seed<unsigned>(const int*, unsigned).
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 88: Where: Instantiated from unsigned dqrng::convert_seed<unsigned>(Rcpp::Vector<13, PreserveStorage>).
     "convert.cpp", line 12: Where: Instantiated from std::string convert_base<unsigned>(Rcpp::Vector<13, PreserveStorage>).
     "convert.cpp", line 25: Where: Instantiated from non-template code.
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 58: Warning: Shift count is too large.
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 83: Where: While instantiating "unsigned dqrng::convert_seed_internal<unsigned, int, 32>(const int*, unsigned)".
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 83: Where: Instantiated from unsigned dqrng::convert_seed<unsigned>(const int*, unsigned).
     "/home/ripley/R/Lib32/dqrng/include/convert_seed.h", line 88: Where: Instantiated from unsigned dqrng::convert_seed<unsigned>(Rcpp::Vector<13, PreserveStorage>).
     "convert.cpp", line 12: Where: Instantiated from std::string convert_base<unsigned>(Rcpp::Vector<13, PreserveStorage>).
     "convert.cpp", line 25: Where: Instantiated from non-template code.
     4 Warning(s) detected.
    
     *** caught segfault ***
     address 0, cause 'memory not mapped'

 Traceback:
     1: convert_16(-1)
     2: eval_bare(get_expr(quo), get_env(quo))
     3: doTryCatch(return(expr), name, parentenv, handler)
     4: tryCatchOne(expr, names, parentenv, handlers[[1L]])
     5: tryCatchList(expr, classes, parentenv, handlers)
     6: tryCatch({ code NULL}, error = function(e) e)
     7: capture(act$val <- eval_bare(get_expr(quo), get_env(quo)))
     8: quasi_capture(enquo(object), capture_error, label = label)
     9: expect_error(convert_16(-1), "seed element out of range")
     10: eval(code, test_env)
     11: eval(code, test_env)
     12: 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)
     13: doTryCatch(return(expr), name, parentenv, handler)
     14: tryCatchOne(expr, names, parentenv, handlers[[1L]])
     15: tryCatchList(expr, names[-nh], parentenv, handlers[-nh])
     16: doTryCatch(return(expr), name, parentenv, handler)
     17: tryCatchOne(tryCatchList(expr, names[-nh], parentenv, handlers[-nh]), names[nh], parentenv, handlers[[nh]])
     18: tryCatchList(expr, classes, parentenv, handlers)
     19: 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) { })
     20: test_code(desc, code, env = parent.frame())
     21: test_that("conversion to 16-bit integers works correctly", { expect_identical(convert_16(0), "0") expect_identical(convert_16(65535), "65535") expect_identical(convert_16(c(0, 0, 0)), "0") expect_identical(convert_16(c(0, 65535)), "65535") expect_identical(convert_16(c(0, 0, 65535)), "65535") expect_error(convert_16(-1), "seed element out of range") expect_error(convert_16(NA_integer_), "seed element out of range") expect_error(convert_16(65536), "seed element out of range") expect_error(convert_16(c(1, 0)), "vector implies an out-of-range seed")})
     22: eval(code, test_env)
     23: eval(code, test_env)
     24: 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)
     25: doTryCatch(return(expr), name, parentenv, handler)
     26: tryCatchOne(expr, names, parentenv, handlers[[1L]])
     27: tryCatchList(expr, names[-nh], parentenv, handlers[-nh])
     28: doTryCatch(return(expr), name, parentenv, handler)
     29: tryCatchOne(tryCatchList(expr, names[-nh], parentenv, handlers[-nh]), names[nh], parentenv, handlers[[nh]])
     30: tryCatchList(expr, classes, parentenv, handlers)
     31: 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) { })
     32: test_code(NULL, exprs, env)
     33: source_file(path, new.env(parent = env), chdir = TRUE, wrap = wrap)
     34: force(code)
     35: with_reporter(reporter = reporter, start_end_reporter = start_end_reporter, { lister$start_file(basename(path)) source_file(path, new.env(parent = env), chdir = TRUE, wrap = wrap) end_context() })
     36: FUN(X[[i]], ...)
     37: lapply(paths, test_file, env = env, reporter = current_reporter, start_end_reporter = FALSE, load_helpers = FALSE, wrap = wrap)
     38: force(code)
     39: with_reporter(reporter = current_reporter, results <- lapply(paths, test_file, env = env, reporter = current_reporter, start_end_reporter = FALSE, load_helpers = FALSE, wrap = wrap))
     40: test_files(paths, reporter = reporter, env = env, stop_on_failure = stop_on_failure, stop_on_warning = stop_on_warning, wrap = wrap)
     41: test_dir(path = test_path, reporter = reporter, env = env, filter = filter, ..., stop_on_failure = stop_on_failure, stop_on_warning = stop_on_warning, wrap = wrap)
     42: test_package_dir(package = package, test_path = test_path, filter = filter, reporter = reporter, ..., stop_on_failure = stop_on_failure, stop_on_warning = stop_on_warning, wrap = wrap)
     43: test_check("dqrng")

CC: @LTLA

[LTLA]

Hm. Is the mere presence of UB code sufficient to cause the segfault, even if it never gets executed? I will try to circumvent this at compile-time with by using an specialized template function for the shift that simply returns zero (or does nothing) for the most common (yet invalid) integer size/SHIFT combinations.

[rstub]

I am not sure about the UB code causing these problems. My current theory is as follows:

• From the stacktrace we learn that convert_16(-1) triggered the segfault. That is the first test that produces an error.
• The warning messages have a different wording than those from gcc, making it plausible that the compiler from Oracle Developer Studio is used.
• On rhub, the wording of the warnings indicate usage of gcc, c.f. https://artifacts.r-hub.io/dqrng_0.1.0.tar.gz-c6fb11ef8cef4098ba367bc6053a6e4d/dqrng.Rcheck/tests/testthat.Rout
• The error cases use std::out_of_range("..."), but that particular constructor was only added in C++11, c.f. https://en.cppreference.com/w/cpp/error/out_of_range.
• The compiler from Oracle Developer Studio is known to be rather strict w.r.t. C++ standards and probably does not use C++11 by default, as opposed to recent gcc/clang versions.

So maybe it might be sufficient to add // [[Rcpp::plugins(cpp11)]] to convert.cpp. What do you think?

[LTLA]

I thought we already had C++11 turned on in convert.cpp. Hard to imagine anything compiling without it, given the constexprs in convert_seed.h.

[rstub]

You are correct, C++11 is requested right in the first line of the file.

I still find it interesting that it fails for convert_16(-1), and that CRAN seems to use a different compiler than rhub.

[LTLA]

I've been reading up about UB and it's pretty crazy. They weren't joking when they say that the compiler can do literally anything it wants. Possibly even more so if it knows that it's UB at compile time.

Getting rid of the UB may or may not fix it, but at least we don't have any warnings anymore. That's one less thing we need to argue about if we need to escalate this to the CRAN maintainers.

[rstub]

Interestingly, meanwhile MacOS also reports ERRORS:

Result: ERROR
     Running ‘testthat.R’ [24s/24s]
    Running the tests in ‘tests/testthat.R’ failed.
    Last 13 lines of output:
     `convert_64(c(1, 1, 0))` threw an error with unexpected message.
     Expected match: "vector implies an out-of-range seed"
     Actual message: "c++ exception (unknown reason)"
    
     ══ testthat results ═══════════════════════════════════════════════════════════
     OK: 79 SKIPPED: 0 FAILED: 6
     1. Failure: conversion to 16-bit integers works correctly (@test-convert.R#15)
     2. Failure: conversion to 16-bit integers works correctly (@test-convert.R#16)
     3. Failure: conversion to 16-bit integers works correctly (@test-convert.R#17)
     4. Failure: conversion to 16-bit integers works correctly (@test-convert.R#18)
     5. Failure: conversion to 32-bit integers works correctly (@test-convert.R#33)
     6. Failure: conversion to 64-bit integers works correctly (@test-convert.R#55)
    
     Error: testthat unit tests failed
     Execution halted 

This is odd, since MacOS is tested via Travis CI.

Anyway, the failed tests are all the ones that check for error conditions. So my suggestion would be to just skip those tests on CRAN. I don't like that, but I see no better immediate solution without the ability to reproduce the issues first.

[LTLA]

Is this with the shift-avoiding code? Might be worth one last roll of the dice, especially if it is some UB. I suppose UBSan doesn't pick it up because it's not part of the dqrng shared library.

[rstub]

The tests on CRAN use the original version (it typically takes a few days for all CRAN machines to build a new or updated package). However, I have meanwhile been able to reproduce the MacOS failure with the most recent version. In addition, one of the failure happens for 64bit integers, where there is no UB.

[LTLA]

Possibly relevant:

https://discourse.mc-stan.org/t/error-in-stanc-c-exception-unknown-reason/2120/6

[rstub]

That looks very relevant. And it fits to my recent experiences. Even the examples from http://gallery.rcpp.org/articles/intro-to-exceptions/ do not work properly on MacOS (CRAN build with matching clang). Only Rcpp::stop does work properly. I guess a custom exception class derived from std::exception would also work. So for MacOS I see four possibilities:

1. Skip the exception checking tests (on CRAN).
2. Dump down the test, i.e. only expect an exception, not any particular output.
3. Use Rcpp::stop in an otherwise Rcpp-independent piece of code.
4. Introduce a custom exception class derived from std::exception.

I don't like 2. and 3. The cleanest solution would b 4. (if it works). And 1. is the easiest solution, but also the one most likely to help with the Solaris ERROR. I am currently leaning towards 1.

[LTLA]

Yes, agree that 1 is the best, if we can just do the skips on OSX. Alternatively, we could write a wrapper around expect_error that just does 2 for OSX only, something like:

safe_expect_error <- function(..., msg) {
   os.type <- Sys.info()["sysname"]    
   if (os.type=="Darwin") {
       expect_error(...)
    } else {
       expect_error(..., msg)
    }
}

[rstub]

Thanks, I like the safe_except_error and put it into #17. I have also put in a skip_on_os("solaris") for all the exception-triggering tests. I am not in the mood for gambling ... ;-)

Upload on Monday, since one build result is still missing and it avoids a NOTE.

[rstub]

Skipping on Solaris was successful. Whether it is necessary or merely sufficient is a different question. I meanwhile fear that it is some sort of ABI incompatibility triggered by compilation with the Oracle compiler instead of g++, c.f. https://stackoverflow.com/questions/54492667/segfault-when-throwing-stdruntime-error-on-ubuntu-xenial-with-rcpp.

[rstub]

Version 0.1.1 has been build on CRAN on all platforms. However, there are currently two limitations concerning the tests that deliberately throw a C++ exception:

• On MacOS the actual message from the exception is not checked.
• On Solaris these tests are skipped completely.

It would be nice if a better solution could be found.