Skip to content

gocore: bugfix to Go 1.17 cores, Go 1.16 cores and Go 1.11+ cores #7

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
Closed
wants to merge 9 commits into from
Closed

gocore: bugfix to Go 1.17 cores, Go 1.16 cores and Go 1.11+ cores #7

wants to merge 9 commits into from

Conversation

jordanlewis
Copy link
Contributor

@jordanlewis jordanlewis commented May 22, 2021 •
edited
Loading

This CL fixes several bugs in gocore, the library that viewcore uses to
parse a core dump, as well as adds general updates for the more recent Go
versions.

First, we fix _func types for Go 1.16. In 1.16, the types of a few fields in
_func were changed. This commit makes the corresponding change to gocore. This
is the commit the changed the field types: CL 248332. Additionally, we
updated the func parsing to deal with the new split pclntab.

Previously, the code that deserialized the heapArena.bitmap field to
check whether addresses contained pointers or not was incorrect. It was
treating the entire bitmap as 1 bit per pointer.

However, that is not what the bitmap represents. Each byte in the bitmap
is actually split in half. The high bits contain the 4 scan bits. And
the low bits contain the 4 pointer bits.

See
https://github.com/golang/go/blob/3b304ce7fe35b9d1e8cf0b0518ed2550c361a010/src/runtime/mbitmap.go#L17-L35
for a more detailed description.

This commit corrects the issue and allows gocore to correctly traverse
the object graph.

Finally, for Go 1.17, we stop subtracting the heap unallocated space
represented by curArena from the total size of the heap. This reflects the
change that was made in CL 270537.

Also, a few incidental fixes:

  • Stop thrashing the viewcore cache on the first command in interactive mode.
  • Synthesize types for 0-size arrays in structs

@google-cla google-cla bot added the cla: yes label May 22, 2021
@jordanlewis jordanlewis mentioned this pull request May 22, 2021
Open
@gopherbot
Copy link
Contributor

This PR (HEAD: 2ee1eb8) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/debug/+/321736 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Alessandro Arzilli:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jordan Lewis:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: fe7bfbf) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/debug/+/321736 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Jordan Lewis:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Alessandro Arzilli:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

jeremyfaller
jeremyfaller approved these changes Jun 10, 2021
View reviewed changes
Copy link

@jeremyfaller jeremyfaller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@randall77 @mknyszek For the process.go changes.

pclntab specific work LGTM.

@gopherbot
Copy link
Contributor

Message from Keith Randall:

Patch Set 2: Code-Review+2

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jordan Lewis:

Patch Set 2:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Keith Randall:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Cherry Mui:

Patch Set 2: Run-TryBot+1 Trust+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 2: TryBot-Result-1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Matthew Dempsky:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jordan Lewis:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Matthew Dempsky:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@bboreham bboreham mentioned this pull request Jun 30, 2021
Open
@gopherbot
Copy link
Contributor

Message from Jordan Lewis:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jordan Lewis:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Alessandro Arzilli:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Keith Randall:

Patch Set 2: Code-Review+2

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jordan Lewis:

Patch Set 2:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Keith Randall:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Cherry Mui:

Patch Set 2: Run-TryBot+1 Trust+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 2: TryBot-Result-1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@jordanlewis jordanlewis changed the title gocore: bugfix to Go 1.16 cores and Go 1.11+ cores gocore: bugfix to Go 1.17 cores, Go 1.16 cores and Go 1.11+ cores Nov 20, 2021
@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 5:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 5: TryBot-Result+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jordan Lewis:

Patch Set 7:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Daniel Martí:

Patch Set 8: Run-TryBot+1 Trust+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 8:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 8: TryBot-Result+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jordan Lewis:

Patch Set 8:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from DeJiang Zhu:

Patch Set 8:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Hyang-Ah Hana Kim:

Patch Set 8: Code-Review+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from thepudds:

Patch Set 8:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jordan Lewis:

