Do not allow hack exceptions in setprofile callbacks

Summary:
If a setprofile callback fails with a Hack exception, turn it into a fatal
instead. Similar to what we do for many other handlers.

It is not safe to throw Hack exceptions from some event hooks, such as
EventHook::FunctionResumeAwait(), where they cause segfaults.

Reviewed By: mdko

Differential Revision: D107585135

fbshipit-source-id: 1b5de630f8f8e71b93ac5105e3d03a1b43442b05

Author: jano

hphp/runtime/vm/event-hook.cpp

@@ -339,9 +339,14 @@ void runUserProfilerOnFunctionEnter(const ActRec* ar, bool isResume) {
     frameinfo
   );
 
-  // TODO(named_params) thread named arg names to profiling
-  g_context->invokeFunc(func, params, nullptr /* namedArgNames */, ctx.this_,
-                        ctx.cls, RuntimeCoeffects::defaults(), ctx.dynamic);
+  try {
+    // TODO(named_params) thread named arg names to profiling
+    g_context->invokeFunc(func, params, nullptr /* namedArgNames */, ctx.this_,
+                          ctx.cls, RuntimeCoeffects::defaults(), ctx.dynamic);
+  } catch (Object& ex) {
+    raise_error("Uncaught exception escaping setprofile callback: %s",
+                throwable_to_string(ex.get()).data());
+  }
 }
 
 void runUserProfilerOnFunctionExit(const ActRec* ar, const TypedValue* retval,
@@ -374,10 +379,15 @@ void runUserProfilerOnFunctionExit(const ActRec* ar, const TypedValue* retval,
     frameinfo
   );
 
-  // TODO(named_params) add user profiler support for calls with named args.
-  const ArrayData* namedArgNames = nullptr;
-  g_context->invokeFunc(func, params, namedArgNames, ctx.this_, ctx.cls,
-                        RuntimeCoeffects::defaults(), ctx.dynamic);
+  try {
+    // TODO(named_params) add user profiler support for calls with named args.
+    const ArrayData* namedArgNames = nullptr;
+    g_context->invokeFunc(func, params, namedArgNames, ctx.this_, ctx.cls,
+                          RuntimeCoeffects::defaults(), ctx.dynamic);
+  } catch (Object& ex) {
+    raise_error("Uncaught exception escaping setprofile callback: %s",
+                throwable_to_string(ex.get()).data());
+  }
 }
 
 static Variant call_intercept_handler(

hphp/test/quick/constructor_throw.php.expect

@@ -15,6 +15,6 @@ function handler($kind, $name) :mixed{
   try {
     new X;
   } catch (Exception $e) {
-    echo "ok\n";
+    echo "not ok\n";
   }
 }

hphp/test/quick/constructor_throw.php …test/quick/profile/constructor_throw.phphphp/test/quick/constructor_throw.php renamed to hphp/test/quick/profile/constructor_throw.php hphp/test/quick/constructor_throw.php renamed to hphp/test/quick/profile/constructor_throw.php

@@ -0,0 +1,7 @@
+
+Fatal error: Uncaught exception escaping setprofile callback: exception 'Exception' with message '' in %s/constructor_throw.php:10
+Stack trace:
+#0 %s/constructor_throw.php(6): handler()
+#1 %s/constructor_throw.php(16): X->__construct()
+#2 (): test()
+#3 %s/constructor_throw.php on line 6

hphp/test/quick/profile/constructor_throw.php.expectf

@@ -20,7 +20,6 @@ public function method() :mixed{
 
 abstract final class ProfilerStatics {
   public static $indent = 2;
-  public static $threw = false;
 }
 
 function profiler($event, $name, $info) :mixed{
@@ -30,30 +29,11 @@ function profiler($event, $name, $info) :mixed{
   printf("\n%s%s %s: %s\n", str_repeat('  ', ProfilerStatics::$indent), $event,
          $name, serialize($info));
   if ($event == 'enter') ++ProfilerStatics::$indent;
-  if ($event == 'exit' &&
-      ((!ProfilerStatics::$threw && strncmp('C::', $name, 3) == 0) ||
-       $name === 'C::method')) {
-    ProfilerStatics::$threw = true;
-    throw new Exception($name);
-  }
 }
 
-function main() :mixed{
-  try {
-    new C();
-  } catch (Exception $e) {
-    echo "\nCaught ".$e->getMessage()."\n";
-  }
-
-  try {
-    (new C())->method();
-  } catch (Exception $e) {
-    echo "\nCaught ".$e->getMessage()."\n";
-  }
-}
 <<__EntryPoint>>
-function entrypoint_setprofilethis(): void {
-
+function setprofile_this(): void {
   fb_setprofile(profiler<>);
-  main();
+  new C();
+  (new C())->method();
 }

hphp/test/quick/profile/setprofile-this.php

@@ -1,43 +1,28 @@
-exit fb_setprofile: D:1:{s:6:"return";N;}
 
-  enter main: D:1:{s:4:"args";v:0:{}}
+  exit fb_setprofile: D:1:{s:6:"return";N;}
 
-    enter C::__construct: D:1:{s:4:"args";v:0:{}}
+  enter C::__construct: D:1:{s:4:"args";v:0:{}}
 
 C constructing
 
-    exit C::__construct: D:1:{s:6:"return";N;}
+  exit C::__construct: D:1:{s:6:"return";N;}
 
-    enter Exception::getMessage: D:1:{s:4:"args";v:0:{}}
-
-    exit Exception::getMessage: D:1:{s:6:"return";s:14:"C::__construct";}
-
-Caught C::__construct
-
-    enter C::__construct: D:1:{s:4:"args";v:0:{}}
+  enter C::__construct: D:1:{s:4:"args";v:0:{}}
 
 C constructing
 
-    exit C::__construct: D:1:{s:6:"return";N;}
+  exit C::__construct: D:1:{s:6:"return";N;}
 
-    enter C::method: D:1:{s:4:"args";v:0:{}}
+  enter C::method: D:1:{s:4:"args";v:0:{}}
 
 C method
 
-      enter A::__construct: D:1:{s:4:"args";v:0:{}}
+    enter A::__construct: D:1:{s:4:"args";v:0:{}}
 
 A constructing
 
-      exit A::__construct: D:1:{s:6:"return";N;}
-
-    exit C::method: D:1:{s:6:"return";O:1:"A":0:{}}
-
-    enter Exception::getMessage: D:1:{s:4:"args";v:0:{}}
-
-    exit Exception::getMessage: D:1:{s:6:"return";s:9:"C::method";}
-
-Caught C::method
+    exit A::__construct: D:1:{s:6:"return";N;}
 
-  exit main: D:1:{s:6:"return";N;}
+  exit C::method: D:1:{s:6:"return";O:1:"A":0:{}}
 
-exit entrypoint_setprofilethis: D:1:{s:6:"return";N;}
+exit setprofile_this: D:1:{s:6:"return";N;}

hphp/test/quick/profile/setprofile-this.php.expect

@@ -14,86 +14,11 @@ function foo($f) :mixed{
   $f();
 }
 
-function signal_thrower() :mixed{
-  echo "signal throwing\n";
-  throw new Exception("Sig exception");
-}
-
-function func_to_enter() :mixed{}
-
-class DtorObj {
-  public function __construct($x) { $this->x = $x; echo "__ctor $x\n"; }
-}
-
-// During function exit
-function func_entry() :mixed{
-  $x1 = new DtorObj(1);
-  $x2 = new DtorObj(2);
-  $x3 = new DtorObj(3);
-  $x4 = new DtorObj(4);
-  $x5 = new DtorObj(5);
-
-  $x6 = new DtorObj(6);
-  $x7 = new DtorObj(7);
-  $x8 = new DtorObj(8);
-  $x9 = new DtorObj(9);
-  $xa = new DtorObj(0xa);
-
-  $xb = new DtorObj(0xb);
-  $xc = new DtorObj(0xc);
-  $xd = new DtorObj(0xd);
-  $xe = new DtorObj(0xe);
-  $xf = new DtorObj(0xf);
-
-  posix_kill(posix_getpid(), SIGUSR1);
-
-  // Surprise flags are checked on function entry.
-  func_to_enter();
-}
-
-// During backward branches
-function func_backward() :mixed{
-  $x1 = new DtorObj(1);
-  $x2 = new DtorObj(2);
-  $x3 = new DtorObj(3);
-  $x4 = new DtorObj(4);
-  $x5 = new DtorObj(5);
-
-  $x6 = new DtorObj(6);
-  $x7 = new DtorObj(7);
-  $x8 = new DtorObj(8);
-  $x9 = new DtorObj(9);
-  $xa = new DtorObj(0xa);
-
-  $xb = new DtorObj(0xb);
-  $xc = new DtorObj(0xc);
-  $xd = new DtorObj(0xd);
-  $xe = new DtorObj(0xe);
-  $xf = new DtorObj(0xf);
-
-  posix_kill(posix_getpid(), SIGUSR1);
-
-  // Surprise flags are checked on backward branch.
-  for ($i = 0; $i < 2; ++$i) {}
-}
-
-function main() :mixed{
+<<__EntryPoint>>
+function setprofile_throw(): void {
 // Test throwing on function entry
   fb_setprofile('throwing_profiler');
   try { foo('bar'); } catch (Exception $x) { echo "Caught\n"; }
   try { foo('baz'); } catch (Exception $x) { echo "Caught\n"; }
   fb_setprofile(null);
-
-// Test throwing exceptions from surprise flags things (OOM, signals,
-// and req timeout all work this way).
-
-  pcntl_signal(SIGUSR1, 'signal_thrower');
-
-  try { func_entry(); } catch (Exception $x) { echo "caught\n"; }
-  try { func_backward(); } catch (Exception $x) { echo "caught\n"; }
-}
-<<__EntryPoint>>
-function entrypoint_setprofile_throw(): void {
-
-  main();
 }

hphp/test/quick/profile/setprofile_throw.php

@@ -1,52 +1,8 @@
-Caught
-Caught
-__ctor 1
-__ctor 2
-__ctor 3
-__ctor 4
-__ctor 5
-__ctor 6
-__ctor 7
-__ctor 8
-__ctor 9
-__ctor 10
-__ctor 11
-__ctor 12
-__ctor 13
-__ctor 14
-__ctor 15
-signal throwing
 
-Warning: signal handler 'signal_thrower' threw exception 'Exception' with message 'Sig exception' in %s/setprofile_throw.php:19
+Fatal error: Uncaught exception escaping setprofile callback: exception 'Exception' with message 'yeah' in %s/setprofile_throw.php:6
 Stack trace:
-#0 (): signal_thrower()
-#1 %s/setprofile_throw.php(48): posix_kill()
-#2 %s/setprofile_throw.php(92): func_entry()
-#3 %s/setprofile_throw.php(98): main()
-#4 (): entrypoint_setprofile_throw()
-#5 {main} in %s/setprofile_throw.php on line 48
-__ctor 1
-__ctor 2
-__ctor 3
-__ctor 4
-__ctor 5
-__ctor 6
-__ctor 7
-__ctor 8
-__ctor 9
-__ctor 10
-__ctor 11
-__ctor 12
-__ctor 13
-__ctor 14
-__ctor 15
-signal throwing
-
-Warning: signal handler 'signal_thrower' threw exception 'Exception' with message 'Sig exception' in %s/setprofile_throw.php:19
-Stack trace:
-#0 (): signal_thrower()
-#1 %s/setprofile_throw.php(74): posix_kill()
-#2 %s/setprofile_throw.php(93): func_backward()
-#3 %s/setprofile_throw.php(98): main()
-#4 (): entrypoint_setprofile_throw()
-#5 {main} in %s/setprofile_throw.php on line 74
+#0 %s/setprofile_throw.php(10): throwing_profiler()
+#1 %s/setprofile_throw.php(14): bar()
+#2 %s/setprofile_throw.php(21): foo()
+#3 (): setprofile_throw()
+#4 {main} in %s/setprofile_throw.php on line 10

hphp/test/quick/profile/setprofile_throw.php.expectf

@@ -0,0 +1,10 @@
+hi exit fb_setprofile
+hi enter foo
+yep
+
+Fatal error: Uncaught exception escaping setprofile callback: exception 'Exception' with message 'yo' in %s/surprise_throw.php:7
+Stack trace:
+#0 %s/surprise_throw.php(16): asd()
+#1 %s/surprise_throw.php(23): foo()
+#2 (): entrypoint_surprise_throw()
+#3 {main} in %s/surprise_throw.php on line 16