Bug 1623226 - Use the current pending exception stack to get the file-name/line etc. for uncaught exceptions. r=jwalden
☠☠ backed out by ce00073cc7e3 ☠ ☠
authorTom Schuster <evilpies@gmail.com>
Mon, 30 Mar 2020 22:45:14 +0000
changeset 521175 798079ba88f77fba158c3a6e0c341240ff9e225c
parent 521174 83f0b1c5399c70fa96a45013c0d73a5486d3eb5a
child 521176 312da02ea6d6aef4ae98390fd2939543c3cd6b64
push id111538
push userevilpies@gmail.com
push dateMon, 30 Mar 2020 22:46:36 +0000
treeherderautoland@312da02ea6d6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalden
bugs1623226
milestone76.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1623226 - Use the current pending exception stack to get the file-name/line etc. for uncaught exceptions. r=jwalden Differential Revision: https://phabricator.services.mozilla.com/D68233
js/src/jsexn.cpp
js/src/jsfriendapi.h
js/src/shell/js.cpp
js/src/vm/SavedStacks.cpp
js/src/vm/SavedStacks.h
--- a/js/src/jsexn.cpp
+++ b/js/src/jsexn.cpp
@@ -23,16 +23,17 @@
 #include "jsapi.h"
 #include "jsfriendapi.h"
 #include "jstypes.h"
 
 #include "gc/Rooting.h"
 #include "js/CharacterEncoding.h"
 #include "js/Class.h"
 #include "js/Conversions.h"
+#include "js/SavedFrameAPI.h"
 #include "js/UniquePtr.h"
 #include "js/Value.h"
 #include "js/Warnings.h"  // JS::{,Set}WarningReporter
 #include "js/Wrapper.h"
 #include "util/Memory.h"
 #include "util/StringBuffer.h"
 #include "vm/Compartment.h"
 #include "vm/ErrorObject.h"
@@ -52,16 +53,18 @@
 #include "vm/ErrorObject-inl.h"
 #include "vm/JSContext-inl.h"
 #include "vm/JSObject-inl.h"
 #include "vm/ObjectOperations-inl.h"  // js::GetProperty
 #include "vm/SavedStacks-inl.h"
 
 using namespace js;
 
