Skip to content

Commit 3724a12

Browse files
mmarchiniCommit Bot
mmarchini
authored and
Commit Bot
committed
Reland "[error] extend error stack w/ function parameters"
This is a reland of 97628ee. Original change's description: > [error] extend error stack w/ function parameters > > Extend FrameArray to hold weak references to parameters forfunctions in > the call stack. The goal here is to provide more metadata for postmortem > tools (such as llnode), especially in cases of rethrowing (this will be > particularly useful when using postmortem with promises on Node.js). > > Besides postmortem, these changes allow us to print a more detailed > stack trace for errors with parameters types (or even values), which can > be useful since JavaScript functions can receive any number of > parameters of any type, and having a function behave differently > according to the number of parameters received as well as their types is > a common pattern on JS libraries and frameworks. > > R=<U+200B>[email protected], [email protected] > > Change-Id: Idf0984d0dbac16041f11d738d4b1c095a8eecd61 > Reviewed-on: https://chromium-review.googlesource.com/c/1289489 > Commit-Queue: Yang Guo <[email protected]> > Reviewed-by: Yang Guo <[email protected]> > Cr-Commit-Position: refs/heads/master@{#58468} [email protected], [email protected], [email protected] Change-Id: I53d90bb862d9c5e9541116b375fa4de70e3e76dd Reviewed-on: https://chromium-review.googlesource.com/c/1405568 Commit-Queue: Yang Guo <[email protected]> Reviewed-by: Jakob Kummerow <[email protected]> Reviewed-by: Yang Guo <[email protected]> Cr-Commit-Position: refs/heads/master@{#59458}
1 parent c9a9d82 commit 3724a12

7 files changed

Lines changed: 200 additions & 31 deletions

File tree

