Skip to content

tab completion on module objects drops methods or reads past the method map when [[ is overloaded #1529

Open

Description

@kevinushey

Tab completion on a module object (.DollarNames() → complete() → class_<T>::complete()) drops methods, and in some cases reads past the end of the class's method map, when the class has an overloaded [ or [[ method.

class_<T>::complete() lists every method except the special [-prefixed ones, followed by the properties. It sizes its result as vec_methods.size() - specials, where vec_methods has one entry per method name:

Rcpp::CharacterVector complete(){
size_t n = vec_methods.size() - specials ;
size_t ntotal = n + properties.size() ;
Rcpp::CharacterVector out(ntotal) ;
typename map_vec_signed_method::iterator it = vec_methods.begin( ) ;
std::string buffer ;
size_t i=0 ;
for( ; i<n; ++it){
buffer = it->first ;
if( buffer[0] == '[' ) continue ;
// if( (it->second)->nargs() == 0){
// buffer += "() " ;
// } else {
// buffer += "( " ;
// }
buffer += "( " ;
out[i] = buffer ;
i++ ;
}

but specials is incremented once per overload in AddMethod():

(it->second)->push_back( new signed_method_class(m, valid, docstring ) ) ;
if( *name_ == '[' ) ptr->specials++ ;

(it->second)->push_back( new signed_method_class(m, valid, docstring ) ) ;
if( *name_ == '[' ) ptr->specials++ ;

So with [[ exposed twice, specials is 2 but only one name is special:

  • If the class has other methods, the count comes out one short and the last ordinary method in the map is missing from the completions.
  • If the class has no other methods, vec_methods.size() - specials underflows (size_t). The loop for( ; i<n; ++it) then runs off the end of the map and dereferences end() (undefined behavior). In the example below that happens to be caught by the bounds check when the garbage name is written into the zero-length result, but it is a read of invalid memory.

Reproducible example

repro.cpp:

#include <Rcpp.h>

using namespace Rcpp;

class Grid {
public:
    Grid() : scale(1.0) {}
    double at(int i) { return scale * i; }
    double at2(int i, int j) { return scale * i * j; }
    double total() { return scale; }
    double scale;
};

class Table {
public:
    Table() : scale(1.0) {}
    double at(int i) { return scale * i; }
    double at2(int i, int j) { return scale * i * j; }
    double scale;
};

RCPP_MODULE(grid_module) {
    // overloaded [[ plus an ordinary method and a field
    class_<Grid>("Grid")
        .constructor()
        .method("[[", &Grid::at)
        .method("[[", &Grid::at2)
        .method("total", &Grid::total)
        .field("scale", &Grid::scale);

    // overloaded [[ and a field, no ordinary methods
    class_<Table>("Table")
        .constructor()
        .method("[[", &Table::at)
        .method("[[", &Table::at2)
        .field("scale", &Table::scale);
}

repro.R:

library(Rcpp)
sourceCpp("repro.cpp")

# what tab completion on `grid$` / `table$` calls
grid <- new(Grid)
print(.DollarNames(grid, ""))   # expected: "total( " "scale"

table <- new(Table)
print(tryCatch(.DollarNames(table, ""), error = function(e) conditionMessage(e)))   # expected: "scale"

Output with R 4.6.1 (macOS 27, Rcpp 1.1.2.6 / current master):

[1] "scale"
[1] "attempt to set index 0/0 in SET_STRING_ELT"
Warning message:
In complete(x) : subscript out of bounds (index 0 >= vector size 0)

total is missing for Grid, and completion on Table errors after walking past the end of the method map.

I'll follow up with a PR that counts each special method name once (when the name is first added) and adds tests for both cases.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions