Skip to content

Commit 3f922d6

Browse files
committed
Fix: SET_ATTRIB should do no validation of the attribute values
1 parent 271fd64 commit 3f922d6

File tree

3 files changed

+66
-3
lines changed

3 files changed

+66
-3
lines changed

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,3 +157,5 @@ lib.install.packages.fastr
157157
lib.install.packages.gnur
158158
/com.oracle.truffle.r.release/doc/
159159
.Rproj.user
160+
com.oracle.truffle.r.test.native/packages/*/*/src/*.so
161+
com.oracle.truffle.r.test.native/packages/*/*/src/*.o

com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/AttributesAccessNodes.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.oracle.truffle.api.dsl.Fallback;
3333
import com.oracle.truffle.api.dsl.Specialization;
3434
import com.oracle.truffle.api.library.CachedLibrary;
35+
import com.oracle.truffle.api.object.DynamicObject;
3536
import com.oracle.truffle.api.profiles.ConditionProfile;
3637
import com.oracle.truffle.r.ffi.impl.nodes.AttributesAccessNodesFactory.ATTRIBNodeGen;
3738
import com.oracle.truffle.r.ffi.impl.nodes.AttributesAccessNodesFactory.CopyMostAttribNodeGen;
@@ -43,7 +44,9 @@
4344
import com.oracle.truffle.r.nodes.attributes.GetAttributesNode;
4445
import com.oracle.truffle.r.nodes.attributes.RemoveAttributeNode;
4546
import com.oracle.truffle.r.nodes.attributes.SetAttributeNode;
47+
import com.oracle.truffle.r.nodes.attributes.SetPropertyNode;
4648
import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctions.GetRowNamesAttributeNode;
49+
import com.oracle.truffle.r.nodes.function.opt.ShareObjectNode;
4750
import com.oracle.truffle.r.nodes.function.opt.UpdateShareableChildValueNode;
4851
import com.oracle.truffle.r.nodes.unary.CastNode;
4952
import com.oracle.truffle.r.nodes.unary.InternStringNode;
@@ -269,7 +272,9 @@ public static CopyMostAttrib create() {
269272

270273
/**
271274
* Overrides the attributes pairlist of given object with a new pairlist. In FastR, we have to
272-
* convert the pairlist to our representation.
275+
* convert the pairlist to our representation. This doesn't do any validation in GNU-R and
276+
* simply sets the attributes pairlist to given value and some packages assume that they can,
277+
* e.g., fixup any inconsistencies in special attributes like dims afterwards.
273278
*/
274279
public abstract static class SetAttribNode extends FFIUpCallNode.Arg2 {
275280

@@ -279,9 +284,11 @@ public static SetAttribNode create() {
279284

280285
@Specialization
281286
protected Object doIt(RSharingAttributeStorage target, RPairList attributes,
282-
@Cached("create()") SetAttributeNode setAttribNode,
287+
@Cached SetPropertyNode setPropertyNode,
288+
@Cached ShareObjectNode shareObjectNode,
283289
@CachedLibrary(limit = "1") RPairListLibrary plLib) {
284290
clearAttrs(target);
291+
DynamicObject attrs = target.getAttributes();
285292
for (RPairList attr : attributes) {
286293
Object tag = plLib.getTag(attr);
287294
if (!(tag instanceof RSymbol)) {
@@ -291,7 +298,9 @@ protected Object doIt(RSharingAttributeStorage target, RPairList attributes,
291298
RError.warning(NO_CALLER, Message.NO_TAG_IN_SET_ATTRIB, Utils.getTypeName(tag));
292299
continue;
293300
}
294-
setAttribNode.execute(target, ((RSymbol) tag).getName(), plLib.car(attr));
301+
Object value = plLib.car(attr);
302+
shareObjectNode.execute(value);
303+
setPropertyNode.execute(attrs, ((RSymbol) tag).getName(), value);
295304
}
296305
return RNull.instance;
297306
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
# Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
2+
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
3+
#
4+
# This code is free software; you can redistribute it and/or modify it
5+
# under the terms of the GNU General Public License version 3 only, as
6+
# published by the Free Software Foundation.
7+
#
8+
# This code is distributed in the hope that it will be useful, but WITHOUT
9+
# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
10+
# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
11+
# version 3 for more details (a copy is included in the LICENSE file that
12+
# accompanied this code).
13+
#
14+
# You should have received a copy of the GNU General Public License version
15+
# 3 along with this work; if not, write to the Free Software Foundation,
16+
# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
17+
#
18+
# Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
19+
# or visit www.oracle.com if you need additional information or have any
20+
# questions.
21+
22+
# Contains unit-tests of the individual R API functions
23+
24+
assertEquals <- function(expected, actual) {
25+
width <- 80L
26+
name <- substr(deparse(sys.call(), width)[[1L]], 1, width)
27+
cat(name, paste(rep('.', width + 3L - nchar(name)), collapse=''))
28+
cat(if (identical(expected, actual)) 'pass' else 'fail', '\n')
29+
}
30+
31+
ignore <- function(...) {}
32+
33+
library(testrffi)
34+
35+
# ---------------------------------------------------------------------------------------
36+
# SET_ATTRIB
37+
38+
x <- c(1,3,10)
39+
assertEquals(NULL, api.SET_ATTRIB(x, pairlist(names=c('a','b','q'))))
40+
assertEquals(c('a','b','q'), names(x))
41+
42+
# there is no validation
43+
x <- c(1,3,10)
44+
assertEquals(NULL, api.SET_ATTRIB(x, as.pairlist(list(names=c('a','b')))))
45+
assertEquals(c('a','b'), names(x))
46+
# note: printing x on GNU-R causes segfault
47+
48+
# ---------------------------------------------------------------------------------------
49+
# Rf_mkCharLenCE, note: last arg is encoding and 0 ~ native encoding
50+
51+
assertEquals("hello world", api.Rf_mkCharLenCE("hello world", 11, 0))
52+
ignore("FastR bug", assertEquals("hello", api.Rf_mkCharLenCE("hello this will be cut away", 5, 0)))

0 commit comments

Comments
 (0)