Remove tvReadCell; use cellDup, sometimes with tvToCell instead

This function used memcpy on TypedValues where it doesn't
know the storage location, which is dangerous if the target lives in a
HphpArray (or anything else that uses m_aux).  It also just seems like
it's now an unnecessary/redundant entry point in the typed value API.
Several uses of it are on things that can't be KindOfRef anyway (I
fixed constants, but where I wasn't sure I used tvToCell first).
Esse commit está contido em:
Jordan DeLong
2013-07-05 13:13:18 -07:00
commit de Sara Golemon
commit 5732b1c491
10 arquivos alterados com 63 adições e 66 exclusões
+5 -5
Ver Arquivo
@@ -561,12 +561,12 @@ public:
m_clsCnsData[class_] = clsCnsData;
}
TypedValue* lookupClsCns(const HPHP::NamedEntity* ne,
const StringData* cls,
const StringData* cns);
Cell* lookupClsCns(const HPHP::NamedEntity* ne,
const StringData* cls,
const StringData* cns);
TypedValue* lookupClsCns(const StringData* cls,
const StringData* cns);
Cell* lookupClsCns(const StringData* cls,
const StringData* cns);
// Get the next outermost VM frame, even accross re-entry
ActRec* getOuterVMFrame(const ActRec* ar);
-14
Ver Arquivo
@@ -176,20 +176,6 @@ inline void tvUnbox(TypedValue* tv) {
assert(tvIsPlausible(tv));
}
// Assumes 'fr' is live and 'to' is dead. Store a reference to 'fr',
// as a Cell, into 'to'.
inline void tvReadCell(const TypedValue* fr, TypedValue* to) {
assert(tvIsPlausible(fr));
if (fr->m_type != KindOfRef) {
memcpy(to, fr, sizeof(TypedValue));
} else {
TypedValue* fr2 = fr->m_data.pref->tv();
to->m_data.num = fr2->m_data.num;
to->m_type = fr2->m_type;
}
tvRefcountedIncRef(to);
}
/*
* Raw copy of a TypedValue from one location to another, without
* doing any reference count manipulation.
+20 -20
Ver Arquivo
@@ -916,14 +916,14 @@ ActRec* VMExecutionContext::getOuterVMFrame(const ActRec* ar) {
return nullptr;
}
TypedValue* VMExecutionContext::lookupClsCns(const NamedEntity* ne,
Cell* VMExecutionContext::lookupClsCns(const NamedEntity* ne,
const StringData* cls,
const StringData* cns) {
Class* class_ = Unit::loadClass(ne, cls);
if (class_ == nullptr) {
raise_error(Strings::UNKNOWN_CLASS, cls->data());
}
TypedValue* clsCns = class_->clsCnsGet(cns);
Cell* clsCns = class_->clsCnsGet(cns);
if (clsCns == nullptr) {
raise_error("Couldn't find constant %s::%s",
cls->data(), cns->data());
@@ -3559,8 +3559,8 @@ inline void OPTBLD_INLINE VMExecutionContext::iopCns(PC& pc) {
m_stack.pushStaticString(s);
return;
}
Cell* c1 = m_stack.allocC();
tvReadCell(cns, c1);
auto const c1 = m_stack.allocC();
cellDup(*cns, *c1);
}
inline void OPTBLD_INLINE VMExecutionContext::iopCnsE(PC& pc) {
@@ -3570,8 +3570,8 @@ inline void OPTBLD_INLINE VMExecutionContext::iopCnsE(PC& pc) {
if (cns == nullptr) {
raise_error("Undefined constant '%s'", s->data());
}
Cell* c1 = m_stack.allocC();
tvReadCell(cns, c1);
auto const c1 = m_stack.allocC();
cellDup(*cns, *c1);
}
inline void OPTBLD_INLINE VMExecutionContext::iopCnsU(PC& pc) {
@@ -3591,8 +3591,8 @@ inline void OPTBLD_INLINE VMExecutionContext::iopCnsU(PC& pc) {
return;
}
}
Cell* c1 = m_stack.allocC();
tvReadCell(cns, c1);
auto const c1 = m_stack.allocC();
cellDup(*cns, *c1);
}
inline void OPTBLD_INLINE VMExecutionContext::iopDefCns(PC& pc) {
@@ -3609,12 +3609,12 @@ inline void OPTBLD_INLINE VMExecutionContext::iopClsCns(PC& pc) {
assert(tv->m_type == KindOfClass);
Class* class_ = tv->m_data.pcls;
assert(class_ != nullptr);
TypedValue* clsCns = class_->clsCnsGet(clsCnsName);
auto const clsCns = class_->clsCnsGet(clsCnsName);
if (clsCns == nullptr) {
raise_error("Couldn't find constant %s::%s",
class_->name()->data(), clsCnsName->data());
}
tvReadCell(clsCns, tv);
cellDup(*clsCns, *tv);
}
inline void OPTBLD_INLINE VMExecutionContext::iopClsCnsD(PC& pc) {
@@ -3624,11 +3624,11 @@ inline void OPTBLD_INLINE VMExecutionContext::iopClsCnsD(PC& pc) {
const NamedEntityPair& classNamedEntity =
m_fp->m_func->unit()->lookupNamedEntityPairId(classId);
TypedValue* clsCns = lookupClsCns(classNamedEntity.second,
classNamedEntity.first, clsCnsName);
auto const clsCns = lookupClsCns(classNamedEntity.second,
classNamedEntity.first, clsCnsName);
assert(clsCns != nullptr);
Cell* c1 = m_stack.allocC();
tvReadCell(clsCns, c1);
auto const c1 = m_stack.allocC();
cellDup(*clsCns, *c1);
}
inline void OPTBLD_INLINE VMExecutionContext::iopConcat(PC& pc) {
@@ -4353,7 +4353,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopCGetG(PC& pc) {
} \
refDup(*val, *output); \
} else { \
tvReadCell(val, output); \
cellDup(*tvToCell(val), *output); \
} \
m_stack.popA(); \
SPROP_OP_POSTLUDE \
@@ -4793,7 +4793,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopSetOpL(PC& pc) {
TypedValue* to = frame_local(m_fp, local);
SETOP_BODY(to, op, fr);
tvRefcountedDecRefCell(fr);
tvReadCell(to, fr);
cellDup(*tvToCell(to), *fr);
}
inline void OPTBLD_INLINE VMExecutionContext::iopSetOpN(PC& pc) {
@@ -4809,7 +4809,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopSetOpN(PC& pc) {
SETOP_BODY(to, op, fr);
tvRefcountedDecRef(fr);
tvRefcountedDecRef(tv2);
tvReadCell(to, tv2);
cellDup(*tvToCell(to), *tv2);
m_stack.discard();
decRefStr(name);
}
@@ -4827,7 +4827,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopSetOpG(PC& pc) {
SETOP_BODY(to, op, fr);
tvRefcountedDecRef(fr);
tvRefcountedDecRef(tv2);
tvReadCell(to, tv2);
cellDup(*tvToCell(to), *tv2);
m_stack.discard();
decRefStr(name);
}
@@ -4851,7 +4851,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopSetOpS(PC& pc) {
SETOP_BODY(val, op, fr);
tvRefcountedDecRefCell(propn);
tvRefcountedDecRef(fr);
tvReadCell(val, output);
cellDup(*tvToCell(val), *output);
m_stack.ndiscard(2);
decRefStr(name);
}
@@ -4891,7 +4891,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopSetOpM(PC& pc) {
}
tvRefcountedDecRef(rhs);
tvReadCell(result, rhs);
cellDup(*tvToCell(result), *rhs);
}
setHelperPost<1>(SETHELPERPOST_ARGS);
}
+9 -6
Ver Arquivo
@@ -938,18 +938,20 @@ HphpArray* Class::initClsCnsData() const {
return constants;
}
TypedValue* Class::cnsNameToTV(const StringData* clsCnsName,
Slot& clsCnsInd) const {
Cell* Class::cnsNameToTV(const StringData* clsCnsName,
Slot& clsCnsInd) const {
clsCnsInd = m_constants.findIndex(clsCnsName);
if (clsCnsInd == kInvalidSlot) {
return nullptr;
}
return const_cast<TypedValue*>(&m_constants[clsCnsInd].m_val);
auto const ret = const_cast<Cell*>(&m_constants[clsCnsInd].m_val);
assert(cellIsPlausible(ret));
return ret;
}
TypedValue* Class::clsCnsGet(const StringData* clsCnsName) const {
Cell* Class::clsCnsGet(const StringData* clsCnsName) const {
Slot clsCnsInd;
TypedValue* clsCns = cnsNameToTV(clsCnsName, clsCnsInd);
Cell* clsCns = cnsNameToTV(clsCnsName, clsCnsInd);
if (!clsCns || clsCns->m_type != KindOfUninit) {
return clsCns;
}
@@ -974,12 +976,13 @@ TypedValue* Class::clsCnsGet(const StringData* clsCnsName) const {
g_vmContext->invokeFuncFew(clsCns, meth86cinit, ActRec::encodeClass(this),
nullptr, 1, tv);
}
assert(cellIsPlausible(clsCns));
return clsCns;
}
DataType Class::clsCnsType(const StringData* cnsName) const {
Slot slot;
TypedValue* cns = cnsNameToTV(cnsName, slot);
auto const cns = cnsNameToTV(cnsName, slot);
// TODO: lookup the constant in target cache in case it's dynamic
// and already initialized.
if (!cns) return KindOfUninit;
+2 -2
Ver Arquivo
@@ -627,7 +627,7 @@ public:
*
* Performs dynamic initialization if necessary.
*/
TypedValue* clsCnsGet(const StringData* clsCnsName) const;
Cell* clsCnsGet(const StringData* clsCnsName) const;
/*
* Look up a class constant's TypedValue if it doesn't require
@@ -638,7 +638,7 @@ public:
* class constants need to run 86cinit code to determine their value
* at runtime.)
*/
TypedValue* cnsNameToTV(const StringData* name, Slot& slot) const;
Cell* cnsNameToTV(const StringData* name, Slot& slot) const;
/*
* Provide the current runtime type of this class constant. This
+4 -4
Ver Arquivo
@@ -4978,15 +4978,15 @@ void CodeGenerator::cgLdCns(IRInstruction* inst) {
cgLoad(rVmTl, ch, inst);
}
static TypedValue lookupCnsHelper(const TypedValue* tv, StringData* nm) {
static Cell lookupCnsHelper(const TypedValue* tv, StringData* nm) {
assert(tv->m_type == KindOfUninit);
TypedValue *cns = nullptr;
TypedValue c1;
Cell *cns = nullptr;
Cell c1;
if (UNLIKELY(tv->m_data.pref != nullptr)) {
ClassInfo::ConstantInfo* ci =
(ClassInfo::ConstantInfo*)(void*)tv->m_data.pref;
cns = const_cast<Variant&>(ci->getDeferredValue()).asTypedValue();
tvReadCell(cns, &c1);
cellDup(*cns, c1);
} else {
if (UNLIKELY(TargetCache::s_constants != nullptr)) {
cns = TargetCache::s_constants->HphpArray::nvGet(nm);
+3 -3
Ver Arquivo
@@ -845,7 +845,7 @@ CacheHandle allocClassConstant(StringData* name) {
sizeof(TypedValue), sizeof(TypedValue));
}
TypedValue*
Cell*
lookupClassConstant(TypedValue* cache,
const NamedEntity* ne,
const StringData* cls,
@@ -853,14 +853,14 @@ lookupClassConstant(TypedValue* cache,
Stats::inc(Stats::TgtCache_ClsCnsHit, -1);
Stats::inc(Stats::TgtCache_ClsCnsMiss, 1);
TypedValue* clsCns;
Cell* clsCns;
clsCns = g_vmContext->lookupClsCns(ne, cls, cns);
*cache = *clsCns;
return cache;
}
TypedValue
Cell
lookupClassConstantTv(TypedValue* cache,
const NamedEntity* ne,
const StringData* cls,
+4 -4
Ver Arquivo
@@ -1330,8 +1330,8 @@ static inline TypedValue setOpPropImpl(TypedValue* base, TypedValue keyVal,
TypedValue* result = HPHP::SetOpProp<isObj>(
mis->tvScratch, mis->tvRef, mis->ctx, op, base, &keyVal, &val);
TypedValue ret;
tvReadCell(result, &ret);
Cell ret;
cellDup(*tvToCell(result), ret);
return ret;
}
@@ -2249,8 +2249,8 @@ static inline TypedValue setOpElemImpl(TypedValue* base, TypedValue keyVal,
TypedValue* result =
HPHP::SetOpElem(mis->tvScratch, mis->tvRef, op, base, &keyVal, &val);
TypedValue ret;
tvReadCell(result, &ret);
Cell ret;
cellDup(*tvToCell(result), ret);
return ret;
}
+13 -5
Ver Arquivo
@@ -923,15 +923,21 @@ void Unit::initialMerge() {
}
}
TypedValue* Unit::lookupCns(const StringData* cnsName) {
Cell* Unit::lookupCns(const StringData* cnsName) {
TargetCache::CacheHandle handle = StringData::GetCnsHandle(cnsName);
if (LIKELY(handle != 0)) {
TypedValue& tv = TargetCache::handleToRef<TypedValue>(handle);
if (LIKELY(tv.m_type != KindOfUninit)) return &tv;
if (LIKELY(tv.m_type != KindOfUninit)) {
assert(cellIsPlausible(&tv));
return &tv;
}
if (UNLIKELY(tv.m_data.pref != nullptr)) {
ClassInfo::ConstantInfo* ci =
(ClassInfo::ConstantInfo*)(void*)tv.m_data.pref;
return const_cast<Variant&>(ci->getDeferredValue()).asTypedValue();
auto const tvRet = const_cast<Variant&>(
ci->getDeferredValue()).asTypedValue();
assert(cellIsPlausible(tvRet));
return tvRet;
}
}
if (UNLIKELY(TargetCache::s_constants != nullptr)) {
@@ -940,10 +946,12 @@ TypedValue* Unit::lookupCns(const StringData* cnsName) {
return nullptr;
}
TypedValue* Unit::lookupPersistentCns(const StringData* cnsName) {
Cell* Unit::lookupPersistentCns(const StringData* cnsName) {
TargetCache::CacheHandle handle = StringData::GetCnsHandle(cnsName);
if (!TargetCache::isPersistentHandle(handle)) return nullptr;
return &TargetCache::handleToRef<TypedValue>(handle);
auto const ret = &TargetCache::handleToRef<TypedValue>(handle);
assert(cellIsPlausible(ret));
return ret;
}
TypedValue* Unit::loadCns(const StringData* cnsName) {
+3 -3
Ver Arquivo
@@ -495,9 +495,9 @@ struct Unit {
static bool aliasClass(Class* original, const StringData* alias);
void defTypedef(Id id);
static TypedValue* lookupCns(const StringData* cnsName);
static TypedValue* lookupPersistentCns(const StringData* cnsName);
static TypedValue* loadCns(const StringData* cnsName);
static Cell* lookupCns(const StringData* cnsName);
static Cell* lookupPersistentCns(const StringData* cnsName);
static Cell* loadCns(const StringData* cnsName);
static bool defCns(const StringData* cnsName, const TypedValue* value,
bool persistent = false);
static uint64_t defCnsHelper(uint64_t ch,