dap: fix skipped breakpoint when the breakpoint and the entrypoint were the same

We erroneously skipped a breakpoint when that breakpoint was the same as
the entrypoint and we did not use stop on entry. This is because we only
started evaluating breakpoints after the first step on the entrypoint
instead of at the entrypoint.

Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>

Author: jsternberg

dap/thread.go

@@ -71,18 +71,20 @@ func (t *thread) Evaluate(ctx Context, c gateway.Client, headRef gateway.Referen
 	}
 	defer t.reset()
 
+	var next *step
 	action := stepContinue
 	if cfg.StopOnEntry {
-		action = stepNext
+		// If we are stopping on entry, automatically advance to the
+		// entrypoint.
+		action, next = stepNext, t.entrypoint
 	}
 
 	var (
 		k    string
 		refs map[string]gateway.Reference
-		next = t.entrypoint
 		err  error
 	)
-	for next != nil {
+	for {
 		event := t.needsDebug(next, action, err)
 		if event.Reason != "" {
 			select {
@@ -98,7 +100,9 @@ func (t *thread) Evaluate(ctx Context, c gateway.Client, headRef gateway.Referen
 		}
 
 		t.setBreakpoints(ctx)
-		k, next, refs, err = t.seekNext(ctx, next, action)
+		if k, next, refs, err = t.seekNext(ctx, next, action); next == nil {
+			break
+		}
 	}
 	return nil
 }
@@ -509,19 +513,24 @@ func (t *thread) setBreakpoints(ctx Context) {
 }
 
 func (t *thread) seekNext(ctx Context, from *step, action stepType) (string, *step, map[string]gateway.Reference, error) {
-	// If we're at the end, return no digest to signal that
-	// we should conclude debugging.
-	var target *step
+	// Determine how we are going to limit the scan for the next step.
+	var limit func(s *step) *step
 	switch action {
 	case stepNext:
-		target = t.continueDigest(from, from.next)
+		limit = func(s *step) *step {
+			return s.next
+		}
 	case stepIn:
-		target = from.in
+		limit = func(s *step) *step {
+			return s.in
+		}
 	case stepOut:
-		target = t.continueDigest(from, from.out)
-	case stepContinue:
-		target = t.continueDigest(from, nil)
+		limit = func(s *step) *step {
+			return s.out
+		}
 	}
+
+	target := t.continueDigest(from, limit)
 	return t.seek(ctx, target)
 }
 
@@ -547,8 +556,10 @@ func (t *thread) seek(ctx Context, target *step) (k string, result *step, mounts
 	return k, result, refs, nil
 }
 
-func (t *thread) continueDigest(from, until *step) *step {
-	if len(t.bps) == 0 && until == nil {
+func (t *thread) continueDigest(from *step, limit func(*step) *step) *step {
+	// First chance to exit early. If there's no function for limiting
+	// the until step and no breakpoints then just go directly to the end step.
+	if len(t.bps) == 0 && limit == nil {
 		return nil
 	}
 
@@ -561,6 +572,27 @@ func (t *thread) continueDigest(from, until *step) *step {
 		return ok
 	}
 
+	// Special case. When we aren't coming from any step we consider
+	// whether the entrypoint itself is a breakpoint. If it is, we stop
+	// there. Otherwise, we treat the entrypoint as the from location.
+	if from == nil {
+		if isBreakpoint(t.entrypoint.dgst) {
+			return t.entrypoint
+		}
+		from = t.entrypoint
+	}
+
+	var until *step
+	if limit != nil {
+		until = limit(from)
+	}
+
+	// Second chance to exit early. If we've fully resolved from and the
+	// limit function doesn't return an end step, just go directly to the end.
+	if len(t.bps) == 0 && until == nil {
+		return nil
+	}
+
 	next := func(s *step) *step {
 		cur := s.in
 		for cur != nil && cur != until {

tests/dap_build.go

@@ -78,6 +78,7 @@ var dapBuildTests = []func(t *testing.T, sb integration.Sandbox){
 	testDapBuild,
 	testDapBuildStopOnEntry,
 	testDapBuildSetBreakpoints,
+	testDapBuildEntryBreakpoint,
 	testDapBuildVerifiedBreakpoints,
 	testDapBuildLoadedSource,
 	testDapBuildStepIn,
@@ -121,17 +122,19 @@ func testDapBuildStopOnEntry(t *testing.T, sb integration.Sandbox) {
 	})
 
 	stopped := waitForInterrupt[*dap.StoppedEvent](t, interruptCh)
+	require.Equal(t, "step", stopped.Body.Reason)
+
 	threads := doThreads(t, client)
 	require.ElementsMatch(t, []int{stopped.Body.ThreadId}, threads)
 
-	stackTraceResp := <-daptest.DoRequest[*dap.StackTraceResponse](t, client, &dap.StackTraceRequest{
-		Request: dap.Request{Command: "stackTrace"},
-		Arguments: dap.StackTraceArguments{
-			ThreadId: stopped.Body.ThreadId,
+	stackFrames := doStackTrace(t, client, stopped.Body.ThreadId)
+	assertStackTrace(t, stackFrames, []stackFrameMatcher{
+		{
+			SourceName: "Dockerfile",
+			Line:       7,
+			Name:       `^\[stage-1 .*\] COPY`,
 		},
 	})
-	require.True(t, stackTraceResp.Success)
-	require.Len(t, stackTraceResp.Body.StackFrames, 1)
 
 	var exitErr *exec.ExitError
 	require.ErrorAs(t, done(true), &exitErr)
@@ -202,6 +205,39 @@ func testDapBuildSetBreakpoints(t *testing.T, sb integration.Sandbox) {
 	require.NoError(t, done(false))
 }
 
+// testDapBuildEntryBreakpoint checks that the entrypoint is a valid breakpoint.
+func testDapBuildEntryBreakpoint(t *testing.T, sb integration.Sandbox) {
+	dir := createTestProject(t)
+	client, done, err := dapBuildCmd(t, sb, withArgs(dir))
+	require.NoError(t, err)
+
+	interruptCh := pollInterruptEvents(client)
+	doLaunch(t, client, commands.LaunchConfig{
+		Dockerfile:  path.Join(dir, "Dockerfile"),
+		ContextPath: dir,
+	},
+		dap.SourceBreakpoint{Line: 7},
+	)
+
+	stopped := waitForInterrupt[*dap.StoppedEvent](t, interruptCh)
+	require.Equal(t, "breakpoint", stopped.Body.Reason)
+
+	threads := doThreads(t, client)
+	require.ElementsMatch(t, []int{stopped.Body.ThreadId}, threads)
+
+	stackFrames := doStackTrace(t, client, stopped.Body.ThreadId)
+	assertStackTrace(t, stackFrames, []stackFrameMatcher{
+		{
+			SourceName: "Dockerfile",
+			Line:       7,
+			Name:       `^\[stage-1 .*\] COPY`,
+		},
+	})
+
+	var exitErr *exec.ExitError
+	require.ErrorAs(t, done(true), &exitErr)
+}
+
 func testDapBuildVerifiedBreakpoints(t *testing.T, sb integration.Sandbox) {
 	dir := createTestProject(t)
 	client, done, err := dapBuildCmd(t, sb, withArgs(dir))