‎src/flag-definitions.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,6 +1072,9 @@ DEFINE_INT(fuzzer_random_seed, 0,
10721072
DEFINE_BOOL(trace_rail, false, "trace RAIL mode")
10731073
DEFINE_BOOL(print_all_exceptions, false,
10741074
"print exception object and stack trace on each thrown exception")
1075+
DEFINE_BOOL(
1076+
detailed_error_stack_trace, false,
1077+
"includes arguments for each function call in the error stack frames array")
10751078

10761079
// runtime.cc
10771080
DEFINE_BOOL(runtime_call_stats, false, "report runtime call counts and times")

‎src/frames.cc‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,9 +1079,10 @@ void JavaScriptFrame::Summarize(std::vector<FrameSummary>* functions) const {
10791079
Code code = LookupCode();
10801080
int offset = static_cast<int>(pc() - code->InstructionStart());
10811081
AbstractCode abstract_code = AbstractCode::cast(code);
1082-
FrameSummary::JavaScriptFrameSummary summary(isolate(), receiver(),
1083-
function(), abstract_code,
1084-
offset, IsConstructor());
1082+
Handle<FixedArray> params = GetParameters();
1083+
FrameSummary::JavaScriptFrameSummary summary(
1084+
isolate(), receiver(), function(), abstract_code, offset, IsConstructor(),
1085+
*params);
10851086
functions->push_back(summary);
10861087
}
10871088

@@ -1241,6 +1242,20 @@ int JavaScriptFrame::ComputeParametersCount() const {
12411242
return function()->shared()->internal_formal_parameter_count();
12421243
}
12431244

1245+
Handle<FixedArray> JavaScriptFrame::GetParameters() const {
1246+
if (V8_LIKELY(!FLAG_detailed_error_stack_trace)) {
1247+
return isolate()->factory()->empty_fixed_array();
1248+
}
1249+
int param_count = ComputeParametersCount();
1250+
Handle<FixedArray> parameters =
1251+
isolate()->factory()->NewFixedArray(param_count);
1252+
for (int i = 0; i < param_count; i++) {
1253+
parameters->set(i, GetParameter(i));
1254+
}
1255+
1256+
return parameters;
1257+
}
1258+
12441259
int JavaScriptBuiltinContinuationFrame::ComputeParametersCount() const {
12451260
// Assert that the first allocatable register is also the argument count
12461261
// register.
@@ -1277,13 +1292,15 @@ void JavaScriptBuiltinContinuationWithCatchFrame::SetException(
12771292

12781293
FrameSummary::JavaScriptFrameSummary::JavaScriptFrameSummary(
12791294
Isolate* isolate, Object receiver, JSFunction function,
1280-
AbstractCode abstract_code, int code_offset, bool is_constructor)
1295+
AbstractCode abstract_code, int code_offset, bool is_constructor,
1296+
FixedArray parameters)
12811297
: FrameSummaryBase(isolate, FrameSummary::JAVA_SCRIPT),
12821298
receiver_(receiver, isolate),
12831299
function_(function, isolate),
12841300
abstract_code_(abstract_code, isolate),
12851301
code_offset_(code_offset),
1286-
is_constructor_(is_constructor) {
1302+
is_constructor_(is_constructor),
1303+
parameters_(parameters, isolate) {
12871304
DCHECK(abstract_code->IsBytecodeArray() ||
12881305
Code::cast(abstract_code)->kind() != Code::OPTIMIZED_FUNCTION);
12891306
}
@@ -1529,9 +1546,10 @@ void OptimizedFrame::Summarize(std::vector<FrameSummary>* frames) const {
15291546
}
15301547

15311548
// Append full summary of the encountered JS frame.
1532-
FrameSummary::JavaScriptFrameSummary summary(isolate(), *receiver,
1533-
*function, *abstract_code,
1534-
code_offset, is_constructor);
1549+
Handle<FixedArray> params = GetParameters();
1550+
FrameSummary::JavaScriptFrameSummary summary(
1551+
isolate(), *receiver, *function, *abstract_code, code_offset,
1552+
is_constructor, *params);
15351553
frames->push_back(summary);
15361554
is_constructor = false;
15371555
} else if (it->kind() == TranslatedFrame::kConstructStub) {
@@ -1742,9 +1760,10 @@ void InterpretedFrame::WriteInterpreterRegister(int register_index,
17421760
void InterpretedFrame::Summarize(std::vector<FrameSummary>* functions) const {
17431761
DCHECK(functions->empty());
17441762
AbstractCode abstract_code = AbstractCode::cast(GetBytecodeArray());
1763+
Handle<FixedArray> params = GetParameters();
17451764
FrameSummary::JavaScriptFrameSummary summary(
17461765
isolate(), receiver(), function(), abstract_code, GetBytecodeOffset(),
1747-
IsConstructor());
1766+
IsConstructor(), *params);
17481767
functions->push_back(summary);
17491768
}
17501769

‎src/frames.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,7 @@ class BuiltinExitFrame : public ExitFrame {
446446
inline Object new_target_slot_object() const;
447447

448448
friend class StackFrameIteratorBase;
449+
friend class FrameArrayBuilder;
449450
};
450451

451452
class StandardFrame;
@@ -480,13 +481,15 @@ class FrameSummary {
480481
public:
481482
JavaScriptFrameSummary(Isolate* isolate, Object receiver,
482483
JSFunction function, AbstractCode abstract_code,
483-
int code_offset, bool is_constructor);
484+
int code_offset, bool is_constructor,
485+
FixedArray parameters);
484486

485487
Handle<Object> receiver() const { return receiver_; }
486488
Handle<JSFunction> function() const { return function_; }
487489
Handle<AbstractCode> abstract_code() const { return abstract_code_; }
488490
int code_offset() const { return code_offset_; }
489491
bool is_constructor() const { return is_constructor_; }
492+
Handle<FixedArray> parameters() const { return parameters_; }
490493
bool is_subject_to_debugging() const;
491494
int SourcePosition() const;
492495
int SourceStatementPosition() const;
@@ -500,6 +503,7 @@ class FrameSummary {
500503
Handle<AbstractCode> abstract_code_;
501504
int code_offset_;
502505
bool is_constructor_;
506+
Handle<FixedArray> parameters_;
503507
};
504508

505509
class WasmFrameSummary : public FrameSummaryBase {
@@ -694,6 +698,7 @@ class JavaScriptFrame : public StandardFrame {
694698
inline Address GetParameterSlot(int index) const;
695699
Object GetParameter(int index) const override;
696700
int ComputeParametersCount() const override;
701+
Handle<FixedArray> GetParameters() const;
697702

698703
// Debugger access.
699704
void SetParameterValue(int index, Object value) const;

‎src/isolate.cc‎

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -529,8 +529,6 @@ StackTraceFailureMessage::StackTraceFailureMessage(Isolate* isolate, void* ptr1,
529529
}
530530
}
531531

532-
namespace {
533-
534532
class FrameArrayBuilder {
535533
public:
536534
enum FrameFilterMode { ALL, CURRENT_SECURITY_CONTEXT };
@@ -573,8 +571,19 @@ class FrameArrayBuilder {
573571
// The stored bytecode offset is relative to a different base than what
574572
// is used in the source position table, hence the subtraction.
575573
offset -= BytecodeArray::kHeaderSize - kHeapObjectTag;
574+
575+
Handle<FixedArray> parameters = isolate_->factory()->empty_fixed_array();
576+
if (V8_UNLIKELY(FLAG_detailed_error_stack_trace)) {
577+
int param_count = function->shared()->internal_formal_parameter_count();
578+
parameters = isolate_->factory()->NewFixedArray(param_count);
579+
for (int i = 0; i < param_count; i++) {
580+
parameters->set(i,
581+
generator_object->parameters_and_registers()->get(i));
582+
}
583+
}
584+
576585
elements_ = FrameArray::AppendJSFrame(elements_, receiver, function, code,
577-
offset, flags);
586+
offset, flags, parameters);
578587
}
579588

580589
void AppendPromiseAllFrame(Handle<Context> context, int offset) {
@@ -587,8 +596,12 @@ class FrameArrayBuilder {
587596

588597
Handle<Object> receiver(native_context->promise_function(), isolate_);
589598
Handle<AbstractCode> code(AbstractCode::cast(function->code()), isolate_);
599+
600+
// TODO(mmarchini) save Promises list from Promise.all()
601+
Handle<FixedArray> parameters = isolate_->factory()->empty_fixed_array();
602+
590603
elements_ = FrameArray::AppendJSFrame(elements_, receiver, function, code,
591-
offset, flags);
604+
offset, flags, parameters);
592605
}
593606

594607
void AppendJavaScriptFrame(
@@ -606,9 +619,13 @@ class FrameArrayBuilder {
606619
if (IsStrictFrame(function)) flags |= FrameArray::kIsStrict;
607620
if (is_constructor) flags |= FrameArray::kIsConstructor;
608621

622+
Handle<FixedArray> parameters = isolate_->factory()->empty_fixed_array();
623+
if (V8_UNLIKELY(FLAG_detailed_error_stack_trace))
624+
parameters = summary.parameters();
625+
609626
elements_ = FrameArray::AppendJSFrame(
610627
elements_, TheHoleToUndefined(isolate_, summary.receiver()), function,
611-
abstract_code, offset, flags);
628+
abstract_code, offset, flags, parameters);
612629
}
613630

614631
void AppendWasmCompiledFrame(
@@ -655,9 +672,18 @@ class FrameArrayBuilder {
655672
if (IsStrictFrame(function)) flags |= FrameArray::kIsStrict;
656673
if (exit_frame->IsConstructor()) flags |= FrameArray::kIsConstructor;
657674

675+
Handle<FixedArray> parameters = isolate_->factory()->empty_fixed_array();
676+
if (V8_UNLIKELY(FLAG_detailed_error_stack_trace)) {
677+
int param_count = exit_frame->ComputeParametersCount();
678+
parameters = isolate_->factory()->NewFixedArray(param_count);
679+
for (int i = 0; i < param_count; i++) {
680+
parameters->set(i, exit_frame->GetParameter(i));
681+
}
682+
}
683+
658684
elements_ = FrameArray::AppendJSFrame(elements_, receiver, function,
659685
Handle<AbstractCode>::cast(code),
660-
offset, flags);
686+
offset, flags, parameters);
661687
}
662688

663689
bool full() { return elements_->FrameCount() >= limit_; }
@@ -858,8 +884,6 @@ void CaptureAsyncStackTrace(Isolate* isolate, Handle<JSPromise> promise,
858884
}
859885
}
860886

861-
} // namespace
862-
863887
Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
864888
FrameSkipMode mode,
865889
Handle<Object> caller) {

‎src/objects.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3942,7 +3942,8 @@ Handle<FrameArray> FrameArray::AppendJSFrame(Handle<FrameArray> in,
39423942
Handle<Object> receiver,
39433943
Handle<JSFunction> function,
39443944
Handle<AbstractCode> code,
3945-
int offset, int flags) {
3945+
int offset, int flags,
3946+
Handle<FixedArray> parameters) {
39463947
const int frame_count = in->FrameCount();
39473948
const int new_length = LengthFor(frame_count + 1);
39483949
Handle<FrameArray> array =
@@ -3952,6 +3953,7 @@ Handle<FrameArray> FrameArray::AppendJSFrame(Handle<FrameArray> in,
39523953
array->SetCode(frame_count, *code);
39533954
array->SetOffset(frame_count, Smi::FromInt(offset));
39543955
array->SetFlags(frame_count, Smi::FromInt(flags));
3956+
array->SetParameters(frame_count, *parameters);
39553957
array->set(kFrameCountIndex, Smi::FromInt(frame_count + 1));
39563958
return array;
39573959
}

‎src/objects/frame-array.h‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ class Handle;
2525
V(Function, JSFunction) \
2626
V(Code, AbstractCode) \
2727
V(Offset, Smi) \
28-
V(Flags, Smi)
28+
V(Flags, Smi) \
29+
V(Parameters, FixedArray)
2930

3031
// Container object for data collected during simple stack trace captures.
3132
class FrameArray : public FixedArray {
@@ -60,7 +61,8 @@ class FrameArray : public FixedArray {
6061
Handle<Object> receiver,
6162
Handle<JSFunction> function,
6263
Handle<AbstractCode> code, int offset,
63-
int flags);
64+
int flags,
65+
Handle<FixedArray> parameters);
6466
static Handle<FrameArray> AppendWasmFrame(
6567
Handle<FrameArray> in, Handle<WasmInstanceObject> wasm_instance,
6668
int wasm_function_index, wasm::WasmCode* code, int offset, int flags);
@@ -87,7 +89,9 @@ class FrameArray : public FixedArray {
8789

8890
static const int kFlagsOffset = 4;
8991

90-
static const int kElementsPerFrame = 5;
92+
static const int kParametersOffset = 5;
93+
94+
static const int kElementsPerFrame = 6;
9195

9296
// Array layout indices.
9397

0 commit comments

Comments
 (0)