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
Closed

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

mvdan opened this issue Mar 7, 2019 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.

Comments

@mvdan
Copy link
Member

mvdan 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

@mvdan mvdan added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsFix The path to resolution is known, but the work has not been done. labels Mar 7, 2019
@xinxiao
Copy link

xinxiao commented Mar 20, 2019

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

@mvdan
Copy link
Member Author

mvdan 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
Copy link
Contributor

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
Copy link

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

@dr2chase
Copy link
Contributor

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....

@golang golang locked and limited conversation to collaborators Mar 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

4 participants