Skip to content

Commit db08da5

Browse files
authored
validate module objects before converting them to c++ classes (#1524)
* validate module objects before converting them to c++ classes * use regex patterns instead of fixed = TRUE, unsupported by tinytest < 1.3.1
1 parent 5308aa7 commit db08da5

5 files changed

Lines changed: 153 additions & 4 deletions

File tree

‎ChangeLog‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,16 @@
2525
element, which it is not once coverage tools have instrumented the body
2626
* R/00_classes.R (.cppfunction_set_external_args): New helper
2727
* inst/tinytest/test_module.R: Test it with an instrumented body
28+
* inst/include/Rcpp/api/meat/as.h (as_module_object_internal): Check
29+
that the argument is a live module object, raising an error for
30+
deserialized, uninitialized and non-module objects instead of handing
31+
out a NULL or garbage address; a new overload taking the expected C++
32+
type also requires the object's class to be that type or one that
33+
derives() from it (#1523)
34+
* inst/include/Rcpp/as.h: Use the new overload when converting to
35+
module classes
36+
* inst/tinytest/cpp/Module.cpp: Add a class deriving from another
37+
* inst/tinytest/test_module.R: Test the new checks
2838

2939
2026-10-23 Dirk Eddelbuettel <edd@debian.org>
3040

‎inst/include/Rcpp/api/meat/as.h‎

Lines changed: 92 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,100 @@
2121
namespace Rcpp{
2222
namespace internal{
2323

24+
// the address of the C++ object held by the module object whose
25+
// environment is env (see cpp_object_initializer() in R/Module.R)
26+
inline void* module_object_address(const Environment& env){
27+
SEXP xp = env.get(".pointer") ;
28+
if( xp == R_NilValue )
29+
throw not_compatible( "Expecting a C++ module object" ) ;
30+
31+
// the dummy object of a class without a default constructor
32+
if( TYPEOF(xp) != EXTPTRSXP )
33+
throw not_initialized() ;
34+
35+
// e.g. a deserialized object
36+
void* ptr = R_ExternalPtrAddr(xp) ;
37+
if( ptr == NULL )
38+
throw ::Rcpp::exception( "external pointer is not valid" ) ;
39+
40+
return ptr ;
41+
}
42+
43+
#ifndef RCPP_NO_MODULES
44+
// whether the module class cl is the C++ class typeinfo_name, or derives
45+
// from it through class_::derives(), which records each parent by its R
46+
// class name, "Rcpp_" followed by its name in the module
47+
inline bool module_class_is( class_Base* cl, Module* module, const char* typeinfo_name ){
48+
if( cl->has_typeinfo_name(typeinfo_name) )
49+
return true ;
50+
51+
std::vector<class_Base*> todo(1, cl) ;
52+
std::set<class_Base*> seen ;
53+
while( module && !todo.empty() ){
54+
class_Base* current = todo.back() ;
55+
todo.pop_back() ;
56+
if( !seen.insert(current).second )
57+
continue ;
58+
59+
if( current->has_typeinfo_name(typeinfo_name) )
60+
return true ;
61+
62+
for( size_t i = 0; i < current->parents.size(); i++ ){
63+
const std::string& parent = current->parents[i] ;
64+
if( parent.compare(0, 5, "Rcpp_") != 0 )
65+
continue ;
66+
std::string name = parent.substr(5) ;
67+
if( module->has_class(name) )
68+
todo.push_back( module->get_class_pointer(name) ) ;
69+
}
70+
}
71+
return false ;
72+
}
73+
#endif
74+
75+
// packages call this one directly, so it stays; it cannot check the class
2476
inline void* as_module_object_internal(SEXP obj){
77+
if( !Rf_isS4(obj) )
78+
throw not_compatible( "Expecting a C++ module object" ) ;
2579
Environment env(obj) ;
26-
SEXP xp = env.get(".pointer") ;
27-
return R_ExternalPtrAddr(xp );
80+
return module_object_address(env) ;
81+
}
82+
83+
inline void* as_module_object_internal(SEXP obj, const char* typeinfo_name){
84+
if( !Rf_isS4(obj) ){
85+
throw not_compatible(
86+
"Expecting an object of C++ class '%s'",
87+
demangle(typeinfo_name)
88+
) ;
89+
}
90+
Environment env(obj) ;
91+
void* ptr = module_object_address(env) ;
92+
93+
#ifndef RCPP_NO_MODULES
94+
SEXP cppclass = env.get(".cppclass") ;
95+
if( TYPEOF(cppclass) != EXTPTRSXP || R_ExternalPtrAddr(cppclass) == NULL ){
96+
throw not_compatible(
97+
"Expecting an object of C++ class '%s'",
98+
demangle(typeinfo_name)
99+
) ;
100+
}
101+
class_Base* cl = reinterpret_cast<class_Base*>( R_ExternalPtrAddr(cppclass) ) ;
102+
103+
SEXP module_xp = env.get(".module") ;
104+
Module* module = NULL ;
105+
if( TYPEOF(module_xp) == EXTPTRSXP )
106+
module = reinterpret_cast<Module*>( R_ExternalPtrAddr(module_xp) ) ;
107+
108+
if( !module_class_is(cl, module, typeinfo_name) ){
109+
throw not_compatible(
110+
"Expecting an object of C++ class '%s', got an object of class '%s'",
111+
demangle(typeinfo_name),
112+
cl->name
113+
) ;
114+
}
115+
#endif
116+
117+
return ptr ;
28118
}
29119

30120
}

‎inst/include/Rcpp/as.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,15 +89,17 @@ namespace Rcpp {
8989
}
9090

9191
void* as_module_object_internal(SEXP obj);
92+
void* as_module_object_internal(SEXP obj, const char* typeinfo_name);
9293

9394
template <typename T> object<T> as_module_object(SEXP x) {
94-
return (T*) as_module_object_internal(x);
95+
return (T*) as_module_object_internal(x, typeid(T).name());
9596
}
9697

9798
/** handling object<T> */
9899
template <typename T> T as(SEXP x, ::Rcpp::traits::r_type_module_object_const_pointer_tag) {
99100
typedef typename Rcpp::traits::remove_const<T>::type T_NON_CONST;
100-
return const_cast<T>((T_NON_CONST)as_module_object_internal(x));
101+
typedef typename Rcpp::traits::un_pointer<T>::type CLASS;
102+
return const_cast<T>((T_NON_CONST)as_module_object_internal(x, typeid(CLASS).name()));
101103
}
102104

103105
template <typename T> T as(SEXP x, ::Rcpp::traits::r_type_module_object_pointer_tag) {

‎inst/tinytest/cpp/Module.cpp‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,24 @@ double Test_get_x_pointer(ModuleTest* x) {
155155
return x->value;
156156
}
157157

158+
class ModuleBase {
159+
public:
160+
ModuleBase() : value(1.0) {}
161+
virtual ~ModuleBase() {}
162+
double value;
163+
};
164+
165+
class ModuleDerived : public ModuleBase {
166+
public:
167+
ModuleDerived() { value = 2.0; }
168+
};
169+
170+
RCPP_EXPOSED_CLASS_NODECL(ModuleBase)
171+
172+
double ModuleBase_value(const ModuleBase& x) {
173+
return x.value;
174+
}
175+
158176
class ModuleGadget {
159177
public:
160178
ModuleGadget() {}
@@ -254,6 +272,17 @@ RCPP_MODULE(demoModule) {
254272
.method("get" , &ModuleRandomizer::get)
255273
;
256274

275+
class_<ModuleBase>("ModuleBase")
276+
.constructor()
277+
;
278+
279+
class_<ModuleDerived>("ModuleDerived")
280+
.derives<ModuleBase>("ModuleBase")
281+
.constructor()
282+
;
283+
284+
function("ModuleBase_value", &ModuleBase_value);
285+
257286
class_<ModuleGadget>("ModuleGadget")
258287
.constructor()
259288
.method("value", static_cast<void (ModuleGadget::*)(int)>(&ModuleGadget::value))

‎inst/tinytest/test_module.R‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,24 @@ expect_equal( attr_Test_get_x_const_pointer(test), 3.0 )
9090
expect_equal( attr_Test_get_x_ref(test), 3.0 )
9191
expect_equal( attr_Test_get_x_pointer(test), 3.0 )
9292

93+
## module object arguments are validated before their address is handed to
94+
## C++, instead of reinterpreting or dereferencing whatever is there (#1523);
95+
## patterns are regular expressions, as tinytest < 1.3.1 has no 'fixed'
96+
expect_error( Test_get_x_const_ref( new( ModuleWorld ) ),
97+
"Expecting an object of C\\+\\+ class 'ModuleTest', got an object of class 'ModuleWorld'" )
98+
expect_error( attr_Test_get_x_ref( new( ModuleWorld ) ),
99+
"got an object of class 'ModuleWorld'" )
100+
expect_error( Test_get_x_pointer( unserialize( serialize( test, NULL ) ) ),
101+
"external pointer is not valid" )
102+
uninitialized <- new( ModuleTest )
103+
expect_error( Test_get_x_ref( uninitialized ), "not initialized" )
104+
expect_error( Test_get_x_const_pointer( 1 ),
105+
"Expecting an object of C\\+\\+ class 'ModuleTest'" )
106+
107+
## objects of a class that derives() from the expected one are accepted
108+
expect_equal( ModuleBase_value( new( ModuleBase ) ), 1.0 )
109+
expect_equal( ModuleBase_value( new( ModuleDerived ) ), 2.0 )
110+
93111
# test.Module.property <- function(){
94112
w <- new( ModuleNum )
95113
expect_equal( w$x, 0.0 )

0 commit comments

Comments
 (0)