Skip to content

Commit

Permalink
Standardize when we trust a Class* 3/n
Browse files Browse the repository at this point in the history
Summary:
- Unify logic that checks whether a Class* is known
- Renamed `lookupUnique()` helpers to be `lookupKnown` since we no longer use the `AttrUnique`, and we don't just test uniqueness, since we run this in sandbox mode when we can guarantee the translation will be re-jitted when things change.

Reviewed By: ricklavoie

Differential Revision: D57010553

fbshipit-source-id: 252d20cf7ccd65684c6191cecc7883c74bd960ad
  • Loading branch information
voorka authored and facebook-github-bot committed May 16, 2024
1 parent 405a8e2 commit d3251d8
Show file tree
Hide file tree
Showing 14 changed files with 69 additions and 66 deletions.
3 changes: 1 addition & 2 deletions hphp/doc/ir.specification
Original file line number Diff line number Diff line change
Expand Up @@ -630,8 +630,7 @@ To string conversions:
| InstanceOfIface, D(Bool), S(Cls) CStr, NF

Fast path for interface checks. Sets D based on whether S0 implements S1, but
S1 must be a unique interface. This should only be used in repo-authoritative
mode.
S1 must be a known interface.

| InstanceOfIfaceVtable<iface,canOptimize>, D(Bool), S(Cls), NF

