eclipse-aspectj / aspectj

Other
303 stars 86 forks source link

TABLESWITCH throw ArrayIndexOutOfBoundsException when the high value is Integer.MAX_VALUE #190

Closed wurensen closed 1 year ago

wurensen commented 2 years ago

the source code will cause ArrayIndexOutOfBoundsException if the high value of tableswitch is Integer.MAX_VALUE:

    /**
     * Read needed data (e.g. index) from file.
     */
    public TABLESWITCH(ByteSequence bytes) throws IOException {
        super(Constants.TABLESWITCH, bytes);

        int low = bytes.readInt();
        int high = bytes.readInt();

        matchLength = high - low + 1;
        fixedLength = (short) (13 + matchLength * 4);
        length = (short) (fixedLength + padding);

        match = new int[matchLength];
        indices = new int[matchLength];
        targets = new InstructionHandle[matchLength];

// if high=Integer.MAX_VALUE, it cause java.lang.ArrayIndexOutOfBoundsException
        for (int i = low; i <= high; i++) {
            match[i - low] = i;
        }

        for (int i = 0; i < matchLength; i++) {
            indices[i] = bytes.readInt();
        }
    }

exception like this:

java.lang.RuntimeException: Crashed whilst crashing with this exception: org.aspectj.apache.bcel.generic.ClassGenException: java.lang.ArrayIndexOutOfBoundsException: Index 3 out of bounds for length 3
        at org.aspectj.weaver.bcel.BcelWeaver.weave(BcelWeaver.java:1728)
        at org.aspectj.weaver.bcel.BcelWeaver.weaveWithoutDump(BcelWeaver.java:1651)
        at org.aspectj.weaver.bcel.BcelWeaver.weaveAndNotify(BcelWeaver.java:1418)
        at org.aspectj.weaver.bcel.BcelWeaver.weave(BcelWeaver.java:1192)
        at org.aspectj.ajdt.internal.compiler.AjPipeliningCompilerAdapter.weaveQueuedEntries(AjPipeliningCompilerAdapter.java:510)
        at org.aspectj.ajdt.internal.compiler.AjPipeliningCompilerAdapter.afterCompiling(AjPipeliningCompilerAdapter.java:374)
        at org.aspectj.ajdt.internal.compiler.CompilerAdapter.ajc$afterReturning$org_aspectj_ajdt_internal_compiler_CompilerAdapter$2$f9cc9ca0(CompilerAdapter.java:69)
        at org.aspectj.org.eclipse.jdt.internal.compiler.Compiler.compile(Compiler.java:428)
        at org.aspectj.ajdt.internal.core.builder.AjBuildManager.performCompilation(AjBuildManager.java:1096)
        at org.aspectj.ajdt.internal.core.builder.AjBuildManager.performBuild(AjBuildManager.java:275)
        at org.aspectj.ajdt.internal.core.builder.AjBuildManager.batchBuild(AjBuildManager.java:188)
        at org.aspectj.ajdt.ajc.AjdtCommand.doCommand(AjdtCommand.java:103)
        at org.aspectj.ajdt.ajc.AjdtCommand.runCommand(AjdtCommand.java:47)
        at org.aspectj.tools.ajc.Main.run(Main.java:374)
        at com.hujiang.gradle.plugin.android.aspectjx.internal.procedure.AJXTask.call(AJXTask.kt:137)
        at com.hujiang.gradle.plugin.android.aspectjx.internal.procedure.DoAspectProcedure.runAJXTask(DoAspectProcedure.kt:252)
        at com.hujiang.gradle.plugin.android.aspectjx.internal.procedure.DoAspectProcedure.processDirectoryInput(DoAspectProcedure.kt:158)
        at com.hujiang.gradle.plugin.android.aspectjx.internal.procedure.DoAspectProcedure.access$processDirectoryInput(DoAspectProcedure.kt:12)
        at com.hujiang.gradle.plugin.android.aspectjx.internal.procedure.DoAspectProcedure$process$1$1$1.invoke(DoAspectProcedure.kt:32)
        at com.hujiang.gradle.plugin.android.aspectjx.internal.procedure.DoAspectProcedure$process$1$1$1.invoke(DoAspectProcedure.kt:31)
        at com.hujiang.gradle.plugin.android.aspectjx.internal.concurrent.BatchTaskScheduler$schedule$1.call(BatchTaskScheduler.kt:36)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
        at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: org.aspectj.apache.bcel.generic.ClassGenException: java.lang.ArrayIndexOutOfBoundsException: Index 3 out of bounds for length 3
        at org.aspectj.apache.bcel.generic.Instruction.readInstruction(Instruction.java:233)
        at org.aspectj.apache.bcel.generic.InstructionList.<init>(InstructionList.java:190)
        at org.aspectj.apache.bcel.generic.MethodGen.<init>(MethodGen.java:211)
        at org.aspectj.weaver.bcel.LazyMethodGen.initialize(LazyMethodGen.java:370)
        at org.aspectj.weaver.bcel.LazyMethodGen.getReturnType(LazyMethodGen.java:917)
        at org.aspectj.weaver.bcel.LazyMethodGen.toShortString(LazyMethodGen.java:560)
        at org.aspectj.weaver.bcel.LazyMethodGen.print(LazyMethodGen.java:601)
        at org.aspectj.weaver.bcel.LazyClassGen.printOne(LazyClassGen.java:918)
        at org.aspectj.weaver.bcel.LazyClassGen.print(LazyClassGen.java:877)
        at org.aspectj.weaver.bcel.LazyClassGen.toLongString(LazyClassGen.java:865)
        at org.aspectj.weaver.bcel.BcelWeaver.weave(BcelWeaver.java:1726)
        ... 27 more

