Skip to content

Commit

Permalink
Restore arity-checking with an opt-out flag
Browse files Browse the repository at this point in the history
Removing automatic arity-checking for Java-based Ruby methods
broke some users' test suites that expected ArgumentError to be
thrown; without a manual arity-check, the target methods now tried
to access the incoming vararg array blindly and raised Java index
errors. This was reported as jruby#7851.

This fixes the issue by restoring arity-checking by default and
providing an opt-out flag that we and users can use to indicate
that the Java code will check arity manually.
  • Loading branch information
headius committed Jul 17, 2023
1 parent c0de12d commit cd3059d
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 0 deletions.
6 changes: 6 additions & 0 deletions core/src/main/java/org/jruby/anno/JRubyMethod.java
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@
*/
boolean notImplemented() default false;

/**
* Whether automatic arity-checking should be added to the Ruby binding for this
* method. Users can opt-out and do their own arity-checking in the method body.
*/
boolean checkArity() default true;

/**
* A list of classes that implement an abstract JRubyMethod, for backtrace purposes.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,75 @@ public JavaMethod constructJavaMethod(RubyModule implementationClass, JavaMethod
return ic;
}

/**
* Emit code to check the arity of a call to a Java-based method.
*
* @param jrubyMethod The annotation of the called method
* @param method The code generator for the handle being created
*/
private static void checkArity(JRubyMethod jrubyMethod, SkinnyMethodAdapter method, int specificArity) {
switch (specificArity) {
case 0:
case 1:
case 2:
case 3:
// for zero, one, two, three arities, JavaMethod.JavaMethod*.call(...IRubyObject[] args...) will check
return;
default:
final Label arityError = new Label();
final Label noArityError = new Label();
boolean checkArity = false;
if (jrubyMethod.rest()) {
if (jrubyMethod.required() > 0) {
// just confirm minimum args provided
method.aload(ARGS_INDEX);
method.arraylength();
method.ldc(jrubyMethod.required());
method.if_icmplt(arityError);
checkArity = true;
}
} else if (jrubyMethod.optional() > 0) {
if (jrubyMethod.required() > 0) {
// confirm minimum args provided
method.aload(ARGS_INDEX);
method.arraylength();
method.ldc(jrubyMethod.required());
method.if_icmplt(arityError);
}

// confirm maximum not greater than optional
method.aload(ARGS_INDEX);
method.arraylength();
method.ldc(jrubyMethod.required() + jrubyMethod.optional());
method.if_icmpgt(arityError);
checkArity = true;
} else {
// just confirm args length == required
method.aload(ARGS_INDEX);
method.arraylength();
method.ldc(jrubyMethod.required());
method.if_icmpne(arityError);
checkArity = true;
}

if (checkArity) {
method.go_to(noArityError);

// Raise an error if arity does not match requirements
method.label(arityError);
method.aload(THREADCONTEXT_INDEX);
method.invokevirtual(p(ThreadContext.class), "getRuntime", sig(Ruby.class));
method.aload(ARGS_INDEX);
method.ldc(jrubyMethod.required());
method.ldc(jrubyMethod.required() + jrubyMethod.optional());
method.invokestatic(p(Arity.class), "checkArgumentCount", sig(int.class, Ruby.class, IRubyObject[].class, int.class, int.class));
method.pop();

method.label(noArityError);
}
}
}

private static ClassWriter createJavaMethodCtor(String namePath, String sup, String parameterDesc) {
ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES);
String sourceFile = namePath.substring(namePath.lastIndexOf('/') + 1) + ".gen";
Expand Down Expand Up @@ -662,6 +731,10 @@ private void createAnnotatedMethodInvocation(JavaMethodDescriptor desc, SkinnyMe
method.putfield("org/jruby/runtime/ThreadContext", "callInfo", "I");
}

if (desc.anno.checkArity()) {
checkArity(desc.anno, method, specificArity);
}

CallConfiguration callConfig = CallConfiguration.getCallConfigByAnno(desc.anno);
if (!callConfig.isNoop()) {
invokeCallConfigPre(method, superClass, specificArity, block, callConfig);
Expand Down

0 comments on commit cd3059d

Please sign in to comment.