Skip to content

Improvements for unwind-protect #877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,38 @@

2018-07-10 Lionel Henry <lionel@rstudio.com>

* inst/include/Rcpp/exceptions.h: Move LongjumpException from the
Rcpp::internal namespace to the public Rcpp namespace. If you have a
catch-all statement like `catch (...)`, be sure to catch and rethrow
Rcpp::LongjumpException to prevent the R longjump from being ignored.
* inst/include/Rcpp/macros/macros.h (VOID_END_RCPP): idem
* src/attributes.cpp: idem

2018-07-05 Lionel Henry <lionel@rstudio.com>

* inst/include/Rcpp/api/meat/Rcpp_eval.h: Rename `RCPP_PROTECTED_EVAL`
to `RCPP_USE_UNWIND_PROTECT` because the new API is now more general
than just evaluation of R code.
* inst/NEWS.Rd: idem
* inst/unitTests/runit.interface.R: idem

2018-07-05 Lionel Henry <lionel@rstudio.com>

* R/unit.tests.R (unitTestSetup): Pass extra arguments to sourceCpp()
for easier debugging.

2018-07-05 Lionel Henry <lionel@rstudio.com>

* R/Attributes.R (.plugins[["unwindProtect"]]): You can now add
`[[Rcpp::plugins(unwindProtect)]]` in one of your source file to enable
the new unwind-protect mechanism easily. It appends
`-DRCPP_USE_UNWIND_PROTECT` to `PKG_CPPFLAGS`.

This is safer than using a `#define` because it ensures unwind-protect
is enabled in all compilation units, including RcppExports.cpp.

* inst/unitTests/cpp/stack.cpp: Use new plugin to enable unwind-protect.

2018-06-22 Kevin Ushey <kevinushey@gmail.com>

* inst/include/Rcpp/api/meat/Rcpp_eval.h: Ensure R_BaseEnv is used
Expand Down
4 changes: 4 additions & 0 deletions R/Attributes.R
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,10 @@ compileAttributes <- function(pkgdir = ".", verbose = getOption("verbose")) {
PKG_LIBS="-fopenmp"))
}

.plugins[["unwindProtect"]] <- function() {
list(env = list(PKG_CPPFLAGS = "-DRCPP_USE_UNWIND_PROTECT"))
}

