From 7d7edb8be2257fab126267ae5c5d71c4de4de711 Mon Sep 17 00:00:00 2001 From: bsimmers Date: Thu, 28 Mar 2013 09:16:05 -0700 Subject: [PATCH] Implement all of VerifyParamType in hhir TranslatorX64 interps cases that it expects to fail but I didn't like that, so the hhir version never punts or interps. The fast path code reuses the stuff Jordan added for InstanceOfD and should be the same as tx64's, modulo some branch fusing that doesn't appear to matter in perflab. --- hphp/doc/ir.specification | 31 ++++- hphp/runtime/vm/translator/hopt/codegen.cpp | 54 ++++++--- .../vm/translator/hopt/hhbctranslator.cpp | 109 ++++++++++++------ .../vm/translator/hopt/hhbctranslator.h | 3 +- hphp/runtime/vm/translator/hopt/ir.h | 3 + .../vm/translator/hopt/irtranslator.cpp | 30 +---- .../vm/translator/hopt/nativecalls.cpp | 5 + .../runtime/vm/translator/hopt/simplifier.cpp | 2 +- .../vm/translator/hopt/tracebuilder.cpp | 15 +-- .../runtime/vm/translator/hopt/tracebuilder.h | 57 ++++++++- .../vm/translator/translator-runtime.cpp | 25 ++++ .../vm/translator/translator-runtime.h | 4 + hphp/runtime/vm/translator/translator-x64.cpp | 33 +----- hphp/runtime/vm/translator/translator-x64.h | 4 + hphp/runtime/vm/type_constraint.cpp | 12 +- hphp/runtime/vm/type_constraint.h | 3 +- hphp/test/vm/verify-param-type.php | 15 +++ hphp/test/vm/verify-param-type.php.exp | 38 ++++++ 18 files changed, 302 insertions(+), 141 deletions(-) create mode 100644 hphp/test/vm/verify-param-type.php create mode 100644 hphp/test/vm/verify-param-type.php.exp diff --git a/hphp/doc/ir.specification b/hphp/doc/ir.specification index c0de878ba..7b47700e9 100644 --- a/hphp/doc/ir.specification +++ b/hphp/doc/ir.specification @@ -647,13 +647,13 @@ D:T = LdConst D:ConstT = DefConst -D:Cls = LdCls S0:Str S1:Cls +D:Cls = LdCls S0:Str S1:ConstCls - Loads the class named S0 in the context of the class S1. Invokes - autoload and may raise an error if the class is not defined. The - explicit context parameter allows the compiler to optimize this - instruction by simplifying it to LdClsCached, which uses the target - cache. + Loads the class named S0 in the context of the class S1. Invokes autoload and + may raise an error if the class is not defined. The explicit context + parameter allows the compiler to simplify this instruction to a DefConst in + some cases. If S0 is constant, this instruction may be simplified to a + LdClsCached. D:Cls = LdClsCached S0:ConstStr @@ -1029,6 +1029,25 @@ Nop 14. Runtime helpers +VerifyParamCls S0:Cls S1:Cls S2:ConstInt + + Verify parameter type for classes or traits. If S0 does not extend + (if S1 is a class) or implement (if S1 is an interface) S1, this + instruction will raise a recoverable fatal error describing the type + mismatch. + +VerifyParamCallable S0:Cell S1:ConstInt + + If S0 is not callable, as defined by the php function is_callable, + this instruction will raise a recoverable fatal error describing the + type mismatch. + +VerifyParamFail S0:ConstInt + + Assumes that parameter number S0 in the current function has failed + its typehint and raises a recoverable fatal error describing the + type mismatch. + RaiseUninitLoc Raise a notice for an uninitialized local variable. diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp index da01b4b7d..673288aaa 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.cpp +++ b/hphp/runtime/vm/translator/hopt/codegen.cpp @@ -316,6 +316,8 @@ CALL_OPCODE(DbgAssertPtr) CALL_OPCODE(LdSwitchDblIndex); CALL_OPCODE(LdSwitchStrIndex); CALL_OPCODE(LdSwitchObjIndex); +CALL_OPCODE(VerifyParamCallable); +CALL_OPCODE(VerifyParamFail); CALL_OPCODE(RaiseUninitLoc) CALL_OPCODE(WarnNonObjProp) CALL_OPCODE(ThrowNonObjProp) @@ -4768,6 +4770,24 @@ void traceCallback(ActRec* fp, Cell* sp, int64_t pcOff, void* rip) { checkFrame(fp, sp, /*checkLocals*/true); } +void CodeGenerator::cgVerifyParamCls(IRInstruction* inst) { + SSATmp* objClass = inst->getSrc(0); + assert(!objClass->isConst()); + auto objClassReg = objClass->getReg(); + SSATmp* constraint = inst->getSrc(1); + + if (constraint->isConst()) { + m_as. cmpq(constraint->getValClass(), objClassReg); + } else { + m_as. cmpq(constraint->getReg(), objClassReg); + } + + // The native call for this instruction is the slow path that does + // proper subtype checking. The comparison above is just to + // short-circuit the overhead when the Classes are an exact match. + ifThen(m_as, CC_NE, [&]{ cgCallNative(inst); }); +} + void CodeGenerator::emitTraceCall(CodeGenerator::Asm& as, int64_t pcOff, Transl::TranslatorX64* tx64) { // call to a trace function @@ -4840,26 +4860,28 @@ void cgTrace(Trace* trace, Asm& amain, Asm& astubs, Transl::TranslatorX64* tx64, TCA astubsStart = astubs.code.frontier; patchJumps(*as, state, block); - // If the block ends with a Jmp_ to the next block we're translating into - // the same assembler, it doesn't need to actually emit a jmp. - state.noTerminalJmp_ = false; - IRInstruction* last = block->back(); - if (last->getOpcode() == Jmp_) { - for (auto next = it; next != end; ++next) { - if (chooseAs(*next) == as) { - state.noTerminalJmp_ = last->getTaken() == *next; - break; - } + // Grab the next block that will go into this assembler + Block* nextThisAs = nullptr; + for (auto next = it; next != end; ++next) { + if (chooseAs(*next) == as) { + nextThisAs = *next; + break; } } + + // If the block ends with a Jmp_ to the next block for this + // assembler, it doesn't need to actually emit a jmp. + IRInstruction* last = block->back(); + state.noTerminalJmp_ = + last->getOpcode() == Jmp_ && last->getTaken() == nextThisAs; + CodeGenerator cg(trace, *as, astubs, tx64, state); cg.cgBlock(block, bcMap); - if (Block* next = block->getNext()) { - // if there's a fallthrough block and it's not the next thing going - // into this assembler, then emit a jump to it. - if (it == end || next != *it || as != chooseAs(next)) { - CodeGenerator::emitFwdJmp(*as, next, state); - } + Block* next = block->getNext(); + if (next && next != nextThisAs) { + // if there's a fallthrough block and it's not the next thing + // going into this assembler, then emit a jump to it. + CodeGenerator::emitFwdJmp(*as, next, state); } if (state.asmInfo) { state.asmInfo->asmRanges[block] = TcaRange(asmStart, as->code.frontier); diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp index f23f90394..820a8be5c 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp @@ -1858,48 +1858,86 @@ Trace* HhbcTranslator::guardRefs(int64_t entryArDelta, return exitTrace; } -void HhbcTranslator::emitVerifyParamType(int32_t paramId, - const StringData* constraintClsName) { - /* It's currently better to interpOne all of these, since the slow path punts - * for the entire tracelet */ - emitInterpOneOrPunt(Type::None); - return; - - assert(!constraintClsName || constraintClsName->isStatic()); +void HhbcTranslator::emitVerifyParamType(int32_t paramId) { const Func* func = getCurFunc(); - const Class* constraint = (constraintClsName ? - Unit::lookupUniqueClass(constraintClsName) : - nullptr); const TypeConstraint& tc = func->params()[paramId].typeConstraint(); - const StringData* tcTypeName = tc.typeName(); + SSATmp* locVal = m_tb->genLdLoc(paramId); + Type locType = locVal->getType().unbox(); + assert(locType.isKnownDataType()); - // VerifyParamType does not remove the parameter from the stack - TRACE(3, "%u: VerifyParamType %s\n", m_bcOff, tcTypeName->data()); - Trace* exitTrace2 = getExitSlowTrace(); - // XXX Should verify param type unbox? - SSATmp* param = m_tb->genLdAssertedLoc(paramId, Type::Gen); - if (param->getType() != Type::Obj) { - PUNT(VerifyParamType_nonobj); + if (tc.nullable() && locType.isNull()) { + return; + } + if (tc.isCallable()) { + spillStack(); + locVal = m_tb->gen(Unbox, getExitTrace(), locVal); + m_tb->gen(VerifyParamCallable, locVal, cns(paramId)); + return; + } + // For non-object guards, or object guards with non-object runtime + // types, we rely on what we know from the tracelet guards and never + // have to do runtime checks. + if ((!tc.isObject() && !tc.check(locType.toDataType())) || + (tc.isObject() && !locType.subtypeOf(Type::Obj))) { + spillStack(); + m_tb->gen(VerifyParamFail, cns(paramId)); + return; + } + if (!tc.isObject()) { + return; } - SSATmp* objClass = m_tb->gen(LdObjClass, param); - if (tc.isObject()) { - if (!(param->getType() == Type::Obj || - (param->getType().isNull() && tc.nullable()))) { - emitInterpOne(Type::None); - return; - } + const StringData* clsName; + const Class* knownConstraint = nullptr; + if (!tc.isSelf() && !tc.isParent()) { + clsName = tc.typeName(); + knownConstraint = Unit::lookupClass(clsName); } else { - if (!tc.check(frame_local(curFrame() /* FIXME */, paramId), getCurFunc())) { - emitInterpOne(Type::None); + if (tc.isSelf()) { + tc.selfToClass(getCurFunc(), &knownConstraint); + } else if (tc.isParent()) { + tc.parentToClass(getCurFunc(), &knownConstraint); + } + if (knownConstraint) { + clsName = knownConstraint->preClass()->name(); + } else { + // The hint was self or parent and there's no corresponding + // class for the current func. This typehint will always fail. + spillStack(); + m_tb->gen(VerifyParamFail, cns(paramId)); return; } } + assert(clsName); + // We can only burn in the Class* if it's unique or in the + // inheritance hierarchy of our context. It's ok if the class isn't + // defined yet - all paths below are tolerant of a null constraint. + if (!classIsUniqueOrCtxParent(knownConstraint)) knownConstraint = nullptr; - m_tb->genVerifyParamType(objClass, - m_tb->genDefConst(tcTypeName), - constraint, - exitTrace2); + Class::initInstanceBits(); + bool haveBit = Class::haveInstanceBit(clsName); + SSATmp* constraint = knownConstraint ? cns(knownConstraint) + : m_tb->gen(LdCachedClass, cns(clsName)); + locVal = m_tb->gen(Unbox, getExitTrace(), locVal); + SSATmp* objClass = m_tb->gen(LdObjClass, locVal); + if (haveBit || classIsUniqueNormalClass(knownConstraint)) { + SSATmp* isInstance = haveBit ? + m_tb->gen(InstanceOfBitmask, objClass, cns(clsName)) + : m_tb->gen(ExtendsClass, objClass, constraint); + m_tb->ifThen(getCurFunc(), + [&](Block* taken) { + m_tb->gen(JmpZero, taken, isInstance); + }, + [&] { // taken: the param type does not match + m_tb->hint(Block::Unlikely); + spillStack(); + m_tb->gen(VerifyParamFail, cns(paramId)); + } + ); + } else { + spillStack(); + m_tb->gen(VerifyParamCls, objClass, constraint, cns(paramId)); + } } void HhbcTranslator::emitInstanceOfD(int classNameStrId) { @@ -1929,11 +1967,8 @@ void HhbcTranslator::emitInstanceOfD(int classNameStrId) { const bool haveBit = Class::haveInstanceBit(className); Class* const maybeCls = Unit::lookupUniqueClass(className); - const bool isNormalClass = maybeCls && - !(maybeCls->attrs() & - (AttrTrait | AttrInterface)); - const bool isUnique = RuntimeOption::RepoAuthoritative && - maybeCls && (maybeCls->attrs() & AttrUnique); + const bool isNormalClass = classIsUniqueNormalClass(maybeCls); + const bool isUnique = classIsUnique(maybeCls); /* * If the class is unique or a parent of the current context, we diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.h b/hphp/runtime/vm/translator/hopt/hhbctranslator.h index 2c5ebddec..fa63b681b 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.h +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.h @@ -271,8 +271,7 @@ struct HhbcTranslator { void emitIsBoolC(); void emitIsDoubleL(int id); void emitIsDoubleC(); - void emitVerifyParamType(int32_t paramId, - const StringData* constraintClsName); + void emitVerifyParamType(int32_t paramId); void emitInstanceOfD(int classNameStrId); void emitNop() {} void emitCastBool(); diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index a991ebb46..f14855678 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -332,6 +332,9 @@ O(DefLabel, DMulti, SUnk, E) \ O(Marker, ND, NA, E) \ O(DefFP, D(StkPtr), NA, E) \ O(DefSP, D(StkPtr), S(StkPtr) C(Int), E) \ +O(VerifyParamCls, ND, S(Cls) S(Cls) C(Int),E|N|Mem|Refs|Er) \ +O(VerifyParamCallable, ND, S(Cell) C(Int), E|N|Mem|Refs|Er) \ +O(VerifyParamFail, ND, C(Int), E|N|Mem|Refs|Er) \ O(RaiseUninitLoc, ND, S(Str), E|N|Mem|Refs|Er) \ O(WarnNonObjProp, ND, NA, E|N|Refs|Er|Mem) \ O(ThrowNonObjProp, ND, NA, T|E|N|Refs|Er|Mem) \ diff --git a/hphp/runtime/vm/translator/hopt/irtranslator.cpp b/hphp/runtime/vm/translator/hopt/irtranslator.cpp index 129a1c7e7..d5663fb43 100644 --- a/hphp/runtime/vm/translator/hopt/irtranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/irtranslator.cpp @@ -1150,35 +1150,9 @@ TranslatorX64::irTranslateStaticLocInit(const Tracelet& t, // check class hierarchy and fail if no match void TranslatorX64::irTranslateVerifyParamType(const Tracelet& t, - const NormalizedInstruction& i) { + const NormalizedInstruction& i) { int param = i.imm[0].u_IVA; - const TypeConstraint& tc = curFunc()->params()[param].typeConstraint(); - - // not quite a nop. The guards should have verified that the m_type field - // is compatible, but for objects we need to go one step further and - // ensure that we're dealing with the right class. - // NULL inputs only get traced when constraint is nullable. - assert(i.inputs.size() == 1); - if (!i.inputs[0]->isObject()) { - HHIR_UNIMPLEMENTED_WHEN(i.m_txFlags == Interp, VerifyParamType); - return; // nop. - } - - bool isSelf = tc.isSelf(); - bool isParent = tc.isParent(); - const Class *constraint = nullptr; - - UNUSED TargetCache::CacheHandle ch = 0; - if (isSelf) { - tc.selfToClass(curFunc(), &constraint); - } else if (isParent) { - tc.parentToClass(curFunc(), &constraint); - } else { - const StringData* clsName = tc.typeName(); - ch = TargetCache::allocKnownClass(clsName); - } - - HHIR_EMIT(VerifyParamType, param, (constraint ? constraint->name() : nullptr)); + HHIR_EMIT(VerifyParamType, param); } void diff --git a/hphp/runtime/vm/translator/hopt/nativecalls.cpp b/hphp/runtime/vm/translator/hopt/nativecalls.cpp index 99aa7833a..da4373b57 100644 --- a/hphp/runtime/vm/translator/hopt/nativecalls.cpp +++ b/hphp/runtime/vm/translator/hopt/nativecalls.cpp @@ -76,6 +76,11 @@ static CallMap s_callMap({ {PrintStr, (TCA)print_string, DNone, SNone, {{SSA, 0}}}, {PrintInt, (TCA)print_int, DNone, SNone, {{SSA, 0}}}, {PrintBool, (TCA)print_boolean, DNone, SNone, {{SSA, 0}}}, + {VerifyParamCls, (TCA)VerifyParamTypeSlow, DNone, SSync, + {{SSA, 0}, {SSA, 1}, {SSA, 2}}}, + {VerifyParamCallable, (TCA)VerifyParamTypeCallable, DNone, SSync, + {{TV, 0}, {SSA, 1}}}, + {VerifyParamFail, (TCA)VerifyParamTypeFail, DNone, SSync, {{SSA, 0}}}, {RaiseUninitLoc, (TCA)raiseUndefVariable, DNone, SSync, {{SSA, 0}}}, {WarnNonObjProp, (TCA)raisePropertyOnNonObject, DNone, SSync, {}}, {ThrowNonObjProp, (TCA)throw_null_object_prop, DNone, SSync, {}}, diff --git a/hphp/runtime/vm/translator/hopt/simplifier.cpp b/hphp/runtime/vm/translator/hopt/simplifier.cpp index 6033fba80..4a274cf48 100644 --- a/hphp/runtime/vm/translator/hopt/simplifier.cpp +++ b/hphp/runtime/vm/translator/hopt/simplifier.cpp @@ -337,7 +337,7 @@ SSATmp* Simplifier::simplifyGetCtxFwdCall(IRInstruction* inst) { SSATmp* Simplifier::simplifyLdCls(IRInstruction* inst) { SSATmp* clsName = inst->getSrc(0); if (clsName->isConst()) { - const Class* cls = Unit::lookupUniqueClass(clsName->getValStr()); + const Class* cls = Unit::lookupClass(clsName->getValStr()); if (cls) { if (RuntimeOption::RepoAuthoritative && (cls->attrs() & AttrUnique)) { // the class is unique diff --git a/hphp/runtime/vm/translator/hopt/tracebuilder.cpp b/hphp/runtime/vm/translator/hopt/tracebuilder.cpp index f9c136fa2..1c1f07e58 100644 --- a/hphp/runtime/vm/translator/hopt/tracebuilder.cpp +++ b/hphp/runtime/vm/translator/hopt/tracebuilder.cpp @@ -420,7 +420,7 @@ SSATmp* TraceBuilder::genJmpCond(SSATmp* boolSrc, Trace* target, bool negate) { void TraceBuilder::genJmp(Block* target, SSATmp* src) { EdgeData edge; - gen(Jmp_, target, &edge, src)->getInstruction(); + gen(Jmp_, target, &edge, src); } void TraceBuilder::genExitWhenSurprised(Trace* targetTrace) { @@ -515,18 +515,6 @@ SSATmp* TraceBuilder::genLdClsMethodCache(SSATmp* className, baseClass); } -// TODO(#2058871): move this to hhbctranslator -void TraceBuilder::genVerifyParamType(SSATmp* objClass, - SSATmp* className, - const Class* constraintClass, - Trace* exitTrace) { - // do NOT use genLdCls() since don't want to load class if it isn't loaded - SSATmp* constraint = - constraintClass ? genDefConst(constraintClass) - : gen(LdCachedClass, className); - gen(JmpNSame, getFirstBlock(exitTrace), objClass, constraint); -} - SSATmp* TraceBuilder::genBoxLoc(uint32_t id) { SSATmp* prevValue = genLdLoc(id); Type prevType = prevValue->getType(); @@ -584,6 +572,7 @@ SSATmp* TraceBuilder::genLdLoc(uint32_t id) { SSATmp* TraceBuilder::genLdLocAsCell(uint32_t id, Trace* exitTrace) { SSATmp* tmp = genLdLoc(id); Type type = tmp->getType(); + assert(type.isBoxed() || type.notBoxed()); if (!type.isBoxed()) { return tmp; } diff --git a/hphp/runtime/vm/translator/hopt/tracebuilder.h b/hphp/runtime/vm/translator/hopt/tracebuilder.h index db45ffebd..a44b41367 100644 --- a/hphp/runtime/vm/translator/hopt/tracebuilder.h +++ b/hphp/runtime/vm/translator/hopt/tracebuilder.h @@ -214,8 +214,6 @@ public: SSATmp* genLdStackAddr(int64_t offset) { return genLdStackAddr(m_spValue, offset); } - void genVerifyParamType(SSATmp* objClass, SSATmp* className, - const Class* constraint, Trace* exitTrace); void genNativeImpl(); @@ -297,6 +295,22 @@ public: m_trace->back()->setHint(h); } + struct DisableCseGuard { + DisableCseGuard(TraceBuilder& tb) + : m_tb(tb) + , m_oldEnable(tb.m_enableCse) + { + m_tb.m_enableCse = false; + } + ~DisableCseGuard() { + m_tb.m_enableCse = m_oldEnable; + } + + private: + TraceBuilder& m_tb; + bool m_oldEnable; + }; + /* * cond() generates if-then-else blocks within a trace. The caller * supplies lambdas to create the branch, next-body, and taken-body. @@ -307,9 +321,7 @@ public: SSATmp* cond(const Func* func, Branch branch, Next next, Taken taken) { Block* taken_block = m_irFactory.defBlock(func); Block* done_block = m_irFactory.defBlock(func, 1); - bool oldEnableCse = m_enableCse; - m_enableCse = false; - SCOPE_EXIT { m_enableCse = oldEnableCse; }; + DisableCseGuard guard(*this); branch(taken_block); SSATmp* v1 = next(); genJmp(done_block, v1); @@ -322,6 +334,41 @@ public: return result; } + /* + * ifThen generates if-then blocks within a trace that do not + * produce values. Code emitted in the taken lambda will be executed + * iff the branch emitted in the branch lambda is taken. + */ + template + void ifThen(const Func* func, Branch branch, Taken taken) { + Block* taken_block = m_irFactory.defBlock(func); + Block* done_block = m_irFactory.defBlock(func); + DisableCseGuard guard(*this); + branch(taken_block); + assert(!m_trace->back()->getNext()); + m_trace->back()->setNext(done_block); + appendBlock(taken_block); + taken(); + taken_block->setNext(done_block); + appendBlock(done_block); + } + + /* + * ifElse generates if-then-else blocks with an empty 'then' block + * that do not produce values. Code emitted in the next lambda will + * be executed iff the branch emitted in the branch lambda is not + * taken. + */ + template + void ifElse(const Func* func, Branch branch, Next next) { + Block* done_block = m_irFactory.defBlock(func); + DisableCseGuard guard(*this); + branch(done_block); + next(); + m_trace->back()->setNext(done_block); + appendBlock(done_block); + } + private: static void appendInstruction(IRInstruction* inst, Block* block); void appendInstruction(IRInstruction* inst); diff --git a/hphp/runtime/vm/translator/translator-runtime.cpp b/hphp/runtime/vm/translator/translator-runtime.cpp index 798917eb3..6b3773e23 100644 --- a/hphp/runtime/vm/translator/translator-runtime.cpp +++ b/hphp/runtime/vm/translator/translator-runtime.cpp @@ -16,7 +16,9 @@ #include "runtime/vm/translator/translator-runtime.h" +#include "runtime/ext/ext_function.h" #include "runtime/vm/member_operations.h" +#include "runtime/vm/type_constraint.h" namespace HPHP { namespace VM { namespace Transl { @@ -59,4 +61,27 @@ void raiseUndefProp(ObjectData* base, const StringData* name) { static_cast(base)->raiseUndefProp(name); } +void VerifyParamTypeFail(int paramNum) { + VMRegAnchor _; + const ActRec* ar = curFrame(); + const Func* func = ar->m_func; + const TypeConstraint& tc = func->params()[paramNum].typeConstraint(); + TypedValue* tv = frame_local(ar, paramNum); + assert(!tc.check(tv, func)); + tc.verifyFail(func, paramNum, tv); +} + +void VerifyParamTypeCallable(TypedValue value, int param) { + if (UNLIKELY(!f_is_callable(tvAsCVarRef(&value)))) { + VerifyParamTypeFail(param); + } +} + +HOT_FUNC_VM +void VerifyParamTypeSlow(const Class* cls, const Class* constraint, int param) { + if (UNLIKELY(!(constraint && cls->classof(constraint)))) { + VerifyParamTypeFail(param); + } +} + } } } diff --git a/hphp/runtime/vm/translator/translator-runtime.h b/hphp/runtime/vm/translator/translator-runtime.h index 4de397652..59c3ebd00 100644 --- a/hphp/runtime/vm/translator/translator-runtime.h +++ b/hphp/runtime/vm/translator/translator-runtime.h @@ -59,6 +59,10 @@ void bindNewElemIR(TypedValue* base, RefData* val, MInstrState* mis); RefData* box_value(TypedValue tv); void raisePropertyOnNonObject(); void raiseUndefProp(ObjectData* base, const StringData* name); +void VerifyParamTypeFail(int param); +void VerifyParamTypeCallable(TypedValue value, int param); +void VerifyParamTypeSlow(const Class* cls, const Class* constraint, int param); + int64_t switchDoubleHelper(int64_t val, int64_t base, int64_t nTargets); int64_t switchStringHelper(StringData* s, int64_t base, int64_t nTargets); diff --git a/hphp/runtime/vm/translator/translator-x64.cpp b/hphp/runtime/vm/translator/translator-x64.cpp index 0f829e910..c8538635e 100644 --- a/hphp/runtime/vm/translator/translator-x64.cpp +++ b/hphp/runtime/vm/translator/translator-x64.cpp @@ -311,14 +311,14 @@ classIsPersistent(const Class* cls) { TargetCache::isPersistentHandle(cls->m_cachedOffset); } -inline static bool +bool classIsUnique(const Class* cls) { return RuntimeOption::RepoAuthoritative && cls && (cls->attrs() & AttrUnique); } -inline static bool +bool classIsUniqueOrCtxParent(const Class* cls) { if (!cls) return false; if (classIsUnique(cls)) return true; @@ -327,7 +327,7 @@ classIsUniqueOrCtxParent(const Class* cls) { return ctx->classof(cls); } -inline static bool +bool classIsUniqueNormalClass(const Class* cls) { return classIsUnique(cls) && !(cls->attrs() & (AttrInterface | AttrTrait)); @@ -10323,32 +10323,6 @@ emitClassToReg(X64Assembler& a, const StringData* name, PhysReg r) { } } -static void -VerifyParamTypeFail(int paramNum) { - VMRegAnchor _; - const ActRec* ar = curFrame(); - const Func* func = ar->m_func; - const TypeConstraint& tc = func->params()[paramNum].typeConstraint(); - assert(tc.isObject()); - TypedValue* tv = frame_local(ar, paramNum); - TRACE(3, "%s Obj %s, needs type %s\n", - __func__, - tv->m_data.pobj->getVMClass()->name()->data(), - tc.typeName()->data()); - tc.verifyFail(func, paramNum, tv); -} - -// check class hierarchy and fail if no match -static void -VerifyParamTypeSlow(const Class* cls, const Class* constraint, int param) { - Stats::inc(Stats::Tx64_VerifyParamTypeSlow); - Stats::inc(Stats::Tx64_VerifyParamTypeSlowShortcut, -1); - - if (UNLIKELY(!(constraint && cls->classof(constraint)))) { - VerifyParamTypeFail(param); - } -} - static void VerifyParamCallable(ObjectData* obj, int param) { TypedValue tv; @@ -10428,7 +10402,6 @@ TranslatorX64::translateVerifyParamType(const Tracelet& t, IMM(param)); } } - } void diff --git a/hphp/runtime/vm/translator/translator-x64.h b/hphp/runtime/vm/translator/translator-x64.h index 0773a128a..d3d08cacb 100644 --- a/hphp/runtime/vm/translator/translator-x64.h +++ b/hphp/runtime/vm/translator/translator-x64.h @@ -1151,6 +1151,10 @@ TXFlags planInstrAdd_Int(const NormalizedInstruction& i); TXFlags planInstrAdd_Array(const NormalizedInstruction& i); void dumpTranslationInfo(const Tracelet& t, TCA postGuards); +bool classIsUnique(const Class* cls); +bool classIsUniqueOrCtxParent(const Class* cls); +bool classIsUniqueNormalClass(const Class* cls); + // SpaceRecorder is used in translator-x64.cpp and in hopt/irtranslator.cpp // RAII logger for TC space consumption. struct SpaceRecorder { diff --git a/hphp/runtime/vm/type_constraint.cpp b/hphp/runtime/vm/type_constraint.cpp index 3930d5233..2f58cd6e7 100644 --- a/hphp/runtime/vm/type_constraint.cpp +++ b/hphp/runtime/vm/type_constraint.cpp @@ -79,8 +79,8 @@ TypeConstraint::TypeConstraint(const StringData* typeName /* = NULL */, m_namedEntity = Unit::GetNamedEntity(typeName); return; } - if (RuntimeOption::EnableHipHopSyntax || dtype.m_dt == KindOfArray || - dtype.isParent() || dtype.isSelf()) { + if (RuntimeOption::EnableHipHopSyntax || dtype.m_dt == KindOfArray || + dtype.isParent() || dtype.isSelf()) { m_type = dtype; } else { m_type = { KindOfObject, Precise }; @@ -131,6 +131,14 @@ TypeConstraint::check(const TypedValue* tv, const Func* func) const { return equivDataTypes(m_type.m_dt, tv->m_type); } +bool +TypeConstraint::check(DataType dt) const { + assert(m_type.m_dt != KindOfObject); + assert(dt != KindOfRef); + if (m_nullable && IS_NULL_TYPE(dt)) return true; + return equivDataTypes(m_type.m_dt, dt); +} + void TypeConstraint::verifyFail(const Func* func, int paramNum, const TypedValue* tv) const { Transl::VMRegAnchor _; diff --git a/hphp/runtime/vm/type_constraint.h b/hphp/runtime/vm/type_constraint.h index 221e0bae7..8016e0bf4 100644 --- a/hphp/runtime/vm/type_constraint.h +++ b/hphp/runtime/vm/type_constraint.h @@ -78,7 +78,7 @@ public: bool isParent() const { return m_type.isParent(); } - + bool isCallable() const { return m_type.isCallable(); } @@ -112,6 +112,7 @@ public: } bool check(const TypedValue* tv, const Func* func) const; + bool check(DataType dt) const; // NB: will throw if the check fails. void verify(const TypedValue* tv, diff --git a/hphp/test/vm/verify-param-type.php b/hphp/test/vm/verify-param-type.php new file mode 100644 index 000000000..b7bc23228 --- /dev/null +++ b/hphp/test/vm/verify-param-type.php @@ -0,0 +1,15 @@ + + int(4096) + [1]=> + string(72) "Argument 1 passed to main() must be an instance of integer, double given" + [2]=> + "hphp/test/vm/verify-param-type.php" + [3]=> + int(12) + [4]=> + string(0) "" + [5]=> + array(2) { + [0]=> + array(2) { + ["file"]=> + "hphp/test/vm/verify-param-type.php" + ["line"]=> + int(12) + } + [1]=> + array(4) { + ["file"]=> + "hphp/test/vm/verify-param-type.php" + ["line"]=> + int(14) + ["function"]=> + string(4) "main" + ["args"]=> + array(1) { + [0]=> + float(0) + } + } + } +} +in main +done with main