You can reproduce the bug if the weaved source code contain this:

    private static final int CASE_1 = Integer.MAX_VALUE;
    private static final int CASE_2 = Integer.MAX_VALUE - 1;
    private static final int CASE_3 = Integer.MAX_VALUE - 2;

    public String switchTest(int i) {
        String content = "";
        switch (i) {
            case CASE_1:
                content = "CASE_1";
                break;
            case CASE_2:
                content = "CASE_2";
                break;
            case CASE_3:
                content = "CASE_3";
                break;
            default:
                break;
        }
        return content;
    }

I fixed it like this:

    /**
     * Read needed data (e.g. index) from file.
     *
     * @param bytes bytes
     * @throws IOException IOException
     */
    public TABLESWITCH(ByteSequence bytes) throws IOException {
        super(Constants.TABLESWITCH, bytes);

        int low = bytes.readInt();
        int high = bytes.readInt();

        matchLength = high - low + 1;
        fixedLength = (short) (13 + matchLength * 4);
        length = (short) (fixedLength + padding);

        match = new int[matchLength];
        indices = new int[matchLength];
        targets = new InstructionHandle[matchLength];

        // bugfix: case使用的值可能是Integer.MAX_VALUE,i++后值溢出变为负数导致数组越界,所以需要重新修改算法
//        for (int i = low; i <= high; i++) {
//            match[i - low] = i;
//        }
        for (int i = 0; i < matchLength; i++) {
            match[i] = low + i;
        }

        for (int i = 0; i < matchLength; i++) {
            indices[i] = bytes.readInt();
        }
    }
kriegaex commented 2 years ago

@wurensen, I cannot reproduce the problem. Please provide more context information about JDK version and flavour, AspectJ version and OS platform used. Ideally, extend the example to be a full MCVE including Maven build or exact compiler options. If I cannot reproduce it, I cannot fix it either.

kriegaex commented 1 year ago

@wurensen, because you did not react to my inquiry and failed to help me reproduce the problem, I am assuming that you either cannot reproduce it anymore or simply do not care about the problem enough to get it solved. Therefore, I am closing the issue. We can reopen it, if you make it reproducible for me.

kriegaex commented 1 year ago

@wurensen: OK, I just had an idea and did your job, preparing a reproducer for this problem. Actually, the problem does not occur if we just have a Java file with that kind of code. Additionally, we need an aspect processing that class. Only then will the BCEL TABLESWITCH be used at all, not during normal Java compilation. We actually need to weave something. I.e., the reproducer is:

public class SwitchCaseWith_Integer_MAX_VALUE {
  public static void main(String[] args) {
    System.out.println(switchTest(Integer.MAX_VALUE));
  }

  static String switchTest(int i) {
    switch (i) {
      case Integer.MAX_VALUE:
        return "CASE_1";
      default:
        return "";
    }
  }
}
aspect MyAspect {
  before() : execution(* switchTest(*)) {
    System.out.println(thisJoinPoint);
  }
}

I can also confirm that your suggested fix works. Thanks for that.