Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: write a test for #30167 #30664

Closed
mvdan opened this issue Mar 7, 2019 · 5 comments

Comments

Projects
None yet
4 participants
@mvdan
Copy link
Member

commented Mar 7, 2019

https://go-review.googlesource.com/c/go/+/163019 was sent and submitted without a test, because writing one wasn't possible with the current test suite. David has an idea to refactor the tests to support this kind of test case (see the CL comments); this issue is a reminder to write that test in the future.

/cc @pwaller @dr2chase

@xinxiao

This comment has been minimized.

Copy link

commented Mar 20, 2019

@mvdan May I carry on the task to work on this issue please?

@mvdan

This comment has been minimized.

Copy link
Member Author

commented Mar 20, 2019

It's not assigned, so you're welcome to try. Though I think @dr2chase has specific plans on how to fix it, and they're non-trivial.

@dr2chase

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

Ah, plans are changing slightly, and might become simpler. I experimented with gdb-MI a little bit, and it appears that it might still be flaky (I am not 100% sure of this), plus in practice, people use Delve, not gdb, because gdb doesn't do a good job with goroutines.

And Delve isn't flaky. I have to be sure that Delve is supported on our test boxes, and then we switch to that as the default.

The fix I imagine is to enhance debug_test.go to do one of the following:

  • take a step limit (either read from the file or as part of test specification)
  • take an "exit" annotation in the source file, telling to quit out of the process

And then use that to write the test.

@gopherbot

This comment has been minimized.

Copy link

commented Mar 20, 2019

Change https://golang.org/cl/168477 mentions this issue: cmd/compile: expose ssa/debug_test step limit to bound infinite loops

@dr2chase

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

It needs more than just a test; there's something buggy going on, but I won't get to it today.
Recommended attack (in CL above) is

cd src/cmd/compile/internal/ssa/testdata
GOSSAFUNC=test go build infloop.go

It loops like we should maybe notice that we are creating a one-instruction infinite loop, and throw a nop in there? I'm not sure, it depends on debugger heuristics, but it is definitely tagged as a statement.

That would make the infinite loop take longer to finish....

@gopherbot gopherbot closed this in 591193b Mar 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.