Reorder module exceptions and notices for string-to-class conversion + dynamically referenced

Summary:
Class::resolve throws a module exception before returning the Class* used for the __DR check, but I misordered this in the JIT. String-to-class notice was raised before the module exception, since it doesn't actually need the Class* .

This made it so class-ptr tests had a divergence between interp and jit mode when running w/ EnforceModules=2, DynamicallyReferenced...=1.

Reorder the ClassGetC / FCallClsMethodM checks when cls input is string:

- Class::resolve
  - Class::load -> Class*
  - module check (exn when EnforceModules=2)
- DITCH checks:
  - Normal: raise str-to-class
  - ExplicitConversion: check __DR

As a result, we no longer raise the str-to-class notice when the class doesn't exist

Reviewed By: jtwarren

Differential Revision: D71159208

fbshipit-source-id: ab135a99371745cca0d3ab8be42db0fb8381e919

Author: vassilmladenov

hphp/runtime/vm/bytecode.cpp

@@ -979,9 +979,8 @@ static inline Class* classnameToClass(TypedValue* input) {
   if (tvIsString(input)) {
     auto const name = input->m_data.pstr;
     if (Class* class_ = Class::resolve(name, vmfp()->func())) {
-      if (folly::Random::oneIn(Cfg::Eval::DynamicallyReferencedNoticeSampleRate) &&
-          !class_->isDynamicallyReferenced()) {
-        raise_notice(Strings::MISSING_DYNAMICALLY_REFERENCED, name->data());
+      if (!class_->isDynamicallyReferenced()) {
+        raiseMissingDynamicallyReferenced(class_);
       }
       return class_;
     }
@@ -1006,8 +1005,8 @@ static inline Class* classnameToClass(TypedValue* input) {
 static inline Class* lookupClsRef(TypedValue* input) {
   if (tvIsString(input)) {
     auto const name = input->m_data.pstr;
-    raise_str_to_class_notice(name);
     if (Class* class_ = Class::resolve(name, vmfp()->func())) {
+      raise_str_to_class_notice(name);
       return class_;
     }
     raise_error(Strings::UNKNOWN_CLASS, name->data());

hphp/runtime/vm/jit/irgen-basic.cpp

@@ -60,47 +60,56 @@ void emitClassGetC(IRGS& env, ClassGetCMode mode) {
     return;
   }
 
-  auto const cls = [&] {
+  auto const fallback = [&] {
     switch (mode) {
       case ClassGetCMode::Normal:
-        if (Cfg::Eval::RaiseStrToClsConversionNoticeSampleRate > 0
-            && name->isA(TStr)) {
-          gen(env, RaiseStrToClassNotice, name);
-        }
-        return ldCls(env, name);
+        return LdClsFallback::Fatal;
       case ClassGetCMode::ExplicitConversion:
         // HH\classname_to_class throws a catchable InvalidArgumentException
         // instead of raising a fatal error
         if (name->isA(TStr)) {
-          auto const cls = ldCls(env, name, LdClsFallback::ThrowClassnameToClassString);
-          if (Cfg::Eval::DynamicallyReferencedNoticeSampleRate > 0) {
-            if (cls->hasConstVal() &&
-                !cls->clsVal()->isDynamicallyReferenced()) {
-              gen(env, RaiseMissingDynamicallyReferenced, cls);
-            } else {
-              ifThen(
-                env,
-                [&] (Block* taken) {
-                  auto const data = AttrData { AttrDynamicallyReferenced };
-                  gen(env, JmpZero, taken, gen(env, ClassHasAttr, data, cls));
-                },
-                [&] {
-                  hint(env, Block::Hint::Unlikely);
-                  gen(env, RaiseMissingDynamicallyReferenced, cls);
-                }
-              );
-            }
-          }
-          return cls;
+          return LdClsFallback::ThrowClassnameToClassString;
         } else { // TLazyCls
-          return ldCls(env, name, LdClsFallback::ThrowClassnameToClassLazyClass);
+          return LdClsFallback::ThrowClassnameToClassLazyClass;
         }
     }
   }();
+  auto const cls = ldCls(env, name, fallback);
+
+  if (name->isA(TStr)) {
+    emitModuleBoundaryCheck(env, cls, false);
+
+    switch (mode) {
+      case ClassGetCMode::Normal:
+        if (Cfg::Eval::RaiseStrToClsConversionNoticeSampleRate > 0) {
+          gen(env, RaiseStrToClassNotice, name);
+        }
+        break;
+      case ClassGetCMode::ExplicitConversion:
+        if (Cfg::Eval::DynamicallyReferencedNoticeSampleRate > 0) {
+          if (cls->hasConstVal() &&
+              !cls->clsVal()->isDynamicallyReferenced()) {
+            gen(env, RaiseMissingDynamicallyReferenced, cls);
+          } else {
+            ifThen(
+              env,
+              [&] (Block* taken) {
+                auto const data = AttrData { AttrDynamicallyReferenced };
+                gen(env, JmpZero, taken, gen(env, ClassHasAttr, data, cls));
+              },
+              [&] {
+                hint(env, Block::Hint::Unlikely);
+                gen(env, RaiseMissingDynamicallyReferenced, cls);
+              }
+            );
+          }
+        }
+        break;
+    }
+  }
 
   popC(env);
   decRef(env, name);
-  if (name->isA(TStr)) emitModuleBoundaryCheck(env, cls, false);
   push(env, cls);
 }
 

hphp/runtime/vm/jit/irgen-call.cpp

@@ -470,7 +470,7 @@ void callProfiledFunc(IRGS& env, SSATmp* callee, SSATmp* objOrClass,
     return result;
   }();
 
-  speculateTargetFunction(env, callee, callKnown, callUnknown, choices, 
+  speculateTargetFunction(env, callee, callKnown, callUnknown, choices,
                           0, Cfg::Jit::PGOCalledFuncCheckNumSpeculations);
 }
 
@@ -2225,13 +2225,15 @@ void emitFCallClsMethodM(IRGS& env, FCallArgs fca, const StringData* clsHint,
   }
   auto const cls = [&] {
     if (name->isA(TCls)) return name;
-    if (name->isA(TStr) &&
-      Cfg::Eval::RaiseStrToClsConversionNoticeSampleRate > 0) {
-      gen(env, RaiseStrToClassNotice, name);
-    }
     auto const ret = name->isA(TObj) ?
       gen(env, LdObjClass, name) : ldCls(env, name);
-    if (name->isA(TStr)) emitModuleBoundaryCheck(env, ret, false);
+    if (name->isA(TStr)) {
+      emitModuleBoundaryCheck(env, ret, false);
+
+      if(Cfg::Eval::RaiseStrToClsConversionNoticeSampleRate > 0) {
+        gen(env, RaiseStrToClassNotice, name);
+      }
+    }
     decRef(env, name);
     return ret;
   }();

hphp/runtime/vm/jit/irlower-exception.cpp

@@ -223,12 +223,6 @@ void cgRaiseForbiddenDynConstruct(IRLS& env, const IRInstruction* inst) {
                kVoidDest, SyncOptions::Sync, argGroup(env, inst).ssa(0));
 }
 
-static void raiseMissingDynamicallyReferenced(const Class* cls) {
-  if (folly::Random::oneIn(Cfg::Eval::DynamicallyReferencedNoticeSampleRate)) {
-    raise_notice(Strings::MISSING_DYNAMICALLY_REFERENCED, cls->name()->data());
-  }
-}
-
 void cgRaiseMissingDynamicallyReferenced(IRLS& env, const IRInstruction* inst) {
   cgCallHelper(
     vmain(env),

hphp/runtime/vm/runtime.cpp

@@ -554,6 +554,13 @@ void raiseDeploymentBoundaryViolation(const Class* cls) {
   }
 }
 
+const StaticString s___DynamicallyReferenced("__DynamicallyReferenced");
+
+void raiseMissingDynamicallyReferenced(const Class* cls) {
+  if (folly::Random::oneIn(Cfg::Eval::DynamicallyReferencedNoticeSampleRate)) {
+    raise_notice(Strings::MISSING_DYNAMICALLY_REFERENCED, cls->name()->data());
+  }
+}
 //////////////////////////////////////////////////////////////////////
 
 int64_t zero_error_level() {

hphp/runtime/vm/runtime.h

@@ -99,6 +99,8 @@ void raiseModulePropertyViolation(const Class* cls,
 void raiseDeploymentBoundaryViolation(const Func* callee);
 void raiseDeploymentBoundaryViolation(const Class* cls);
 
+void raiseMissingDynamicallyReferenced(const Class* cls);
+
 inline Iter*
 frame_iter(const ActRec* fp, int i) {
   return (Iter*)(uintptr_t(fp)

hphp/test/slow/class-ptr/module-launder.php.hphp_opts

@@ -1,2 +1 @@
 --inputs=__DIR__/module.inc
--vRuntime.Eval.DynamicallyReferencedNoticeSampleRate=0

hphp/test/slow/class-ptr/module-launder.php.opts

@@ -1,2 +1 @@
 -vEval.EnforceModules=2
--vEval.DynamicallyReferencedNoticeSampleRate=0

hphp/test/slow/class-ptr/module-pair-launder.php.hphp_opts

@@ -1,3 +1,2 @@
 --inputs=__DIR__/module-pair-a.inc
 --inputs=__DIR__/module-pair-b-launder.inc
--vRuntime.Eval.DynamicallyReferencedNoticeSampleRate=0

hphp/test/slow/class-ptr/module-pair-launder.php.opts

@@ -1,2 +1 @@
 -vEval.EnforceModules=2
--vEval.DynamicallyReferencedNoticeSampleRate=0

hphp/test/slow/class-ptr/module-pair.php.hphp_opts

@@ -1,3 +1,2 @@
 --inputs=__DIR__/module-pair-a.inc
 --inputs=__DIR__/module-pair-b.inc
--vRuntime.Eval.DynamicallyReferencedNoticeSampleRate=0

hphp/test/slow/class-ptr/module-pair.php.opts

@@ -1,2 +1 @@
 -vEval.EnforceModules=2
--vEval.DynamicallyReferencedNoticeSampleRate=0

hphp/test/slow/class-ptr/module.php.hphp_opts

@@ -1,2 +1 @@
 --inputs=__DIR__/module.inc
--vRuntime.Eval.DynamicallyReferencedNoticeSampleRate=0

hphp/test/slow/class-ptr/module.php.opts

@@ -1,2 +1 @@
 -vEval.EnforceModules=2
--vEval.DynamicallyReferencedNoticeSampleRate=0