[R-pkg-devel] Fwd: CRAN Submission rkriging 1.0.1

[Ivan Krylov]

Hi Chaofan and welcome to R-package-devel!

В Wed, 17 Jul 2024 11:52:53 -0700
Bill Huang <10billhuang01 using gmail.com> пишет:

> The package can be installed successfully locally and passed CRAN
> auto-check. However, it cannot pass the gcc-UBSAN test with the
> following error message: a problem with the LLT function in Eigen. I
> am not sure how to fix this.

The problem is detected by special, normally disabled instrumentation
called UBSanitizer [1]. If you don't want to compile R with sanitizers
enabled yourself, ready-made Docker/Podman containers
rocker-org/r-devel-san and rocker-org/r-devel-san-clang [2] (and
others, there are sanitizer containers published by the R-hub
project as well) should help you reproduce the problem.

In this case, I suspect that you stumbled upon a problem in Eigen
itself: the Eigen::LLT constructor does not initialise an enumeration
field, ComputationInfo m_info, which results in the field being
populated by something out of range of the enumeration, which invokes
undefined behaviour.

You can reproduce the problem without building an R package at all:

#include <iostream>
#include <Eigen>

struct foo : Eigen::LLT<Eigen::MatrixXd> {
	foo() : Eigen::LLT<Eigen::MatrixXd>(42) {
		// m_info is protected, hence use of inheritance
		std::cout << m_info << std::endl;
	}
};

int main() {
	foo L;
}

g++ -fsanitize=undefined -g -Og -I $LIBRARY/RcppEigen/include/Eigen \
 -o ex ex.cpp
./ex
# ex.cpp:7:16: runtime error: load of value 32608, which is not a valid
# value for type 'ComputationInfo'
# 32608

A workaround for the problem is to move the initialization of the
Eigen::LLT<Eigen::MatrixXd> Kriging::L_ field from the constructor body
into the member initializer list:

--- rkriging.orig/src/kriging.cpp       2024-07-04 06:45:57.000000000 +0300
+++ rkriging/src/kriging.cpp    2024-07-18 14:10:54.000000000 +0300
@@ -20,10 +20,9 @@
     return kriging->get_nllh(log_lengthscale, nugget);
 }

-Kriging::Kriging(const Eigen::MatrixXd& X, const Eigen::VectorXd& y, Kernel& Ker, const bool& interpolation): n_(X.rows()), p_(X.cols()), X_(X), y_(y), Ker_(Ker), interpolation_(interpolation) {
+Kriging::Kriging(const Eigen::MatrixXd& X, const Eigen::VectorXd& y, Kernel& Ker, const bool& interpolation): n_(X.rows()), p_(X.cols()), X_(X), y_(y), Ker_(Ker), interpolation_(interpolation), L_(n_) {
     a_.resize(n_);
     b_.resize(n_);
-    L_ = Eigen::LLT<Eigen::MatrixXd>(n_);
     y_tss_ = y_.squaredNorm() - std::pow(y_.sum(),2)/n_;
     nugget_ = interpolation_ ? 1e-6 : 1e-3;
     nllh_ = std::numeric_limits<double>::infinity();

This seems to avoid poking the bear of undefined behaviour with
potential copying of invalid enum value in L_ until something
initialises it properly.

I'm not sure whether this is worth reporting to Eigen as something to
fix. I hope that people here with more experience in C++ and Eigen will
be able to answer that question.

-- 
Best regards,
Ivan

[1]
https://cran.r-project.org/doc/manuals/R-exts.html#Using-Undefined-Behaviour-Sanitizer

[2]
https://rocker-project.org/images/base/r-devel.html