# register a plugin
registerPlugin <- function(name, plugin) {
.plugins[[name]] <- plugin # #nocov
Expand Down
5 changes: 3 additions & 2 deletions R/unit.tests.R
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,15 @@ test <- function(output=if(file.exists("/tmp")) "/tmp" else getwd(),
}

unitTestSetup <- function(file, packages=NULL,
pathToRcppTests=system.file("unitTests", package = "Rcpp")) {
pathToRcppTests=system.file("unitTests", package = "Rcpp"),
...) {
function() {
if (! is.null(packages)) {
for (p in packages) {
suppressMessages(require(p, character.only=TRUE))
}
}
sourceCpp(file.path(pathToRcppTests, "cpp", file))
sourceCpp(file.path(pathToRcppTests, "cpp", file), ...)
}
}

Expand Down
4 changes: 2 additions & 2 deletions inst/NEWS.Rd
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
finely (Romain in \ghpr{868}).
\item The new \code{Rcpp_fast_eval} is used instead of
\code{Rcpp_eval} though this still requires setting
\code{RCPP_PROTECTED_EVAL} before including \code{Rcpp.h} (Qiang
\code{RCPP_USE_UNWIND_PROTECT} before including \code{Rcpp.h} (Qiang
Kou in \ghpr{867} closing \ghit{866}).
\item The \code{Rcpp::unwindProtect()} function extracts the
unwinding from the \code{Rcpp_fast_eval()} function and makes it
Expand Down Expand Up @@ -110,7 +110,7 @@
(Kevin in \ghpr{784}).
\item Use public R APIs for \code{new_env} (Kevin in \ghpr{785}).
\item Evaluation of R code is now safer when compiled against R
3.5 (you also need to explicitly define \code{RCPP_PROTECTED_EVAL}
3.5 (you also need to explicitly define \code{RCPP_USE_UNWIND_PROTECT}
before including \code{Rcpp.h}). Longjumps of all kinds (condition
catching, returns, restarts, debugger exit) are appropriately
detected and handled, e.g. the C++ stack unwinds correctly
Expand Down
2 changes: 1 addition & 1 deletion inst/include/Rcpp/DataFrame.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ namespace Rcpp{
obj.attr( "names") = names ;
Shield<SEXP> call( Rf_lang3(as_df_symb, obj, wrap( strings_as_factors ) ) ) ;
SET_TAG( CDDR(call), strings_as_factors_symb ) ;
Shield<SEXP> res( Rcpp_fast_eval( call ) ) ;
Shield<SEXP> res(Rcpp_fast_eval(call, R_GlobalEnv));
DataFrame_Impl out( res ) ;
return out ;

Expand Down
6 changes: 3 additions & 3 deletions inst/include/Rcpp/Environment.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ namespace Rcpp{
if( Rf_isEnvironment(x) ) return x ;
SEXP asEnvironmentSym = Rf_install("as.environment");
try {
Shield<SEXP> res( Rcpp_fast_eval( Rf_lang2( asEnvironmentSym, x ) ) );
Shield<SEXP> res(Rcpp_fast_eval(Rf_lang2(asEnvironmentSym, x), R_GlobalEnv));
return res ;
} catch( const eval_error& ex) {
const char* fmt = "Cannot convert object to an environment: "
Expand Down Expand Up @@ -374,7 +374,7 @@ namespace Rcpp{
try{
SEXP getNamespaceSym = Rf_install("getNamespace");
Shield<SEXP> package_str( Rf_mkString(package.c_str()) );
env = Rcpp_fast_eval( Rf_lang2(getNamespaceSym, package_str) ) ;
env = Rcpp_fast_eval(Rf_lang2(getNamespaceSym, package_str), R_GlobalEnv);
} catch( ... ){
throw no_such_namespace( package ) ;
}
Expand All @@ -393,7 +393,7 @@ namespace Rcpp{
*/
Environment_Impl new_child(bool hashed) const {
SEXP newEnvSym = Rf_install("new.env");
return Environment_Impl( Rcpp_fast_eval(Rf_lang3( newEnvSym, Rf_ScalarLogical(hashed), Storage::get__() )) );
return Environment_Impl(Rcpp_fast_eval(Rf_lang3(newEnvSym, Rf_ScalarLogical(hashed), Storage::get__()), R_GlobalEnv));
}


Expand Down
2 changes: 1 addition & 1 deletion inst/include/Rcpp/Function.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ namespace Rcpp{

SEXP operator()() const {
Shield<SEXP> call(Rf_lang1(Storage::get__()));
return Rcpp_fast_eval(call);
return Rcpp_fast_eval(call, R_GlobalEnv);
}

#include <Rcpp/generated/Function__operator.h>
Expand Down
9 changes: 2 additions & 7 deletions inst/include/Rcpp/api/meat/Rcpp_eval.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,10 @@
#include <Rcpp/Interrupt.h>
#include <Rversion.h>

#if (defined(RCPP_PROTECTED_EVAL) && defined(R_VERSION) && R_VERSION >= R_Version(3, 5, 0))
#define RCPP_USE_PROTECT_UNWIND
#include <Rcpp/api/meat/unwind.h>
#endif


namespace Rcpp { namespace internal {

#ifdef RCPP_USE_PROTECT_UNWIND
#ifdef RCPP_USING_UNWIND_PROTECT

struct EvalData {
SEXP expr;
Expand Down Expand Up @@ -61,7 +56,7 @@ inline SEXP Rcpp_eval_impl(SEXP expr, SEXP env) {

namespace Rcpp {

#ifdef RCPP_USE_PROTECT_UNWIND
#ifdef RCPP_USING_UNWIND_PROTECT

inline SEXP Rcpp_fast_eval(SEXP expr, SEXP env) {
internal::EvalData data(expr, env);
Expand Down
78 changes: 42 additions & 36 deletions inst/include/Rcpp/exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,51 +114,57 @@ namespace Rcpp {
throw Rcpp::exception(message.c_str());
} // #nocov end

namespace internal {
} // namespace Rcpp

inline SEXP longjumpSentinel(SEXP token) {
SEXP sentinel = PROTECT(Rf_allocVector(VECSXP, 1));
SET_VECTOR_ELT(sentinel, 0, token);

SEXP sentinelClass = PROTECT(Rf_mkString("Rcpp:longjumpSentinel"));
Rf_setAttrib(sentinel, R_ClassSymbol, sentinelClass) ;
namespace Rcpp { namespace internal {

UNPROTECT(2);
return sentinel;
}
inline SEXP longjumpSentinel(SEXP token) {
SEXP sentinel = PROTECT(Rf_allocVector(VECSXP, 1));
SET_VECTOR_ELT(sentinel, 0, token);

inline bool isLongjumpSentinel(SEXP x) {
return
Rf_inherits(x, "Rcpp:longjumpSentinel") &&
TYPEOF(x) == VECSXP &&
Rf_length(x) == 1;
}
SEXP sentinelClass = PROTECT(Rf_mkString("Rcpp:longjumpSentinel"));
Rf_setAttrib(sentinel, R_ClassSymbol, sentinelClass) ;

inline SEXP getLongjumpToken(SEXP sentinel) {
return VECTOR_ELT(sentinel, 0);
}
UNPROTECT(2);
return sentinel;
}

inline bool isLongjumpSentinel(SEXP x) {
return
Rf_inherits(x, "Rcpp:longjumpSentinel") &&
TYPEOF(x) == VECSXP &&
Rf_length(x) == 1;
}

inline SEXP getLongjumpToken(SEXP sentinel) {
return VECTOR_ELT(sentinel, 0);
}

struct LongjumpException {
SEXP token;
LongjumpException(SEXP token_) : token(token_) {
if (isLongjumpSentinel(token)) {
token = getLongjumpToken(token);
}
}
};

inline void resumeJump(SEXP token) {
if (isLongjumpSentinel(token)) {
token = getLongjumpToken(token);
}
::R_ReleaseObject(token);
inline void resumeJump(SEXP token) {
if (isLongjumpSentinel(token)) {
token = getLongjumpToken(token);
}
::R_ReleaseObject(token);
#if (defined(R_VERSION) && R_VERSION >= R_Version(3, 5, 0))
::R_ContinueUnwind(token);
::R_ContinueUnwind(token);
#endif
Rf_error("Internal error: Rcpp longjump failed to resume");
}
Rf_error("Internal error: Rcpp longjump failed to resume");
}

}} // namespace Rcpp::internal


} // namespace internal
namespace Rcpp {

struct LongjumpException {
SEXP token;
LongjumpException(SEXP token_) : token(token_) {
if (internal::isLongjumpSentinel(token)) {
token = internal::getLongjumpToken(token);
}
}
};

} // namespace Rcpp

Expand Down
Loading