Expand Down
20 changes: 8 additions & 12 deletions hphp/runtime/vm/class-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -772,27 +772,23 @@ inline Class* Class::lookup(const StringData* name) {
* a non-strict subtype of cls. If cls changes, ctx will be
* re-loaded and re-jitted anyways, so we can trust cls.
*/
inline const Class* Class::lookupUniqueInContext(const Class* cls,
const Class* ctx,
const Unit* unit) {
inline const Class* Class::lookupKnown(const Class* cls,
const Class* ctx) {
if (UNLIKELY(cls == nullptr)) return nullptr;
if (cls->attrs() & AttrPersistent) return cls;
if (unit && cls->preClass()->unit() == unit) return cls;
if (!ctx) return nullptr;
return ctx->classof(cls) ? cls : nullptr;
}

inline const Class* Class::lookupUniqueInContext(const NamedType* ne,
const Class* ctx,
const Unit* unit) {
inline const Class* Class::lookupKnown(const NamedType* ne,
const Class* ctx) {
Class* cls = ne->clsList();
return lookupUniqueInContext(cls, ctx, unit);
return lookupKnown(cls, ctx);
}

inline const Class* Class::lookupUniqueInContext(const StringData* name,
const Class* ctx,
const Unit* unit) {
return lookupUniqueInContext(NamedType::getOrCreate(name), ctx, unit);
inline const Class* Class::lookupKnown(const StringData* name,
const Class* ctx) {
return lookupKnown(NamedType::getOrCreate(name), ctx);
}

inline Class* Class::load(const StringData* name) {
Expand Down
15 changes: 6 additions & 9 deletions hphp/runtime/vm/class.h
Original file line number Diff line number Diff line change
Expand Up @@ -1504,15 +1504,12 @@ struct Class : AtomicCountable {
*
* Return nullptr if there is no such class.
*/
static const Class* lookupUniqueInContext(const NamedType* ne,
const Class* ctx,
const Unit* unit);
static const Class* lookupUniqueInContext(const StringData* name,
const Class* ctx,
const Unit* unit);
static const Class* lookupUniqueInContext(const Class* cls,
const Class* ctx,
const Unit* unit);
static const Class* lookupKnown(const NamedType* ne,
const Class* ctx);
static const Class* lookupKnown(const StringData* name,
const Class* ctx);
static const Class* lookupKnown(const Class* cls,
const Class* ctx);

/*
* Same as Class::load but also checks for module boundary violations
Expand Down
3 changes: 1 addition & 2 deletions hphp/runtime/vm/instance-bits.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,7 @@ void init() {
uint64_t accum = 0;
for (auto& item : counts) {
if (i >= kNumInstanceBits) break;
auto const cls = Class::lookupUniqueInContext(
item.first, nullptr, nullptr);
auto const cls = Class::lookupKnown(item.first, nullptr);
if (cls) {
assertx(cls->attrs() & AttrPersistent);
s_instanceBitsMap[item.first] = i;
Expand Down
2 changes: 1 addition & 1 deletion hphp/runtime/vm/jit/ir-instruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ Type newColReturn(const IRInstruction* inst) {
assertx(inst->is(NewCol, NewPair, NewColFromArray));
auto getColClassType = [&](CollectionType ct) -> Type {
auto name = collections::typeToString(ct);
auto cls = Class::lookupUniqueInContext(name, inst->ctx(), nullptr);
auto cls = Class::lookupKnown(name, inst->ctx());
if (cls == nullptr) return TObj;
return Type::ExactObj(cls);
};
Expand Down
26 changes: 13 additions & 13 deletions hphp/runtime/vm/jit/irgen-call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ const Class* callContext(IRGS& env, const FCallArgs& fca, const Class* cls) {
if (RO::RepoAuthoritative) PUNT(Bad-Dyn-Override);
return cls;
}
return lookupUniqueClass(env, fca.context, true /* trustUnit */);
return lookupKnownWithUnit(env, fca.context);
}

bool emitCallerInOutChecksKnown(IRGS& env, const Func* callee,
Expand Down Expand Up @@ -769,7 +769,7 @@ void fcallObjMethodUnknown(

auto const callerCtx = [&] {
if (!fca.context) return curClass(env);
auto const ret = lookupUniqueClass(env, fca.context, true /* trustUnit */);
auto const ret = lookupKnownWithUnit(env, fca.context);
if (!ret) PUNT(no-context);
return ret;
}();
Expand Down Expand Up @@ -1094,7 +1094,7 @@ void fcallObjMethodObj(IRGS& env, const FCallArgs& fca, SSATmp* obj,
if (!methodName->hasConstVal()) return notFound;

if (!clsHint->empty()) {
auto const cls = lookupUniqueClass(env, clsHint);
auto const cls = lookupKnownWithUnit(env, clsHint);
if (cls && isNormalClass(cls)) {
obj = gen(env, AssertType, Type::SubObj(cls), obj);
auto const callCtx =
Expand Down Expand Up @@ -1389,7 +1389,7 @@ void emitFCallFuncD(IRGS& env, FCallArgs fca, const StringData* funcName) {
auto const func = lookupImmutableFunc(funcName);
auto const callerCtx = [&] {
if (!fca.context) return curClass(env);
auto const ret = lookupUniqueClass(env, fca.context, true /* trustUnit */);
auto const ret = lookupKnownWithUnit(env, fca.context);
if (!ret) PUNT(no-context);
return ret;
}();
Expand Down Expand Up @@ -1444,7 +1444,7 @@ void emitResolveMethCaller(IRGS& env, const StringData* name) {
auto const methodName = func->methCallerMethName();

auto const ok = [&] () -> bool {
auto const cls = lookupUniqueClass(env, className);
auto const cls = lookupKnownWithUnit(env, className);
if (cls && !isTrait(cls)) {
auto const callCtx = MemberLookupContext(curClass(env), curFunc(env));
auto const res = lookupImmutableObjMethod(cls, methodName, callCtx, false);
Expand Down Expand Up @@ -1541,7 +1541,7 @@ void emitNewObj(IRGS& env) {
}

void emitNewObjD(IRGS& env, const StringData* className) {
auto const cls = lookupUniqueClass(env, className);
auto const cls = lookupKnownWithUnit(env, className);

auto const knownClass = [&]() {
bool const canInstantiate = isNormalClass(cls) && !isAbstract(cls);
Expand Down Expand Up @@ -1590,7 +1590,7 @@ void emitFCallCtor(IRGS& env, FCallArgs fca, const StringData* clsHint) {

auto const exactCls = [&] {
if (!clsHint->empty()) {
auto const cls = lookupUniqueClass(env, clsHint);
auto const cls = lookupKnownWithUnit(env, clsHint);
if (cls && isNormalClass(cls)) return cls;
}
return obj->type().clsSpec().exactCls();
Expand Down Expand Up @@ -1716,7 +1716,7 @@ void emitFCallClsMethodD(IRGS& env,
FCallArgs fca,
const StringData* className,
const StringData* methodName) {
auto const cls = lookupUniqueClass(env, className);
auto const cls = lookupKnownWithUnit(env, className);
if (cls) {
auto const callCtx =
MemberLookupContext(callContext(env, fca, cls), curFunc(env));
Expand All @@ -1730,7 +1730,7 @@ void emitFCallClsMethodD(IRGS& env,

auto const callerCtx = [&] {
if (!fca.context) return curClass(env);
auto const ret = lookupUniqueClass(env, fca.context, true /* trustUnit */);
auto const ret = lookupKnownWithUnit(env, fca.context);
if (!ret) PUNT(no-context);
return ret;
}();
Expand Down Expand Up @@ -1837,7 +1837,7 @@ void resolveClsMethodCommon(IRGS& env, SSATmp* clsVal,
void checkClsMethodAndLdCtx(IRGS& env, const Class* cls, const Func* func,
const StringData* className) {
gen(env, CheckClsMethFunc, cns(env, func));
if (!classIsTrusted(env, cls)) {
if (!classIsKnown(env, cls)) {
gen(env, LdClsCached, LdClsFallbackData::Fatal(), cns(env, className));
}
ldCtxForClsMethod(env, func, cns(env, cls), cls, true);
Expand Down Expand Up @@ -1870,7 +1870,7 @@ void emitResolveClsMethod(IRGS& env, const StringData* methodName) {

void emitResolveClsMethodD(IRGS& env, const StringData* className,
const StringData* methodName) {
auto const cls = lookupUniqueClass(env, className, false /* trustUnit */);
auto const cls = lookupKnownWithUnit(env, className);
if (cls) {
auto const callCtx = MemberLookupContext(curClass(env), curFunc(env));
auto const func = lookupImmutableClsMethod(cls, methodName, callCtx, true);
Expand Down Expand Up @@ -1916,7 +1916,7 @@ void emitResolveRClsMethodD(IRGS& env, const StringData* className,
return interpOne(env);
}

auto const cls = lookupUniqueClass(env, className, false /* trustUnit */);
auto const cls = lookupKnownWithUnit(env, className);
if (cls) {
auto const callCtx = MemberLookupContext(curClass(env), curFunc(env));
auto const func = lookupImmutableClsMethod(cls, methodName, callCtx, true);
Expand Down Expand Up @@ -1996,7 +1996,7 @@ void fcallClsMethodCommon(IRGS& env,
auto const methodName = methVal->strVal();
auto const knownClass = [&] () -> std::pair<const Class*, bool> {
if (!clsHint->empty()) {
auto const cls = lookupUniqueClass(env, clsHint);
auto const cls = lookupKnownWithUnit(env, clsHint);
if (cls && isNormalClass(cls)) return std::make_pair(cls, true);
}

Expand Down
2 changes: 1 addition & 1 deletion hphp/runtime/vm/jit/irgen-cns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ void exactClsCns(IRGS& env,
// or a parent of the current context, and this constant is a scalar
// constant, we can just compile it to a literal.
auto cnsType = TInitCell;
if (classIsTrusted(env, cls)) {
if (classIsKnown(env, cls)) {
Slot ignore;
if (auto const tv = cls->cnsNameToTV(cnsNameStr, ignore)) {
if (type(tv) != KindOfUninit) {
Expand Down
23 changes: 13 additions & 10 deletions hphp/runtime/vm/jit/irgen-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -658,17 +658,20 @@ inline SSATmp* ldCtxCls(IRGS& env) {
//////////////////////////////////////////////////////////////////////
// Other common helpers

inline const Class* lookupUniqueClass(IRGS& env,
const StringData* name,
bool trustUnit = false) {
// TODO: Once top level code is entirely dead it should be safe to always
// trust the unit.
return Class::lookupUniqueInContext(
name, curClass(env), trustUnit ? curUnit(env) : nullptr);
inline const Class* lookupKnownWithUnit(IRGS& env, const Class* cls) {
auto const unit = curUnit(env);
if (cls && cls->preClass()->unit() == unit) return cls;
return Class::lookupKnown(cls, curClass(env));
}

inline bool classIsTrusted(IRGS& env, const Class* cls) {
return Class::lookupUniqueInContext(cls, curClass(env), nullptr) != nullptr;
inline const Class* lookupKnownWithUnit(IRGS& env, const StringData* name) {
auto const ne = NamedType::getOrCreate(name);
auto const cls = ne->clsList();
return lookupKnownWithUnit(env, cls);
}

inline bool classIsKnown(IRGS& env, const Class* cls) {
return lookupKnownWithUnit(env, cls) != nullptr;
}

inline SSATmp* ldCls(IRGS& env,
Expand All @@ -679,7 +682,7 @@ inline SSATmp* ldCls(IRGS& env,
if (lazyClassOrName->hasConstVal()) {
auto const cnameStr = isLazy ? lazyClassOrName->lclsVal().name() :
lazyClassOrName->strVal();
auto const cls = lookupUniqueClass(env, cnameStr);
auto const cls = lookupKnownWithUnit(env, cnameStr);
if (cls) {
return cns(env, cls);
} else {
Expand Down
8 changes: 4 additions & 4 deletions hphp/runtime/vm/jit/irgen-types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const StaticString
* Returns a {Cls|Nullptr} suitable for use in instance checks.
*/
SSATmp* ldClassSafe(IRGS& env, const StringData* className) {
if (auto const knownCls = lookupUniqueClass(env, className)) {
if (auto const knownCls = lookupKnownWithUnit(env, className)) {
return cns(env, knownCls);
}

Expand Down Expand Up @@ -93,7 +93,7 @@ SSATmp* implInstanceCheck(IRGS& env, SSATmp* src, const StringData* className,
}

auto knownCls = checkCls->hasConstVal(TCls) ? checkCls->clsVal() : nullptr;
assertx(IMPLIES(knownCls, classIsTrusted(env, knownCls)));
assertx(IMPLIES(knownCls, classIsKnown(env, knownCls)));
assertx(IMPLIES(knownCls, knownCls->name()->tsame(className)));

auto const srcType = src->type();
Expand Down Expand Up @@ -402,7 +402,7 @@ void verifyTypeImpl(IRGS& env,
auto const clsName = tc.isSubObject() ? tc.clsName() : tc.typeName();
if (cls->name()->same(clsName)) return AnnotAction::Pass;

if (auto const knownCls = lookupUniqueClass(env, clsName)) {
if (auto const knownCls = lookupKnownWithUnit(env, clsName)) {
// Subclass of a unique class.
if (cls->classof(knownCls)) return AnnotAction::Pass;
}
Expand Down Expand Up @@ -1000,7 +1000,7 @@ bool emitIsTypeStructWithoutResolvingIfPossible(
auto const classnameForResolvedClass = [&](const ArrayData* arr) -> const StringData* {
auto const clsname = get_ts_classname(arr);
if (arr->exists(s_generic_types)) {
auto cls = lookupUniqueClass(env, clsname);
auto cls = lookupKnownWithUnit(env, clsname);
if ((cls && cls->hasReifiedGenerics()) || !isTSAllWildcards(arr)) {
// If it is a reified class or has non wildcard generics,
// we need to bail
Expand Down
3 changes: 1 addition & 2 deletions hphp/runtime/vm/jit/prof-data-serialize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1478,8 +1478,7 @@ void write_class(ProfDataSerializer& ser, const Class* cls) {

if (cls->preClass()->attrs() & AttrNoExpandTrait) {
for (auto const tName : cls->preClass()->usedTraits()) {
auto const trait =
Class::lookupUniqueInContext(tName, nullptr, nullptr);
auto const trait = Class::lookupKnown(tName, nullptr);
assertx(trait);
record_dep(trait);
}
Expand Down
10 changes: 6 additions & 4 deletions hphp/runtime/vm/jit/simplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ SSATmp* simplifyLdCls(State& env, const IRInstruction* inst) {
if (str->hasConstVal() && (cls->hasConstVal(TCls) || cls->isA(TNullptr))) {
auto const sval = str->strVal();
auto const cval = cls->hasConstVal(TCls) ? cls->clsVal() : nullptr;
auto const result = Class::lookupUniqueInContext(sval, cval, nullptr);
auto const result = Class::lookupKnown(sval, cval);
if (result) return cns(env, result);
}
return nullptr;
Expand Down Expand Up @@ -1888,8 +1888,10 @@ SSATmp* simplifyInstanceOfIface(State& env, const IRInstruction* inst) {
auto const src1 = inst->src(0);
auto const src2 = inst->src(1);

auto const cls2 = Class::lookupUniqueInContext(
src2->strVal(), inst->ctx(), nullptr);
// We only emit InstanceOfIface if we checked that we could trust the class.
// Just grab it from the named-entity map.
auto const ne = NamedType::getOrCreate(src2->strVal());
auto const cls2 = ne->clsList();
assertx(cls2 && isInterface(cls2));
auto const spec2 = ClassSpec{cls2, ClassSpec::ExactTag{}};

Expand Down Expand Up @@ -3557,7 +3559,7 @@ SSATmp* simplifyLookupClsRDS(State& env, const IRInstruction* inst) {
}
if (str->hasConstVal()) {
auto const sval = str->strVal();
auto const result = Class::lookupUniqueInContext(sval, nullptr, nullptr);
auto const result = Class::lookupKnown(sval, nullptr);
if (result) return cns(env, result);
}
return nullptr;
Expand Down
7 changes: 3 additions & 4 deletions hphp/runtime/vm/jit/type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -903,8 +903,7 @@ Type typeFromRAT(RepoAuthType ty, const Class* ctx) {
#define NAME_SPEC(type, spec) \
[&] { \
assertx(ty.name() != nullptr); \
auto const cls = \
Class::lookupUniqueInContext(ty.name(), ctx, nullptr); \
auto const cls = Class::lookupKnown(ty.name(), ctx); \
return cls ? Type::spec(cls) : type; \
}() \

Expand Down Expand Up @@ -1037,7 +1036,7 @@ Type typeFromTCImpl(const HPHP::TypeConstraint& tc,
// Don't try to be clever with magic interfaces.
if (interface_supports_non_objects(tc.clsName())) return TInitCell;

auto const cls = Class::lookupUniqueInContext(tc.clsName(), ctx, nullptr);
auto const cls = Class::lookupKnown(tc.clsName(), ctx);
if (!cls) return TObj;
assertx(!isEnum(cls));
return Type::SubObj(cls);
Expand All @@ -1046,7 +1045,7 @@ Type typeFromTCImpl(const HPHP::TypeConstraint& tc,
assertx(tc.isUnresolved());
if (interface_supports_non_objects(tc.typeName())) return TInitCell;

auto const cls = Class::lookupUniqueInContext(tc.typeName(), ctx, nullptr);
auto const cls = Class::lookupKnown(tc.typeName(), ctx);
if (cls) {
if (isEnum(cls)) {
assertx(tc.isUnresolved());
Expand Down
1 change: 1 addition & 0 deletions hphp/test/slow/profile/setprofile-flags.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?hh

class SomeClass {
public function __construct (){}
function some_method() :mixed{}
}

Expand Down

0 comments on commit d3251d8

Please sign in to comment.