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.

Sign up for GitHub

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

Jump to bottom

runtime/trace: TestTraceSymbolize broken on solaris/amd64 as of CL 463835 #59350

Closed
bcmills opened this issue Mar 31, 2023 · 5 comments
Closed

runtime/trace: TestTraceSymbolize broken on solaris/amd64 as of CL 463835 #59350

bcmills opened this issue Mar 31, 2023 · 5 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-illumos OS-Solaris
Milestone
Backlog

Comments

@bcmills
Copy link
Contributor

bcmills commented Mar 31, 2023 •
edited

https://build.golang.org/log/ec79810c83e1ddca4758df05fd4b8f0351fe462a:

solaris-amd64-oraclerel at ba71817390f78bf8c479dc65d1bc51db98d667a7
…
--- FAIL: TestTraceSymbolize (0.24s)
    trace_stack_test.go:285: Did not match event GoSysCall with stack
          syscall.read					 :0
          syscall.Read					 :0
          internal/poll.ignoringEINTRIO			 :0
          internal/poll.(*FD).Read			 :0
          os.(*File).read				 :0
          os.(*File).Read				 :0
          runtime/trace_test.TestTraceSymbolize.func11	 :0

        Seen 43 events of the type
…
        Offset 855
          syscall.Read					/go/src/syscall/syscall_unix.go:181
          internal/poll.ignoringEINTRIO			/go/src/internal/poll/fd_unix.go:793
          internal/poll.(*FD).Read			/go/src/internal/poll/fd_unix.go:163
          os.(*File).read				/go/src/os/file_posix.go:31
          os.(*File).Read				/go/src/os/file.go:118
          runtime/trace_test.TestTraceSymbolize.func11	/go/src/runtime/trace/trace_stack_test.go:105
…
        Offset 2672
          syscall.Read					/go/src/syscall/syscall_unix.go:181
          internal/poll.ignoringEINTRIO			/go/src/internal/poll/fd_unix.go:793
          internal/poll.(*FD).Read			/go/src/internal/poll/fd_unix.go:163
          os.(*File).read				/go/src/os/file_posix.go:31
          os.(*File).Read				/go/src/os/file.go:118
          runtime/trace_test.TestTraceSymbolize.func11	/go/src/runtime/trace/trace_stack_test.go:105

This failure mode seems to have been introduced by CL 463835. It isn't clear to me whether it affects illumos because the test results are blocked by #58967.

(attn @golang/solaris @felixge @prattmic)
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Mar 31, 2023
@bcmills bcmills added this to the Backlog milestone Mar 31, 2023
@prattmic
Copy link
Member

syscall.read calls syscall.sysvicall6, which is a simple jump to runtime.syscall_sysvicall6.

I'd guess that one of those latter frames doesn't have proper framepointer handling.

@bcmills
Copy link
Contributor Author

bcmills commented Mar 31, 2023

Looks like that call sequence does also apply to illumos (attn @golang/illumos), so adding that tag.

@felixge
Copy link
Contributor

felixge commented Mar 31, 2023 •
edited

syscall.read calls syscall.sysvicall6, which is a simple jump to runtime.syscall_sysvicall6.

I'd guess that one of those latter frames doesn't have proper framepointer handling.

Sounds plausible. It would probably nice to fix the framepointer handling.

But for now we could also try to special-case the syscall test expectations for solaris and illumos? I could probably send a patch for the latter quickly, the former might take more time.

@felixge felixge mentioned this issue Apr 1, 2023
Open
@gopherbot
Copy link

Change https://go.dev/cl/481336 mentions this issue: runtime/trace: Fix TestTraceSymbolize on solaris and illumos

@felixge
Copy link
Contributor

felixge commented Apr 2, 2023 •
edited

Change https://go.dev/cl/481336 mentions this issue: runtime/trace: Fix TestTraceSymbolize on solaris and illumos

This should fix the problem for now.

As far as I can tell the issue wasn't caused by frames with incorrect frame pointer handling, but rather by the different number of frames between the trace events and the actual syscalls on different platforms.

@mknyszek mknyszek added the NeedsFix The path to resolution is known, but the work has not been done. label Apr 3, 2023
@golang golang locked and limited conversation to collaborators Apr 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-illumos OS-Solaris
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@felixge @mknyszek @prattmic @bcmills @gopherbot