From d220daac4bdd8ba98c87dd55f686586fd27548be Mon Sep 17 00:00:00 2001 From: Harry Altman Date: Wed, 12 Jul 2023 16:38:20 -0400 Subject: [PATCH 1/4] Have debugger skip anything called from a generated source while stepping --- .../debugger/lib/controller/sagas/index.js | 7 ++--- .../lib/controller/selectors/index.js | 17 +++++++++++ .../lib/stacktrace/selectors/index.js | 30 ++++++++++++++----- 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/packages/debugger/lib/controller/sagas/index.js b/packages/debugger/lib/controller/sagas/index.js index 522c3f6600e..e5926bb094b 100644 --- a/packages/debugger/lib/controller/sagas/index.js +++ b/packages/debugger/lib/controller/sagas/index.js @@ -80,7 +80,7 @@ function* stepNext() { const starting = yield select(controller.current.location); const allowInternal = yield select(controller.stepIntoInternalSources); - let upcoming, finished; + let upcoming, finished, isUpcomingInternal; do { // advance at least once step @@ -88,6 +88,7 @@ function* stepNext() { // and check the next source range upcoming = yield select(controller.current.location); + isUpcomingInternal = yield select(controller.current.isAnyFrameInternal); finished = yield select(controller.current.trace.finished); @@ -97,9 +98,7 @@ function* stepNext() { (!upcoming || //don't stop on an internal source unless allowInternal is on or //we started in an internal source - (!allowInternal && - upcoming.source.internal && - !starting.source.internal) || + (!allowInternal && isUpcomingInternal && !starting.source.internal) || upcoming.sourceRange.length === 0 || upcoming.source.id === undefined || (upcoming.node && isDeliberatelySkippedNodeType(upcoming.node)) || diff --git a/packages/debugger/lib/controller/selectors/index.js b/packages/debugger/lib/controller/selectors/index.js index a8f8552311d..0c7f23cd004 100644 --- a/packages/debugger/lib/controller/selectors/index.js +++ b/packages/debugger/lib/controller/selectors/index.js @@ -7,6 +7,7 @@ import * as Codec from "@truffle/codec"; import evm from "lib/evm/selectors"; import sourcemapping from "lib/sourcemapping/selectors"; +import stacktrace from "lib/stacktrace/selectors"; import data from "lib/data/selectors"; import trace from "lib/trace/selectors"; @@ -137,6 +138,22 @@ const controller = createSelectorTree({ ) }, + /** + * controller.current.callstack + */ + callstack: createLeaf([stacktrace.current.callstack.preupdated], identity), + + /** + * controller.current.isAnyFrameInternal + * + * This selector checks whether there are any internal (unmapped or + * generated) stackframes on the callstack. We should regard ourselves + * as still inside a generated source until there are none. + */ + isAnyFrameInternal: createLeaf(["./callstack"], callstack => + callstack.some(frame => frame.sourceIsInternal) + ), + /** * controller.current.trace */ diff --git a/packages/debugger/lib/stacktrace/selectors/index.js b/packages/debugger/lib/stacktrace/selectors/index.js index b7746af6faf..a33e41841fc 100644 --- a/packages/debugger/lib/stacktrace/selectors/index.js +++ b/packages/debugger/lib/stacktrace/selectors/index.js @@ -156,9 +156,27 @@ let stacktrace = createSelectorTree({ */ current: { /** - * stacktrace.current.callstack + * stacktrace.current.callstack (namespace) */ - callstack: createLeaf(["/state"], state => state.proc.callstack), + callstack: { + /** + * stacktrace.current.callstack (selector) + */ + _: createLeaf(["/state"], state => state.proc.callstack), + + /** + * stacktrace.current.callstack.preupdated + */ + preupdated: createLeaf( + ["./_", "/current/returnCounter"], + (callstack, returnCounter) => + popNWhere( + callstack, + returnCounter, + frame => frame.type === "external" + ) + ) + }, /** * stacktrace.current.returnCounter @@ -398,18 +416,14 @@ let stacktrace = createSelectorTree({ */ report: createLeaf( [ - "./callstack", + "./callstack/preupdated", "./returnCounter", "./lastPosition", "/current/strippedLocation" ], (callstack, returnCounter, lastPosition, currentLocation) => generateReport( - popNWhere( - callstack, - returnCounter, - frame => frame.type === "external" - ), + callstack, currentLocation || lastPosition, null, undefined From 92a3844458947dfb95e4c330a32c20107d77746a Mon Sep 17 00:00:00 2001 From: Harry Altman Date: Wed, 12 Jul 2023 19:38:04 -0400 Subject: [PATCH 2/4] Restrict this check to generated sources only --- packages/debugger/lib/controller/sagas/index.js | 8 +++++--- packages/debugger/lib/controller/selectors/index.js | 12 +++++++----- packages/debugger/lib/stacktrace/actions/index.js | 11 +++++++++-- packages/debugger/lib/stacktrace/reducers.js | 11 +++++++++-- packages/debugger/lib/stacktrace/sagas/index.js | 6 +++++- packages/debugger/lib/stacktrace/selectors/index.js | 9 +++++++++ 6 files changed, 44 insertions(+), 13 deletions(-) diff --git a/packages/debugger/lib/controller/sagas/index.js b/packages/debugger/lib/controller/sagas/index.js index e5926bb094b..d446ec3be04 100644 --- a/packages/debugger/lib/controller/sagas/index.js +++ b/packages/debugger/lib/controller/sagas/index.js @@ -80,7 +80,7 @@ function* stepNext() { const starting = yield select(controller.current.location); const allowInternal = yield select(controller.stepIntoInternalSources); - let upcoming, finished, isUpcomingInternal; + let upcoming, finished, isUpcomingGenerated; do { // advance at least once step @@ -88,7 +88,7 @@ function* stepNext() { // and check the next source range upcoming = yield select(controller.current.location); - isUpcomingInternal = yield select(controller.current.isAnyFrameInternal); + isUpcomingGenerated = yield select(controller.current.isAnyFrameGenerated); finished = yield select(controller.current.trace.finished); @@ -98,7 +98,9 @@ function* stepNext() { (!upcoming || //don't stop on an internal source unless allowInternal is on or //we started in an internal source - (!allowInternal && isUpcomingInternal && !starting.source.internal) || + (!allowInternal && + (upcoming.source.internal || isUpcomingGenerated) && + !starting.source.internal) || upcoming.sourceRange.length === 0 || upcoming.source.id === undefined || (upcoming.node && isDeliberatelySkippedNodeType(upcoming.node)) || diff --git a/packages/debugger/lib/controller/selectors/index.js b/packages/debugger/lib/controller/selectors/index.js index 0c7f23cd004..7a591673fc6 100644 --- a/packages/debugger/lib/controller/selectors/index.js +++ b/packages/debugger/lib/controller/selectors/index.js @@ -144,14 +144,16 @@ const controller = createSelectorTree({ callstack: createLeaf([stacktrace.current.callstack.preupdated], identity), /** - * controller.current.isAnyFrameInternal + * controller.current.isAnyFrameGenerated * - * This selector checks whether there are any internal (unmapped or - * generated) stackframes on the callstack. We should regard ourselves + * This selector checks whether there are any generated sources + * stackframes on the callstack. We should regard ourselves * as still inside a generated source until there are none. + * (We only consider generated sources here, not other sorts of internal + * sources, to prevent potential problems.) */ - isAnyFrameInternal: createLeaf(["./callstack"], callstack => - callstack.some(frame => frame.sourceIsInternal) + isAnyFrameGenerated: createLeaf(["./callstack"], callstack => + callstack.some(frame => frame.sourceIsGenerated) ), /** diff --git a/packages/debugger/lib/stacktrace/actions/index.js b/packages/debugger/lib/stacktrace/actions/index.js index 4e88bff61fb..d9dd0c47f3c 100644 --- a/packages/debugger/lib/stacktrace/actions/index.js +++ b/packages/debugger/lib/stacktrace/actions/index.js @@ -1,11 +1,18 @@ export const JUMP_IN = "STACKTRACE_JUMP_IN"; -export function jumpIn(location, functionNode, contractNode, sourceIsInternal) { +export function jumpIn( + location, + functionNode, + contractNode, + sourceIsInternal, + sourceIsGenerated +) { return { type: JUMP_IN, location, functionNode, contractNode, - sourceIsInternal + sourceIsInternal, + sourceIsGenerated }; } diff --git a/packages/debugger/lib/stacktrace/reducers.js b/packages/debugger/lib/stacktrace/reducers.js index 540f1b50c0c..8db98c85fd0 100644 --- a/packages/debugger/lib/stacktrace/reducers.js +++ b/packages/debugger/lib/stacktrace/reducers.js @@ -10,7 +10,13 @@ function callstack(state = [], action) { let newFrame; switch (action.type) { case actions.JUMP_IN: - const { location, functionNode, contractNode, sourceIsInternal } = action; + const { + location, + functionNode, + contractNode, + sourceIsInternal, + sourceIsGenerated + } = action; newFrame = { type: "internal", calledFromLocation: location, @@ -27,7 +33,8 @@ function callstack(state = [], action) { ? contractNode.name : undefined, combineWithNextInternal: false, - sourceIsInternal + sourceIsInternal, + sourceIsGenerated //note we don't currently account for getters because currently //we can't; fallback, receive, constructors, & modifiers also remain //unaccounted for at present diff --git a/packages/debugger/lib/stacktrace/sagas/index.js b/packages/debugger/lib/stacktrace/sagas/index.js index 6a37752f5c9..6a866b650a0 100644 --- a/packages/debugger/lib/stacktrace/sagas/index.js +++ b/packages/debugger/lib/stacktrace/sagas/index.js @@ -63,12 +63,16 @@ function* stacktraceSaga() { const nextLocation = yield select(stacktrace.next.location); const nextParent = yield select(stacktrace.next.contractNode); const nextSourceIsInternal = yield select(stacktrace.next.sourceIsInternal); + const nextSourceIsGenerated = yield select( + stacktrace.next.sourceIsGenerated + ); yield put( actions.jumpIn( currentLocation, nextLocation.node, nextParent, - nextSourceIsInternal + nextSourceIsInternal, + nextSourceIsGenerated ) ); positionUpdated = true; diff --git a/packages/debugger/lib/stacktrace/selectors/index.js b/packages/debugger/lib/stacktrace/selectors/index.js index a33e41841fc..dd1ad2c9d91 100644 --- a/packages/debugger/lib/stacktrace/selectors/index.js +++ b/packages/debugger/lib/stacktrace/selectors/index.js @@ -111,6 +111,15 @@ function createMultistepSelectors(stepSelector) { source => source.id === undefined || source.internal ), + /** + * .sourceIsGenerated + * only specifically generated sources, not unmapped code or anything! + */ + sourceIsGenerated: createLeaf( + ["./location/source"], + source => source.internal + ), + /** * .strippedLocation */ From 567008a9f27be6d26e97ec859384b08edde0ed88 Mon Sep 17 00:00:00 2001 From: Harry Altman Date: Wed, 12 Jul 2023 20:47:10 -0400 Subject: [PATCH 3/4] Apply same fix to starting point --- packages/debugger/lib/controller/sagas/index.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/debugger/lib/controller/sagas/index.js b/packages/debugger/lib/controller/sagas/index.js index d446ec3be04..2e7e80bb12c 100644 --- a/packages/debugger/lib/controller/sagas/index.js +++ b/packages/debugger/lib/controller/sagas/index.js @@ -78,6 +78,9 @@ function* advance(action) { */ function* stepNext() { const starting = yield select(controller.current.location); + const isStartingGenerated = yield select( + controller.current.isAnyFrameGenerated + ); const allowInternal = yield select(controller.stepIntoInternalSources); let upcoming, finished, isUpcomingGenerated; @@ -100,7 +103,7 @@ function* stepNext() { //we started in an internal source (!allowInternal && (upcoming.source.internal || isUpcomingGenerated) && - !starting.source.internal) || + !(starting.source.internal || isStartingGenerated)) || upcoming.sourceRange.length === 0 || upcoming.source.id === undefined || (upcoming.node && isDeliberatelySkippedNodeType(upcoming.node)) || From 7dae98a283e97a1f841ffcb741e403b3c269b967 Mon Sep 17 00:00:00 2001 From: Harry Altman Date: Thu, 13 Jul 2023 13:51:05 -0400 Subject: [PATCH 4/4] Add comment explaining "preupdated" name --- packages/debugger/lib/stacktrace/selectors/index.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/debugger/lib/stacktrace/selectors/index.js b/packages/debugger/lib/stacktrace/selectors/index.js index dd1ad2c9d91..bf11ae9a8bb 100644 --- a/packages/debugger/lib/stacktrace/selectors/index.js +++ b/packages/debugger/lib/stacktrace/selectors/index.js @@ -175,6 +175,10 @@ let stacktrace = createSelectorTree({ /** * stacktrace.current.callstack.preupdated + * This selector reflects the callstack as it actually is at the current + * moment, rather than carrying around additional error information on top + * in case it turns out to be relevant -- it's been "preupdated" assuming + * we don't want the error info on top, which in certain cases, we don't. */ preupdated: createLeaf( ["./_", "/current/returnCounter"],