GeezerVisonThe World Wide Web, restored to 800 pixels of glory. |
| Home|About|Gaming cabinet|Archives|How it works|Starter sites |
GeezerVison is showing github.com — sugar expressions give wrong results under aliasing, length mismatch, and captured temporaries · Issue #1512 · RcppCore/Rcpp Closed DescriptionWhile looking at sugar performance (#1510), I found several ways the sugar expression-template machinery gives silently wrong results or reads out of bounds. All of these reproduce on current Reproducer#include <Rcpp.h> using namespace Rcpp; // [[Rcpp::export]] NumericVector alias_rev(NumericVector x) { x = rev(x); return x; } // [[Rcpp::export]] NumericVector alias_rev_copy(NumericVector x) { NumericVector y = x; y = rev(x); return y; } // [[Rcpp::export]] NumericVector alias_mixed(NumericVector x) { x = x + rev(x); return x; } // [[Rcpp::export]] void rebind_same_len(NumericVector x) { x = x * 2.0; } // [[Rcpp::export]] void rebind_diff_len(NumericVector x) { x = head(x, 2) * 2.0; } // [[Rcpp::export]] NumericVector plus(NumericVector x, NumericVector y) { return x + y; } // [[Rcpp::export]] NumericVector dangling_auto(NumericVector x) { auto e = x + x * 2.0; NumericVector big(1000000, 42.0); return e; } alias_rev(c(1, 2, 3, 4, 5)) #> [1] 5 4 3 4 5 # expected 5 4 3 2 1 alias_rev_copy(c(1, 2, 3, 4, 5)) #> [1] 5 4 3 4 5 # expected 5 4 3 2 1 alias_mixed(c(1, 2, 3, 4, 5)) #> [1] 6 6 6 10 11 # expected 6 6 6 6 6 v <- c(1, 2, 3); w <- v rebind_same_len(v) v; w #> [1] 2 4 6 # both v and w modified #> [1] 2 4 6 v <- c(1, 2, 3) rebind_diff_len(v) v #> [1] 1 2 3 # unmodified plus(c(1, 2, 3, 4, 5), c(10, 20)) #> [1] 11 22 3 4 5 # reads past the end of y; only a warning plus(c(1, 2), c(10, 20, 30, 40)) #> [1] 11 22 # silently truncated dangling_auto(c(1, 2, 3)) #> [1] 1 2 3 # expected 3 6 9 1. Assigning an expression into one of its own operands gives wrong resultsWhen a sugar expression has the same length as the target, 2. Assignment semantics depend on the length of the expressionBecause of the same code path, Always evaluating into a freshly allocated vector would fix both 1 and 2. That does change behavior, though, for any code that (knowingly or not) relies on the in-place write. 3. Mismatched operand lengths read out of bounds rather than recyclingBinary sugar operators take their size from the left operand (e.g. Checking operand lengths once, when the expression is constructed, would fix this (either recycling to match R, or signalling an error). It would also make the per-element bounds check redundant for sugar operands, which is the remaining performance gap noted in #1510 / #1511. 4. Sub-expressions are held by reference, so they can dangleEvery sugar node stores its operands as A complete fix would store nested sugar expressions by value and only plain vectors by reference, similar to Eigen's Activity
MetadataMetadataAssigneesLabelsNo labels TypeProjectsMilestoneRelationshipsDevelopmentIssue actions |
| You are visitor 01402520 — GeezerVison does not store the pages you look up. Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required. |