+using JS::SavedFrameSelfHosted;
+
 size_t ExtraMallocSize(JSErrorReport* report) {
   if (report->linebuf()) {
     /*
      * Count with null terminator and alignment.
      * See CopyExtraData for the details about alignment.
      */
     return (report->linebufLength() + 1) * sizeof(char16_t) + 1;
   }
@@ -460,17 +463,18 @@ static JSString* ErrorReportToString(JSC
   return FormatErrorMessage(cx, name, message);
 }
 
 ErrorReport::ErrorReport(JSContext* cx) : reportp(nullptr), exnObject(cx) {}
 
 ErrorReport::~ErrorReport() = default;
 
 bool ErrorReport::init(JSContext* cx, HandleValue exn,
-                       SniffingBehavior sniffingBehavior) {
+                       SniffingBehavior sniffingBehavior,
+                       HandleObject fallbackStack) {
   MOZ_ASSERT(!cx->isExceptionPending());
   MOZ_ASSERT(!reportp);
 
   if (exn.isObject()) {
     // Because ToString below could error and an exception object could become
     // unrooted, we must root our exception object, if any.
     exnObject = &exn.toObject();
     reportp = ErrorFromException(cx, exnObject);
@@ -599,53 +603,72 @@ bool ErrorReport::init(JSContext* cx, Ha
   if (!reportp) {
     // This is basically an inlined version of
     //
     //   JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr,
     //                            JSMSG_UNCAUGHT_EXCEPTION, utf8Message);
     //
     // but without the reporting bits.  Instead it just puts all
     // the stuff we care about in our ownedReport and message_.
-    if (!populateUncaughtExceptionReportUTF8(cx, utf8Message)) {
+    if (!populateUncaughtExceptionReportUTF8(cx, fallbackStack, utf8Message)) {
       // Just give up.  We're out of memory or something; not much we can
       // do here.
       return false;
     }
   } else {
     toStringResult_ = JS::ConstUTF8CharsZ(utf8Message, strlen(utf8Message));
   }
 
   return true;
 }
 
-bool ErrorReport::populateUncaughtExceptionReportUTF8(JSContext* cx, ...) {
+bool ErrorReport::populateUncaughtExceptionReportUTF8(
+    JSContext* cx, HandleObject fallbackStack, ...) {
   va_list ap;
-  va_start(ap, cx);
-  bool ok = populateUncaughtExceptionReportUTF8VA(cx, ap);
+  va_start(ap, fallbackStack);
+  bool ok = populateUncaughtExceptionReportUTF8VA(cx, fallbackStack, ap);
   va_end(ap);
   return ok;
 }
 
-bool ErrorReport::populateUncaughtExceptionReportUTF8VA(JSContext* cx,
-                                                        va_list ap) {
+bool ErrorReport::populateUncaughtExceptionReportUTF8VA(
+    JSContext* cx, HandleObject fallbackStack, va_list ap) {
   new (&ownedReport) JSErrorReport();
   ownedReport.isWarning_ = false;
   ownedReport.errorNumber = JSMSG_UNCAUGHT_EXCEPTION;
-  // XXXbz this assumes the stack we have right now is still
-  // related to our exception object.  It would be better if we
-  // could accept a passed-in stack of some sort instead.
-  NonBuiltinFrameIter iter(cx, cx->realm()->principals());
-  if (!iter.done()) {
-    ownedReport.filename = iter.filename();
-    uint32_t column;
-    ownedReport.sourceId =
-        iter.hasScript() ? iter.script()->scriptSource()->id() : 0;
-    ownedReport.lineno = iter.computeLine(&column);
-    ownedReport.column = FixupColumnForDisplay(column);
-    ownedReport.isMuted = iter.mutedErrors();
+
+  bool skippedAsync;
+  RootedSavedFrame frame(
+      cx, UnwrapSavedFrame(cx, cx->realm()->principals(), fallbackStack,
+                           SavedFrameSelfHosted::Exclude, skippedAsync));
+  if (frame) {
+    filename = StringToNewUTF8CharsZ(cx, *frame->getSource());
+    if (!filename) {
+      return false;
+    }
+
+    // |ownedReport.filename| inherits the lifetime of |ErrorReport::filename|.
+    ownedReport.filename = filename.get();
+    ownedReport.sourceId = frame->getSourceId();
+    ownedReport.lineno = frame->getLine();
+    ownedReport.column = FixupColumnForDisplay(frame->getColumn());
+    ownedReport.isMuted = frame->getMutedErrors();
+  } else {
+    // XXXbz this assumes the stack we have right now is still
+    // related to our exception object.
+    NonBuiltinFrameIter iter(cx, cx->realm()->principals());
+    if (!iter.done()) {
+      ownedReport.filename = iter.filename();
+      uint32_t column;
+      ownedReport.sourceId =
+          iter.hasScript() ? iter.script()->scriptSource()->id() : 0;
+      ownedReport.lineno = iter.computeLine(&column);
+      ownedReport.column = FixupColumnForDisplay(column);
+      ownedReport.isMuted = iter.mutedErrors();
+    }
   }
 
   if (!ExpandErrorArgumentsVA(cx, GetErrorMessage, nullptr,
                               JSMSG_UNCAUGHT_EXCEPTION, ArgumentsAreUTF8,
                               &ownedReport, ap)) {
     return false;
   }
 
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -1307,47 +1307,55 @@ struct MOZ_STACK_CLASS JS_FRIEND_API Err
    * observable side effects. (The Error object's JSErrorReport is reused, if
    * it has one.)
    *
    * Otherwise various attempts are made to derive JSErrorReport information
    * from |exn| and from the current execution state.  This process is
    * *definitely* inconsistent with any standard, and particulars of the
    * behavior implemented here generally shouldn't be relied upon.
    *
+   * To fill in information such as file-name or line number the (optional)
+   * stack for the pending exception can be used. For this first SavedFrame
+   * is used.
+   *
    * If the value of |sniffingBehavior| is |WithSideEffects|, some of these
    * attempts *may* invoke user-configurable behavior when |exn| is an object:
    * converting |exn| to a string, detecting and getting properties on |exn|,
    * accessing |exn|'s prototype chain, and others are possible.  Users *must*
    * tolerate |ErrorReport::init| potentially having arbitrary effects.  Any
    * exceptions thrown by these operations will be caught and silently
    * ignored, and "default" values will be substituted into the JSErrorReport.
    *
    * But if the value of |sniffingBehavior| is |NoSideEffects|, these attempts
    * *will not* invoke any observable side effects.  The JSErrorReport will
    * simply contain fewer, less precise details.
    *
    * Unlike some functions involved in error handling, this function adheres
    * to the usual JSAPI return value error behavior.
    */
   bool init(JSContext* cx, JS::HandleValue exn,
-            SniffingBehavior sniffingBehavior);
+            SniffingBehavior sniffingBehavior,
+            JS::HandleObject fallbackStack = nullptr);
 
   JSErrorReport* report() { return reportp; }
 
   const JS::ConstUTF8CharsZ toStringResult() { return toStringResult_; }
 
  private:
   // More or less an equivalent of JS_ReportErrorNumber/js::ReportErrorNumberVA
   // but fills in an ErrorReport instead of reporting it.  Uses varargs to
   // make it simpler to call js::ExpandErrorArgumentsVA.
   //
   // Returns false if we fail to actually populate the ErrorReport
   // for some reason (probably out of memory).
-  bool populateUncaughtExceptionReportUTF8(JSContext* cx, ...);
-  bool populateUncaughtExceptionReportUTF8VA(JSContext* cx, va_list ap);
+  bool populateUncaughtExceptionReportUTF8(JSContext* cx,
+                                           JS::HandleObject fallbackStack, ...);
+  bool populateUncaughtExceptionReportUTF8VA(JSContext* cx,
+                                             JS::HandleObject fallbackStack,
+                                             va_list ap);
 
   // Reports exceptions from add-on scopes to telemetry.
   void ReportAddonExceptionToTelemetry(JSContext* cx);
 
   // We may have a provided JSErrorReport, so need a way to represent that.
   JSErrorReport* reportp;
 
   // Or we may need to synthesize a JSErrorReport one of our own.
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -9559,17 +9559,17 @@ js::shell::AutoReportException::~AutoRep
   RootedValue exn(cx);
   (void)JS_GetPendingException(cx, &exn);
   RootedObject stack(cx, GetPendingExceptionStack(cx));
 
   JS_ClearPendingException(cx);
 
   ShellContext* sc = GetShellContext(cx);
   js::ErrorReport report(cx);
-  if (!report.init(cx, exn, js::ErrorReport::WithSideEffects)) {
+  if (!report.init(cx, exn, js::ErrorReport::WithSideEffects, stack)) {
     fprintf(stderr, "out of memory initializing ErrorReport\n");
     fflush(stderr);
     JS_ClearPendingException(cx);
     return;
   }
 
   MOZ_ASSERT(!report.report()->isWarning());
 
--- a/js/src/vm/SavedStacks.cpp
+++ b/js/src/vm/SavedStacks.cpp
@@ -721,35 +721,34 @@ static MOZ_MUST_USE bool SavedFrame_chec
 //   - Rooted<SavedFrame*> frame (will be non-null)
 #define THIS_SAVEDFRAME(cx, argc, vp, fnName, args, frame) \
   CallArgs args = CallArgsFromVp(argc, vp);                \
   RootedObject frame(cx);                                  \
   if (!SavedFrame_checkThis(cx, args, fnName, &frame)) return false;
 
 } /* namespace js */
 
-namespace JS {
-
-static inline js::SavedFrame* UnwrapSavedFrame(JSContext* cx,
-                                               JSPrincipals* principals,
-                                               HandleObject obj,
-                                               SavedFrameSelfHosted selfHosted,
-                                               bool& skippedAsync) {
+js::SavedFrame* js::UnwrapSavedFrame(JSContext* cx, JSPrincipals* principals,
+                                     HandleObject obj,
+                                     JS::SavedFrameSelfHosted selfHosted,
+                                     bool& skippedAsync) {
   if (!obj) {
     return nullptr;
   }
 
-  js::RootedSavedFrame frame(cx, obj->maybeUnwrapAs<js::SavedFrame>());
+  RootedSavedFrame frame(cx, obj->maybeUnwrapAs<SavedFrame>());
   if (!frame) {
     return nullptr;
   }
 
   return GetFirstSubsumedFrame(cx, principals, frame, selfHosted, skippedAsync);
 }
 
+namespace JS {
+
 JS_PUBLIC_API SavedFrameResult GetSavedFrameSource(
     JSContext* cx, JSPrincipals* principals, HandleObject savedFrame,
     MutableHandleString sourcep,
     SavedFrameSelfHosted selfHosted /* = SavedFrameSelfHosted::Include */) {
   js::AssertHeapIsIdle();
   CHECK_THREAD(cx);
   MOZ_RELEASE_ASSERT(cx->realm());
 
--- a/js/src/vm/SavedStacks.h
+++ b/js/src/vm/SavedStacks.h
@@ -12,16 +12,20 @@
 #include "mozilla/Maybe.h"
 
 #include "js/HashTable.h"
 #include "js/Wrapper.h"
 #include "vm/JSContext.h"
 #include "vm/SavedFrame.h"
 #include "vm/Stack.h"
 
+namespace JS {
+enum class SavedFrameSelfHosted;
+}
+
 namespace js {
 
 // # Saved Stacks
 //
 // The `SavedStacks` class provides a compact way to capture and save JS stacks
 // as `SavedFrame` `JSObject` subclasses. A single `SavedFrame` object
 // represents one frame that was on the stack, and has a strong reference to its
 // parent `SavedFrame` (the next youngest frame). This reference is null when
@@ -326,11 +330,16 @@ struct MutableWrappedPtrOperations<Saved
   }
 };
 
 JS::UniqueChars BuildUTF8StackString(JSContext* cx, JSPrincipals* principals,
                                      HandleObject stack);
 
 uint32_t FixupColumnForDisplay(uint32_t column);
 
+js::SavedFrame* UnwrapSavedFrame(JSContext* cx, JSPrincipals* principals,
+                                 HandleObject obj,
+                                 JS::SavedFrameSelfHosted selfHosted,
+                                 bool& skippedAsync);
+
 } /* namespace js */
 
 #endif /* vm_SavedStacks_h */