Patch Set 8:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@jordanlewis
gocore: fix _func types for Go 1.16
af6768e 
In Go 1.16, the types of a few fields in _func were changed. This commit
makes the corresponding change to gocore.

This is the commit the changed the field types:
golang/go@26407b2
@jordanlewis
gocore: bugfix to pointer bitmap deserialization
9fe4600 
Previously, the code that deserialized the heapArena.bitmap field to
check whether addresses contained pointers or not was incorrect. It was
treating the entire bitmap as 1 bit per pointer.

However, that is not what the bitmap represents. Each byte in the bitmap
is actually split in half. The high bits contain the 4 scan bits. And
the low bits contain the 4 pointer bits.

See
https://github.com/golang/go/blob/3b304ce7fe35b9d1e8cf0b0518ed2550c361a010/src/runtime/mbitmap.go#L17-L35
for a more detailed description.

This commit corrects the issue and allows gocore to correctly traverse
the object graph.
@jordanlewis
don't thrash cache on first command
c92a01f 
Setting the interactive bit after loading core makes the core cache not
work.
@jordanlewis
fix func parsing for Go 1.16
f309c2a 
Go 1.16 split pclntab up into a few different tables. This commit
updates the code to use the correct tables to parse various pieces of
function data.
@jordanlewis
speed up type parsing for large primitive arrays
b9e5b63 
Don't bother calling functions once per element in a big primitive
array.
@jordanlewis
synthesize types for 0 size arrays in structs
f1b06b4
@jordanlewis
gocore: add version gates for 1.16; update tests
251025d
@jordanlewis
gocore: update for Go1.17
2a4799e
@jordanlewis
fixup! gocore: add version gates for 1.16; update tests
c66ea5a
@gopherbot
Copy link
Contributor

This PR (HEAD: c66ea5a) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/debug/+/321736 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Jordan Lewis:

Patch Set 8:

(5 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Dmitri Shuralyov:

Patch Set 9: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 9:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 9: TryBot-Result+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/321736.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Apr 3, 2022
@jordanlewis @dominikh
gocore: bugfix to Go 1.17 cores, Go 1.16 cores and Go 1.11+ cores
0bd8c7d 
This CL fixes several bugs in `gocore`, the library that `viewcore` uses to
parse a core dump, as well as adds general updates for the more recent Go
versions.

First, we fix _func types for Go 1.16. In 1.16, the types of a few fields in
_func were changed. This commit makes the corresponding change to gocore. This
is the commit the changed the field types: CL 248332. Additionally, we
updated the func parsing to deal with the new split pclntab.

Previously, the code that deserialized the heapArena.bitmap field to
check whether addresses contained pointers or not was incorrect. It was
treating the entire bitmap as 1 bit per pointer.

However, that is not what the bitmap represents. Each byte in the bitmap
is actually split in half. The high bits contain the 4 scan bits. And
the low bits contain the 4 pointer bits.

See
https://github.com/golang/go/blob/3b304ce7fe35b9d1e8cf0b0518ed2550c361a010/src/runtime/mbitmap.go#L17-L35
for a more detailed description.

This commit corrects the issue and allows gocore to correctly traverse
the object graph.

Finally, for Go 1.17, we stop subtracting the heap unallocated space
represented by curArena from the total size of the heap. This reflects the
change that was made in CL 270537.

Also, a few incidental fixes:
- Stop thrashing the viewcore cache on the first command in interactive mode.
- Synthesize types for 0-size arrays in structs

Change-Id: Ia1636932d7c6c59bbd640f6b5b00b221369fed44
GitHub-Last-Rev: c66ea5a
GitHub-Pull-Request: #7
Reviewed-on: https://go-review.googlesource.com/c/debug/+/321736
Trust: Cherry Mui <[email protected]>
Trust: Daniel Martí <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
Run-TryBot: Dmitri Shuralyov <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/321736 has been merged.

@gopherbot gopherbot closed this Apr 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeremyfaller jeremyfaller jeremyfaller approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jordanlewis @gopherbot @jeremyfaller