From 2d0c1c2dbe44458ebb199c47ce1047f266db5349 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 17 Jul 2012 16:31:30 -0700 Subject: [PATCH] Avoid sign extension in packed-switch. This code (at least in the ARM version) is trying to assign to r0 and r1 from C by returning a 64-bit result. The mistaken use of signed integers for pointers can lead to sign extension if the JIT code cache is at a sufficiently high address. Bug: 6799823 Bug: 6703991 Change-Id: Ic3f587f453f0f3f520551383ef1ed29efa1ad551 --- vm/compiler/codegen/arm/CalloutHelper.h | 7 ------- vm/compiler/codegen/arm/CodegenDriver.cpp | 14 +++++++------- vm/compiler/codegen/mips/CalloutHelper.h | 7 ------- vm/compiler/codegen/mips/CodegenDriver.cpp | 18 +++++++++--------- vm/oo/Resolve.cpp | 15 +++++++++++---- 5 files changed, 27 insertions(+), 34 deletions(-) diff --git a/vm/compiler/codegen/arm/CalloutHelper.h b/vm/compiler/codegen/arm/CalloutHelper.h index cc4c0ae98..079c5f646 100644 --- a/vm/compiler/codegen/arm/CalloutHelper.h +++ b/vm/compiler/codegen/arm/CalloutHelper.h @@ -87,13 +87,6 @@ const Method *dvmJitToPatchPredictedChain(const Method *method, const ClassObject *clazz); /* - * Switch dispatch offset calculation for OP_PACKED_SWITCH & OP_SPARSE_SWITCH - * Used in CodegenDriver.c - * static s8 findPackedSwitchIndex(const u2* switchData, int testVal, int pc); - * static s8 findSparseSwitchIndex(const u2* switchData, int testVal, int pc); - */ - -/* * Resolve interface callsites - OP_INVOKE_INTERFACE & OP_INVOKE_INTERFACE_RANGE * * Originally declared in mterp/common/FindInterface.h and only comment it here diff --git a/vm/compiler/codegen/arm/CodegenDriver.cpp b/vm/compiler/codegen/arm/CodegenDriver.cpp index d96aa6561..78809dc74 100644 --- a/vm/compiler/codegen/arm/CodegenDriver.cpp +++ b/vm/compiler/codegen/arm/CodegenDriver.cpp @@ -2789,16 +2789,16 @@ static bool handleFmt23x(CompilationUnit *cUnit, MIR *mir) * chaining cell for case default [8 bytes] * noChain exit */ -static s8 findPackedSwitchIndex(const u2* switchData, int testVal, int pc) +static u8 findPackedSwitchIndex(const u2* switchData, int testVal, uintptr_t pc) { int size; int firstKey; const int *entries; int index; int jumpIndex; - int caseDPCOffset = 0; + uintptr_t caseDPCOffset = 0; /* In Thumb mode pc is 4 ahead of the "mov r2, pc" instruction */ - int chainingPC = (pc + 4) & ~3; + uintptr_t chainingPC = (pc + 4) & ~3; /* * Packed switch data format: @@ -2837,16 +2837,16 @@ static s8 findPackedSwitchIndex(const u2* switchData, int testVal, int pc) } chainingPC += jumpIndex * CHAIN_CELL_NORMAL_SIZE; - return (((s8) caseDPCOffset) << 32) | (u8) chainingPC; + return (((u8) caseDPCOffset) << 32) | (u8) chainingPC; } /* See comments for findPackedSwitchIndex */ -static s8 findSparseSwitchIndex(const u2* switchData, int testVal, int pc) +static u8 findSparseSwitchIndex(const u2* switchData, int testVal, uintptr_t pc) { int size; const int *keys; const int *entries; - int chainingPC = (pc + 4) & ~3; + uintptr_t chainingPC = (pc + 4) & ~3; int i; /* @@ -2888,7 +2888,7 @@ static s8 findSparseSwitchIndex(const u2* switchData, int testVal, int pc) int jumpIndex = (i < MAX_CHAINED_SWITCH_CASES) ? i : MAX_CHAINED_SWITCH_CASES + 1; chainingPC += jumpIndex * CHAIN_CELL_NORMAL_SIZE; - return (((s8) entries[i]) << 32) | (u8) chainingPC; + return (((u8) entries[i]) << 32) | (u8) chainingPC; } else if (k > testVal) { break; } diff --git a/vm/compiler/codegen/mips/CalloutHelper.h b/vm/compiler/codegen/mips/CalloutHelper.h index 6e2343d98..85343619f 100644 --- a/vm/compiler/codegen/mips/CalloutHelper.h +++ b/vm/compiler/codegen/mips/CalloutHelper.h @@ -84,13 +84,6 @@ const Method *dvmJitToPatchPredictedChain(const Method *method, const ClassObject *clazz); /* - * Switch dispatch offset calculation for OP_PACKED_SWITCH & OP_SPARSE_SWITCH - * Used in CodegenDriver.c - * static s8 findPackedSwitchIndex(const u2* switchData, int testVal, int pc); - * static s8 findSparseSwitchIndex(const u2* switchData, int testVal, int pc); - */ - -/* * Resolve interface callsites - OP_INVOKE_INTERFACE & OP_INVOKE_INTERFACE_RANGE * * Originally declared in mterp/common/FindInterface.h and only comment it here diff --git a/vm/compiler/codegen/mips/CodegenDriver.cpp b/vm/compiler/codegen/mips/CodegenDriver.cpp index 3dd95ae29..c7757fe17 100644 --- a/vm/compiler/codegen/mips/CodegenDriver.cpp +++ b/vm/compiler/codegen/mips/CodegenDriver.cpp @@ -428,7 +428,7 @@ static void genIGet(CompilationUnit *cUnit, MIR *mir, OpSize size, size, rlObj.sRegLow); HEAP_ACCESS_SHADOW(false); if (isVolatile) { - dvmCompilerGenMemBarrier(cUnit, 0); + dvmCompilerGenMemBarrier(cUnit, 0); } storeValue(cUnit, rlDest, rlResult); @@ -450,7 +450,7 @@ static void genIPut(CompilationUnit *cUnit, MIR *mir, OpSize size, NULL);/* null object? */ if (isVolatile) { - dvmCompilerGenMemBarrier(cUnit, 0); + dvmCompilerGenMemBarrier(cUnit, 0); } HEAP_ACCESS_SHADOW(true); storeBaseDisp(cUnit, rlObj.lowReg, fieldOffset, rlSrc.lowReg, size); @@ -1553,7 +1553,7 @@ static bool handleFmt10x(CompilationUnit *cUnit, MIR *mir) } switch (dalvikOpcode) { case OP_RETURN_VOID_BARRIER: - dvmCompilerGenMemBarrier(cUnit, 0); + dvmCompilerGenMemBarrier(cUnit, 0); // Intentional fallthrough case OP_RETURN_VOID: genReturnCommon(cUnit,mir); @@ -2935,14 +2935,14 @@ static bool handleFmt23x(CompilationUnit *cUnit, MIR *mir) * chaining cell for case default [16 bytes] * noChain exit */ -static s8 findPackedSwitchIndex(const u2* switchData, int testVal) +static u8 findPackedSwitchIndex(const u2* switchData, int testVal) { int size; int firstKey; const int *entries; int index; int jumpIndex; - int caseDPCOffset = 0; + uintptr_t caseDPCOffset = 0; /* * Packed switch data format: @@ -2984,11 +2984,11 @@ static s8 findPackedSwitchIndex(const u2* switchData, int testVal) jumpIndex = index; } - return (((s8) caseDPCOffset) << 32) | (u8) (jumpIndex * CHAIN_CELL_NORMAL_SIZE + 20); + return (((u8) caseDPCOffset) << 32) | (u8) (jumpIndex * CHAIN_CELL_NORMAL_SIZE + 20); } /* See comments for findPackedSwitchIndex */ -static s8 findSparseSwitchIndex(const u2* switchData, int testVal) +static u8 findSparseSwitchIndex(const u2* switchData, int testVal) { int size; const int *keys; @@ -3035,7 +3035,7 @@ static s8 findSparseSwitchIndex(const u2* switchData, int testVal) /* MAX_CHAINED_SWITCH_CASES + 1 is the start of the overflow case */ int jumpIndex = (i < MAX_CHAINED_SWITCH_CASES) ? i : MAX_CHAINED_SWITCH_CASES + 1; - return (((s8) entries[i]) << 32) | (u8) (jumpIndex * CHAIN_CELL_NORMAL_SIZE + 20); + return (((u8) entries[i]) << 32) | (u8) (jumpIndex * CHAIN_CELL_NORMAL_SIZE + 20); #else int k = (unsigned int)keys[i] >> 16 | keys[i] << 16; if (k == testVal) { @@ -3043,7 +3043,7 @@ static s8 findSparseSwitchIndex(const u2* switchData, int testVal) int jumpIndex = (i < MAX_CHAINED_SWITCH_CASES) ? i : MAX_CHAINED_SWITCH_CASES + 1; int temp = (unsigned int)entries[i] >> 16 | entries[i] << 16; - return (((s8) temp) << 32) | (u8) (jumpIndex * CHAIN_CELL_NORMAL_SIZE + 20); + return (((u8) temp) << 32) | (u8) (jumpIndex * CHAIN_CELL_NORMAL_SIZE + 20); #endif } else if (k > testVal) { break; diff --git a/vm/oo/Resolve.cpp b/vm/oo/Resolve.cpp index a4890a5fc..ab3de5bda 100644 --- a/vm/oo/Resolve.cpp +++ b/vm/oo/Resolve.cpp @@ -219,7 +219,11 @@ Method* dvmResolveMethod(const ClassObject* referrer, u4 methodIdx, } if (resMethod == NULL) { - dvmThrowNoSuchMethodError(name); + std::string msg; + msg += resClass->descriptor; + msg += "."; + msg += name; + dvmThrowNoSuchMethodError(msg.c_str()); return NULL; } @@ -333,11 +337,14 @@ Method* dvmResolveInterfaceMethod(const ClassObject* referrer, u4 methodIdx) DexProto proto; dexProtoSetFromMethodId(&proto, pDvmDex->pDexFile, pMethodId); - LOGVV("+++ looking for '%s' '%s' in resClass='%s'", - methodName, methodSig, resClass->descriptor); + LOGVV("+++ looking for '%s' in resClass='%s'", methodName, resClass->descriptor); resMethod = dvmFindInterfaceMethodHier(resClass, methodName, &proto); if (resMethod == NULL) { - dvmThrowNoSuchMethodError(methodName); + std::string msg; + msg += resClass->descriptor; + msg += "."; + msg += methodName; + dvmThrowNoSuchMethodError(msg.c_str()); return NULL; } -- 2.11.0