Skip to content

Commit 8985530

Browse files
steve-ssteve-s
committed
[GR-22243] Properly convert C strings.
PullRequest: fastr/2403
2 parents a70ddce + fffdb6b commit 8985530

File tree

7 files changed

+149
-40
lines changed

7 files changed

+149
-40
lines changed

‎com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/common/JavaUpCallsRFFIImpl.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import com.oracle.truffle.api.source.SourceSection;
4444
import com.oracle.truffle.r.ffi.impl.javaGD.JavaGDContext;
4545
import com.oracle.truffle.r.ffi.impl.upcalls.UpCallsRFFI;
46-
import com.oracle.truffle.r.ffi.processor.RFFICstring;
4746
import com.oracle.truffle.r.nodes.RASTUtils;
4847
import com.oracle.truffle.r.nodes.function.ClassHierarchyNode;
4948
import com.oracle.truffle.r.runtime.RArguments;
@@ -2436,7 +2435,7 @@ public void Rf_PrintValue(Object value) {
24362435
}
24372436

24382437
@Override
2439-
public int R_nchar(@RFFICstring Object string, int type, int allowNA, int keepNA, @RFFICstring Object msgName) {
2438+
public int R_nchar(Object string, int type, int allowNA, int keepNA, String msgName) {
24402439
throw implementedAsNode();
24412440
}
24422441

‎com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/upcalls/StdUpCallsRFFI.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -201,17 +201,17 @@ public interface StdUpCallsRFFI {
201201
@RFFIUpCallNode(CoerceVectorNode.class)
202202
Object Rf_coerceVector(Object x, int mode);
203203

204-
Object Rf_mkCharLenCE(@RFFICstring(convert = false) Object bytes, int len, int encoding);
204+
Object Rf_mkCharLenCE(@RFFICpointer(isString = true) Object bytes, int len, int encoding);
205205

206206
Object Rf_cons(Object car, Object cdr);
207207

208208
void Rf_defineVar(Object symbolArg, Object value, Object envArg);
209209

210210
@RFFIUpCallNode(GetClassDefNode.class)
211-
Object R_getClassDef(@RFFIResultOwner @RFFICstring(convert = false) Object clazz);
211+
Object R_getClassDef(@RFFIResultOwner @RFFICpointer(isString = true) Object clazz);
212212

213213
@RFFIUpCallNode(DoMakeClassNode.class)
214-
Object R_do_MAKE_CLASS(@RFFICstring(convert = false) Object clazz);
214+
Object R_do_MAKE_CLASS(@RFFICpointer(isString = true) Object clazz);
215215

216216
@RFFIUpCallNode(value = MiscNodes.RDoNewObjectNode.class, needsCallTarget = true)
217217
Object R_do_new_object(Object classDef);
@@ -530,7 +530,7 @@ public interface StdUpCallsRFFI {
530530
Object R_CHAR(Object x);
531531

532532
@RFFIUpCallNode(NewCustomConnectionNode.class)
533-
Object R_new_custom_connection(@RFFICstring(convert = false) Object description, @RFFICstring(convert = false) Object mode, @RFFICstring(convert = false) Object className, Object readAddr);
533+
Object R_new_custom_connection(@RFFICpointer(isString = true) Object description, @RFFICpointer(isString = true) Object mode, @RFFICpointer(isString = true) Object className, Object readAddr);
534534

535535
int R_ReadConnection(int fd, long bufAddress, int size);
536536

@@ -545,7 +545,7 @@ public interface StdUpCallsRFFI {
545545
Object R_do_slot_assign(Object o, Object name, Object value);
546546

547547
@RFFIUpCallNode(Str2TypeNode.class)
548-
int Rf_str2type(@RFFICstring(convert = false) Object name);
548+
int Rf_str2type(@RFFICpointer(isString = true) Object name);
549549

550550
@RFFIUpCallNode(value = RandFunctionsNodes.RandFunction3_1Node.class, functionClass = Unif.DUnif.class)
551551
double Rf_dunif(double a, double b, double c, int d);
@@ -964,7 +964,7 @@ public interface StdUpCallsRFFI {
964964
void Rf_PrintValue(Object value);
965965

966966
@RFFIUpCallNode(RNCharNode.class)
967-
int R_nchar(Object string, int type, int allowNA, int keepNA, @RFFICstring Object msgName);
967+
int R_nchar(Object string, int type, int allowNA, int keepNA, @RFFICstring String msgName);
968968

969969
@RFFIUpCallNode(value = RForceAndCallNode.class, needsCallTarget = true)
970970
Object R_forceAndCall(Object e, Object f, int n, Object args);

‎com.oracle.truffle.r.ffi.processor/src/com/oracle/truffle/r/ffi/processor/FFIProcessor.java‎

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ private void generateCallClass(ExecutableElement m) throws IOException {
204204
StringBuilder unwrapNodes = new StringBuilder();
205205
StringBuilder unwrappedArgs = new StringBuilder();
206206
boolean needsUnwrapImport = false;
207+
boolean needsStringUnwrapImport = false;
207208
Set<String> unwrapArrayImports = new HashSet<>();
208209
int injectedArgsCount = 0;
209210

@@ -217,7 +218,6 @@ private void generateCallClass(ExecutableElement m) throws IOException {
217218
TypeMirror paramType = params.get(i).asType();
218219

219220
RFFICpointer[] pointerAnnotations = params.get(i).getAnnotationsByType(RFFICpointer.class);
220-
RFFICstring[] stringAnnotations = params.get(i).getAnnotationsByType(RFFICstring.class);
221221
RFFICarray[] arrayAnnotations = params.get(i).getAnnotationsByType(RFFICarray.class);
222222
RFFIInject[] injectAnnotations = params.get(i).getAnnotationsByType(RFFIInject.class);
223223
boolean needsArrayUnwrap = arrayAnnotations.length > 0;
@@ -229,13 +229,14 @@ private void generateCallClass(ExecutableElement m) throws IOException {
229229
!paramType.getKind().isPrimitive() &&
230230
!paramTypeName.equals("java.lang.String") &&
231231
pointerAnnotations.length == 0 &&
232-
!needsArrayUnwrap &&
233-
(stringAnnotations.length == 0 || stringAnnotations[0].convert());
232+
!needsArrayUnwrap;
233+
boolean needsStringUnwrap = paramTypeName.equals("java.lang.String");
234234
boolean needCast = !paramTypeName.equals("java.lang.Object");
235235
if (needCast) {
236236
arguments.append('(').append(paramTypeName).append(") ");
237237
}
238238
needsUnwrapImport |= needsUnwrap;
239+
needsStringUnwrapImport |= needsStringUnwrap;
239240
unwrappedArgs.append(" final Object ").append(paramName).append("Unwrapped = ");
240241
if (needsInject) {
241242
if (nodeAnnotation != null) {
@@ -256,7 +257,7 @@ private void generateCallClass(ExecutableElement m) throws IOException {
256257
processingEnv.getMessager().printMessage(Kind.NOTE, "" + arrAnnot.element());
257258
String arrayUnwrapSimpleName = arrAnnot.element().wrapperSimpleClassName;
258259
unwrapArrayImports.add(arrAnnot.element().wrapperClassPackage + "." + arrayUnwrapSimpleName);
259-
unwrapNodes.append(" @Cached() ").append(arrayUnwrapSimpleName).append(' ').append(paramName).append("Unwrap,\n");
260+
unwrapNodes.append(" @Cached ").append(arrayUnwrapSimpleName).append(' ').append(paramName).append("Unwrap,\n");
260261
unwrappedArgs.append(paramName).append("Unwrap").append(".execute(");
261262
if (!"".equals(arrAnnot.length())) {
262263
String lengthExpr = arrAnnot.length();
@@ -266,11 +267,14 @@ private void generateCallClass(ExecutableElement m) throws IOException {
266267
// TODO: report an error
267268
}
268269
unwrappedArgs.append(", ");
270+
} else if (needsStringUnwrap) {
271+
unwrapNodes.append(" @Cached FFIUnwrapString ").append(paramName).append("Unwrap,\n");
272+
unwrappedArgs.append(paramName).append("Unwrap").append(".execute(");
269273
}
270274
if (!needsInject) {
271275
unwrappedArgs.append("arguments[").append(i).append("]");
272276
arguments.append(paramName).append("Unwrapped");
273-
if (needsUnwrap || needsArrayUnwrap) {
277+
if (needsUnwrap || needsArrayUnwrap || needsStringUnwrap) {
274278
unwrappedArgs.append(')');
275279
}
276280
} else {
@@ -337,6 +341,9 @@ private void generateCallClass(ExecutableElement m) throws IOException {
337341
if (needsUnwrapImport) {
338342
w.append("import com.oracle.truffle.r.runtime.ffi.FFIUnwrapNode;\n");
339343
}
344+
if (needsStringUnwrapImport) {
345+
w.append("import com.oracle.truffle.r.runtime.ffi.FFIUnwrapString;\n");
346+
}
340347
if (!unwrapArrayImports.isEmpty()) {
341348
for (String unwrapArrayImport : unwrapArrayImports) {
342349
w.append("import ").append(unwrapArrayImport).append(";\n");
@@ -371,7 +378,7 @@ private void generateCallClass(ExecutableElement m) throws IOException {
371378
w.append(" }\n");
372379
w.append("\n");
373380
w.append(" @ExportMessage\n");
374-
w.append(" @SuppressWarnings(\"unused\")\n");
381+
w.append(" @SuppressWarnings({\"unused\", \"cast\"})\n");
375382
w.append(" Object execute(Object[] arguments,\n");
376383
if (unwrapNodes.length() > 0) {
377384
w.append(unwrapNodes);
@@ -546,7 +553,7 @@ private void generateCallClass(ExecutableElement m) throws IOException {
546553
}
547554
TypeMirror paramType = params.get(i).asType();
548555
String paramTypeName = getTypeName(paramType);
549-
boolean needCast = !paramTypeName.equals("java.lang.Object");
556+
boolean needCast = !paramTypeName.equals("java.lang.Object") && !paramTypeName.equals("java.lang.String");
550557
if (needCast) {
551558
args.append('(').append(paramTypeName).append(") ");
552559
}
@@ -628,25 +635,20 @@ private String getNFISignature(ExecutableElement m) {
628635
sb.append(", ");
629636
}
630637
realArgsCounter++;
631-
RFFICstring[] annotations = param.getAnnotationsByType(RFFICstring.class);
632-
String nfiParam = nfiParamName(param.asType(), annotations.length == 0 ? null : annotations[0], false, param);
638+
String nfiParam = nfiParamName(param.asType(), false, param);
633639
sb.append(nfiParam);
634640
}
635641
sb.append(')');
636642
sb.append(" : ");
637-
sb.append(nfiParamName(m.getReturnType(), null, true, m));
643+
sb.append(nfiParamName(m.getReturnType(), true, m));
638644
return sb.toString();
639645
}
640646

641-
private String nfiParamName(TypeMirror paramType, RFFICstring rffiCstring, boolean isReturn, Element m) {
647+
private String nfiParamName(TypeMirror paramType, boolean isReturn, Element m) {
642648
String paramTypeName = getTypeName(paramType);
643649
switch (paramTypeName) {
644650
case "java.lang.Object":
645-
if (rffiCstring == null) {
646-
return "pointer";
647-
} else {
648-
return rffiCstring.convert() ? "string" : "pointer";
649-
}
651+
return "pointer";
650652
case "boolean":
651653
return "uint8";
652654
case "int":
@@ -667,14 +669,7 @@ private String nfiParamName(TypeMirror paramType, RFFICstring rffiCstring, boole
667669
return "[uint8]";
668670
default:
669671
if ("java.lang.String".equals(paramTypeName)) {
670-
if (isReturn) {
671-
return "string";
672-
} else if (rffiCstring != null) {
673-
if (!rffiCstring.convert()) {
674-
processingEnv.getMessager().printMessage(Kind.ERROR, "Invalid parameter type (without conversion) " + paramTypeName, m);
675-
}
676-
return "string";
677-
}
672+
return "string";
678673
}
679674
if (isReturn) {
680675
processingEnv.getMessager().printMessage(Kind.ERROR, "Invalid return type " + paramTypeName, m);

‎com.oracle.truffle.r.ffi.processor/src/com/oracle/truffle/r/ffi/processor/RFFICpointer.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -34,4 +34,8 @@
3434
@Retention(RetentionPolicy.RUNTIME)
3535
@Target({ElementType.PARAMETER, ElementType.METHOD})
3636
public @interface RFFICpointer {
37+
/**
38+
* This meta-data has not practical use other than documentation for now.
39+
*/
40+
boolean isString() default false;
3741
}

‎com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/NativeDataAccess.java‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,13 +1060,7 @@ static String getData(CharSXPWrapper charSXPWrapper, String data) {
10601060
} else {
10611061
NativeMirror mirror = charSXPWrapper.getNativeMirror();
10621062
assert mirror.getDataAddress() != 0;
1063-
int length = 0;
1064-
while (length < mirror.length && NativeMemory.getByte(mirror.dataAddress, length) != 0) {
1065-
length++;
1066-
}
1067-
byte[] bytes = new byte[length];
1068-
NativeMemory.copyMemory(mirror.dataAddress, bytes, ElementType.BYTE, length);
1069-
return new String(bytes, StandardCharsets.UTF_8);
1063+
return NativeMemory.copyCString(mirror.dataAddress, mirror.length, StandardCharsets.UTF_8);
10701064
}
10711065
}
10721066

‎com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/FFIUnwrapString.java‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/*
2+
* Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 3 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 3 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 3 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
package com.oracle.truffle.r.runtime.ffi;
24+
25+
import java.nio.charset.StandardCharsets;
26+
27+
import com.oracle.truffle.api.CompilerDirectives;
28+
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
29+
import com.oracle.truffle.api.dsl.GenerateUncached;
30+
import com.oracle.truffle.api.dsl.Specialization;
31+
import com.oracle.truffle.api.interop.InteropLibrary;
32+
import com.oracle.truffle.api.interop.UnsupportedMessageException;
33+
import com.oracle.truffle.api.library.CachedLibrary;
34+
import com.oracle.truffle.r.runtime.RInternalError;
35+
import com.oracle.truffle.r.runtime.ffi.util.NativeMemory;
36+
import com.oracle.truffle.r.runtime.nodes.RBaseNode;
37+
38+
/**
39+
* Converts to {@code java.lang.String} an object coming from the native code which is supposed to
40+
* represent a string on the native side. Can unwrap string like objects coming from both NFI and
41+
* Sulong.
42+
*
43+
* TODO: in Sulong, we should convert the native objects proactively by calling polyglot_from_string
44+
* in order to get polyglot object (that supports asString message) and pass that as the argument to
45+
* the actual up-call. However, in any case, we want to use this unwrap node.
46+
*/
47+
@GenerateUncached
48+
public abstract class FFIUnwrapString extends RBaseNode {
49+
50+
public abstract String execute(Object obj);
51+
52+
@Specialization
53+
String doString(String value) {
54+
return value;
55+
}
56+
57+
// Note: this specialization should cover also VectorRFFIWrapper of CharSXP and NativeCharArray
58+
@Specialization(guards = {"!isString(value)", "interopLib.isString(value)"}, limit = "getInteropLibraryCacheSize()")
59+
String doInteropString(Object value,
60+
@CachedLibrary("value") InteropLibrary interopLib) {
61+
try {
62+
return interopLib.asString(value);
63+
} catch (UnsupportedMessageException e) {
64+
throw RInternalError.shouldNotReachHere(e);
65+
}
66+
}
67+
68+
@Specialization(guards = {"!isString(value)", "!interopLib.isString(value)", "!interopLib.isPointer(value)"}, limit = "getInteropLibraryCacheSize()")
69+
String doToNativeAndPointer(Object value,
70+
@CachedLibrary("value") InteropLibrary interopLib) {
71+
interopLib.toNative(value);
72+
if (!interopLib.isPointer(value)) {
73+
CompilerDirectives.transferToInterpreter();
74+
throw RInternalError.shouldNotReachHere(value.toString());
75+
}
76+
return doPointer(value, interopLib);
77+
}
78+
79+
@Specialization(guards = {"!isString(value)", "!interopLib.isString(value)", "interopLib.isPointer(value)"}, limit = "getInteropLibraryCacheSize()")
80+
String doPointer(Object value,
81+
@CachedLibrary("value") InteropLibrary interopLib) {
82+
try {
83+
long ptr = interopLib.asPointer(value);
84+
return copyCString(ptr);
85+
} catch (UnsupportedMessageException e) {
86+
throw RInternalError.shouldNotReachHere(e);
87+
}
88+
}
89+
90+
@TruffleBoundary
91+
private static String copyCString(long ptr) {
92+
return NativeMemory.copyCString(ptr, StandardCharsets.US_ASCII);
93+
}
94+
95+
static boolean isString(Object obj) {
96+
return obj instanceof String;
97+
}
98+
}

‎com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/util/NativeMemory.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.lang.ref.ReferenceQueue;
2828
import java.lang.ref.WeakReference;
2929
import java.lang.reflect.Field;
30+
import java.nio.charset.Charset;
3031
import java.util.Arrays;
3132
import java.util.Map.Entry;
3233
import java.util.Objects;
@@ -150,6 +151,24 @@ private static void copyMemory(long source, int elementBase, int elementSize, Ob
150151
UNSAFE.copyMemory(null, source, destination, elementBase, (long) elementSize * (long) elementsCount);
151152
}
152153

154+
public static String copyCString(long address, Charset encoding) {
155+
return copyCString(address, Integer.MAX_VALUE, encoding);
156+
}
157+
158+
public static String copyCString(NativeMemoryWrapper address, long maxLength, Charset encoding) {
159+
return copyCString(address.getAddress(), maxLength, encoding);
160+
}
161+
162+
public static String copyCString(long address, long maxLength, Charset encoding) {
163+
int length = 0;
164+
while (length < maxLength && NativeMemory.getByte(address, length) != 0) {
165+
length++;
166+
}
167+
byte[] bytes = new byte[length];
168+
NativeMemory.copyMemory(address, bytes, ElementType.BYTE, length);
169+
return new String(bytes, encoding);
170+
}
171+
153172
public static void putByte(NativeMemoryWrapper address, long offset, byte value) {
154173
putByte(address.getAddress(), offset, value);
155174
}

0 commit comments

Comments
 (0)