Skip to content

Commit 9d912e2

Browse files
s-us-u
committed
add protection fixes
1 parent 92e3905 commit 9d912e2

2 files changed

Lines changed: 46 additions & 57 deletions

File tree

‎src/Rglue.c‎

Lines changed: 32 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,6 @@
77
#include <R_ext/Riconv.h>
88
#include <errno.h>
99
#include "rjstring.h"
10-
11-
/* R 4.0.1 broke EXTPTR_PTR ABI so re-map it to safety at
12-
the small expense of speed */
13-
#ifdef EXTPTR_PTR
14-
#undef EXTPTR_PTR
15-
#endif
16-
#define EXTPTR_PTR(X) R_ExternalPtrAddr(X)
17-
/* PROT/TAG are safe so far, but just to make sure ... */
18-
#ifdef EXTPTR_PROT
19-
#undef EXTPTR_PROT
20-
#endif
21-
#define EXTPTR_PROT(X) R_ExternalPtrProtected(X)
22-
#ifdef EXTPTR_TAG
23-
#undef EXTPTR_TAG
24-
#endif
25-
#define EXTPTR_TAG(X) R_ExternalPtrTag(X)
26-
2710
#include <stdarg.h>
2811

2912
/* max supported # of parameters to Java methods */
@@ -197,7 +180,7 @@ HIDE void deserializeSEXP(SEXP o) {
197180
/* Note: currently we don't remove the serialized content, because it was created explicitly using .jcache to allow repeated saving. Once this is handled by a hook, we shall remove it. However, to assure compatibility TAG is always NULL for now, so we do clear the cache if TAG is non-null for future use. */
198181
if (EXTPTR_TAG(o) != R_NilValue) {
199182
/* remove the serialized raw vector */
200-
SETCDR(o, R_NilValue); /* Note: this is abuse of the API since it uses the fact that PROT is stored in CDR */
183+
R_SetExternalPtrTag(o, R_NilValue);
201184
}
202185
}
203186
}
@@ -259,7 +242,9 @@ static int Rpar2jvalue(JNIEnv *env, SEXP par, jvalue *jpar, sig_buffer_t *sig, i
259242
SEXP p=par;
260243
SEXP e;
261244
int jvpos=0;
262-
int i=0;
245+
#ifdef RJ_DEBUG
246+
int i = 0;
247+
#endif
263248

264249
while (p && TYPEOF(p)==LISTSXP && (e=CAR(p))) {
265250
/* skip all named arguments */
@@ -432,7 +417,9 @@ static int Rpar2jvalue(JNIEnv *env, SEXP par, jvalue *jpar, sig_buffer_t *sig, i
432417
_dbg(rjprintf(" (ignoring)\n"));
433418
}
434419
}
420+
#ifdef RJ_DEBUG
435421
i++;
422+
#endif
436423
p=CDR(p);
437424
}
438425
fintmpo(tmpo);
@@ -453,7 +440,8 @@ static void Rfreejpars(JNIEnv *env, jobject *tmpo) {
453440
HIDE jvalue R1par2jvalue(JNIEnv *env, SEXP par, sig_buffer_t *sig, jobject *otr) {
454441
jobject tmpo[4] = {0, 0};
455442
jvalue v[4];
456-
int p = Rpar2jvalue(env, CONS(par, R_NilValue), v, sig, 2, tmpo);
443+
int p = Rpar2jvalue(env, PROTECT(CONS(par, R_NilValue)), v, sig, 2, tmpo);
444+
UNPROTECT(1);
457445
/* this should never happen, but just in case - we can only assume responsibility for one value ... */
458446
if (p != 1 || (tmpo[0] && tmpo[1])) {
459447
Rfreejpars(env, tmpo);
@@ -717,8 +705,11 @@ REPE SEXP RcallSyncMethod(SEXP par) {
717705

718706
e = RcallMethod(par);
719707

720-
if ((*env)->MonitorExit(env, o) != JNI_OK)
708+
if ((*env)->MonitorExit(env, o) != JNI_OK) {
709+
PROTECT(e);
721710
REprintf("Rglue.SERIOUS PROBLEM: MonitorExit failed, subsequent calls may cause a deadlock!\n");
711+
UNPROTECT(1);
712+
}
722713

723714
return e;
724715
}
@@ -832,14 +823,13 @@ static SEXP getObjectClassName(JNIEnv *env, jobject o) {
832823

833824
/** creates a new jobjRef object. If klass is NULL then the class is determined from the object (if also o=NULL then the class is set to java/lang/Object) */
834825
HIDE SEXP new_jobjRef(JNIEnv *env, jobject o, const char *klass) {
835-
SEXP oo = NEW_OBJECT(MAKE_CLASS("jobjRef"));
826+
SEXP oo = PROTECT(NEW_OBJECT(PROTECT(MAKE_CLASS("jobjRef"))));
836827
if (!inherits(oo, "jobjRef"))
837828
error("unable to create jobjRef object");
838-
PROTECT(oo);
839829
SET_SLOT(oo, install("jclass"),
840-
klass?mkString(klass):getObjectClassName(env, o));
841-
SET_SLOT(oo, install("jobj"), j2SEXP(env, o, 1));
842-
UNPROTECT(1);
830+
PROTECT(klass ? mkString(klass) : getObjectClassName(env, o)));
831+
SET_SLOT(oo, install("jobj"), PROTECT(j2SEXP(env, o, 1)));
832+
UNPROTECT(4);
843833
return oo;
844834
}
845835

@@ -851,13 +841,12 @@ HIDE SEXP new_jobjRef(JNIEnv *env, jobject o, const char *klass) {
851841
* @param cl Class instance
852842
*/
853843
HIDE SEXP new_jclassName(JNIEnv *env, jobject/*Class*/ cl ) {
854-
SEXP oo = NEW_OBJECT(MAKE_CLASS("jclassName"));
844+
SEXP oo = PROTECT(NEW_OBJECT(PROTECT(MAKE_CLASS("jclassName"))));
855845
if (!inherits(oo, "jclassName"))
856846
error("unable to create jclassName object");
857-
PROTECT(oo);
858-
SET_SLOT(oo, install("name"), getName(env, cl) );
859-
SET_SLOT(oo, install("jobj"), new_jobjRef( env, cl, "java/lang/Class" ) );
860-
UNPROTECT(1);
847+
SET_SLOT(oo, install("name"), PROTECT(getName(env, cl)) );
848+
SET_SLOT(oo, install("jobj"), PROTECT(new_jobjRef( env, cl, "java/lang/Class" )) );
849+
UNPROTECT(4);
861850
return oo;
862851
}
863852

@@ -875,24 +864,22 @@ HIDE SEXP getName( JNIEnv *env, jobject/*Class*/ cl){
875864
if (sl) (*env)->GetStringUTFRegion(env, r, 0, sl, cn);
876865
char *c=cn; while(*c) { if (*c=='.') *c='/'; c++; }
877866

878-
SEXP res = PROTECT( mkString(cn ) );
867+
SEXP res = mkString(cn);
879868
releaseObject(env, r);
880-
UNPROTECT(1); /* res */
881869
return res;
882870
}
883871

884872
static SEXP new_jarrayRef(JNIEnv *env, jobject a, const char *sig) {
885873
/* it is too tedious to try to do this in C, so we use 'new' R function instead */
886874
/* SEXP oo = eval(LCONS(install("new"),LCONS(mkString("jarrayRef"),R_NilValue)), R_GlobalEnv); */
887-
SEXP oo = NEW_OBJECT(MAKE_CLASS("jarrayRef"));
875+
SEXP oo = PROTECT(NEW_OBJECT(PROTECT(MAKE_CLASS("jarrayRef"))));
888876
/* .. and set the slots in C .. */
889877
if (! IS_JARRAYREF(oo) )
890878
error("unable to create an array");
891-
PROTECT(oo);
892-
SET_SLOT(oo, install("jobj"), j2SEXP(env, a, 1));
893-
SET_SLOT(oo, install("jclass"), mkString(sig));
894-
SET_SLOT(oo, install("jsig"), mkString(sig));
895-
UNPROTECT(1);
879+
SET_SLOT(oo, install("jobj"), PROTECT(j2SEXP(env, a, 1)));
880+
SET_SLOT(oo, install("jclass"), PROTECT(mkString(sig)));
881+
SET_SLOT(oo, install("jsig"), PROTECT(mkString(sig)));
882+
UNPROTECT(5);
896883
return oo;
897884
}
898885

@@ -908,17 +895,16 @@ static SEXP new_jarrayRef(JNIEnv *env, jobject a, const char *sig) {
908895
static SEXP new_jrectRef(JNIEnv *env, jobject a, const char *sig, SEXP dim ) {
909896
/* it is too tedious to try to do this in C, so we use 'new' R function instead */
910897
/* SEXP oo = eval(LCONS(install("new"),LCONS(mkString("jrectRef"),R_NilValue)), R_GlobalEnv); */
911-
SEXP oo = NEW_OBJECT(MAKE_CLASS("jrectRef"));
898+
SEXP oo = PROTECT(NEW_OBJECT(PROTECT(MAKE_CLASS("jrectRef"))));
912899
/* .. and set the slots in C .. */
913900
if (! IS_JRECTREF(oo) )
914901
error("unable to create an array");
915-
PROTECT(oo);
916-
SET_SLOT(oo, install("jobj"), j2SEXP(env, a, 1));
917-
SET_SLOT(oo, install("jclass"), mkString(sig));
918-
SET_SLOT(oo, install("jsig"), mkString(sig));
902+
SET_SLOT(oo, install("jobj"), PROTECT(j2SEXP(env, a, 1)));
903+
SET_SLOT(oo, install("jclass"), PROTECT(mkString(sig)));
904+
SET_SLOT(oo, install("jsig"), PROTECT(mkString(sig)));
919905
SET_SLOT(oo, install("dimension"), dim);
920906

921-
UNPROTECT(1); /* oo */
907+
UNPROTECT(5); /* oo + slots */
922908
return oo;
923909
}
924910
#endif
@@ -1092,7 +1078,7 @@ REPC SEXP javaObjectCache(SEXP o, SEXP what) {
10921078
error("invalid object");
10931079
if (TYPEOF(what) == RAWSXP || what == R_NilValue) {
10941080
/* set PROT to the serialization of NULL */
1095-
SETCDR(o, what);
1081+
R_SetExternalPtrProtected(o, what);
10961082
return what;
10971083
}
10981084
if (TYPEOF(what) == LGLSXP)

‎src/rJava.c‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ HIDE void throwR(SEXP msg, SEXP jobj, SEXP clazzes) {
4343

4444
setAttrib(cond, R_NamesSymbol, names);
4545
setAttrib(cond, R_ClassSymbol, clazzes);
46-
UNPROTECT(2); /* clazzes, names */
47-
eval(LCONS(install("stop"), CONS(cond, R_NilValue)), R_GlobalEnv);
48-
UNPROTECT(1); /* cond */
46+
UNPROTECT(1); /* names */
47+
eval(PROTECT(LCONS(install("stop"), PROTECT(CONS(cond, R_NilValue)))), R_GlobalEnv);
48+
UNPROTECT(3); /* cond + eval-pars */
4949
}
5050

5151
/* check for exceptions and throw them to R level */
@@ -63,15 +63,15 @@ HIDE void ckx(JNIEnv *env) {
6363
/* env is valid and an exception occurred */
6464
/* we create the jobj first, because the exception may in theory disappear after being cleared,
6565
yet this can be (also in theory) risky as it uses further JNI calls ... */
66-
xobj = j2SEXP(env, x, 0);
66+
xobj = PROTECT(j2SEXP(env, x, 0));
6767
if (!rj_RJavaTools_Class) {
6868
REprintf("ERROR: Java exception occurred during rJava bootstrap - see stderr for Java stack trace.\n");
6969
(*env)->ExceptionDescribe(env);
7070
}
7171
(*env)->ExceptionClear(env);
7272

7373
/* grab the list of class names (without package path) */
74-
SEXP clazzes = rj_RJavaTools_Class ? PROTECT( getSimpleClassNames_asSEXP( (jobject)x, (jboolean)1 ) ) : R_NilValue;
74+
SEXP clazzes = PROTECT( rj_RJavaTools_Class ? getSimpleClassNames_asSEXP( (jobject)x, (jboolean)1 ) : R_NilValue );
7575

7676
/* ok, now this is a critical part that we do manually to avoid recursion */
7777
{
@@ -97,7 +97,7 @@ HIDE void ckx(JNIEnv *env) {
9797
/* convert full class name to JNI notation */
9898
char *cn = strdup(c), *d = cn;
9999
while (*d) { if (*d == '.') *d = '/'; d++; }
100-
xclass = mkString(cn);
100+
xclass = PROTECT(mkString(cn));
101101
free(cn);
102102
(*env)->ReleaseStringUTFChars(env, cname, c);
103103
}
@@ -114,15 +114,20 @@ HIDE void ckx(JNIEnv *env) {
114114
(*env)->DeleteLocalRef(env, x);
115115

116116
/* construct the jobjRef */
117-
xr = PROTECT(NEW_OBJECT(MAKE_CLASS("jobjRef")));
117+
xr = PROTECT(NEW_OBJECT(PROTECT(MAKE_CLASS("jobjRef"))));
118+
if (!xclass) xclass = PROTECT(mkString("java/lang/Throwable"));
118119
if (inherits(xr, "jobjRef")) {
119-
SET_SLOT(xr, install("jclass"), xclass ? xclass : mkString("java/lang/Throwable"));
120+
SET_SLOT(xr, install("jclass"), xclass);
120121
SET_SLOT(xr, install("jobj"), xobj);
121122
}
122123

123124
/* and off to R .. (we're keeping xr and clazzes protected) */
124125
throwR(msg, xr, clazzes);
125-
/* throwR never returns so don't even bother ... */
126+
/* should not return, but just in case ... */
127+
128+
/* protect stack: xobj, clz, msg, xcl, xr + mk_class */
129+
/* (it may confuse the UP checker since the order does vary by path, but the count is the same) */
130+
UNPROTECT(6);
126131
}
127132

128133
/* clear any pending exceptions */
@@ -162,8 +167,6 @@ HIDE JNIEnv *getJNIEnv(void)
162167
}
163168
if (env && !eenv) eenv=env;
164169

165-
/* if (eenv!=env)
166-
fprintf(stderr, "Warning! eenv=%x, but env=%x - different environments encountered!\n", eenv, env); */
167170
return env;
168171
}
169172

0 commit comments

Comments
 (0)