Skip to content

Enable disasm mixed gc info#403

Merged
BruceForstall merged 4 commits intodotnet:masterfrom
BruceForstall:EnableDisasmMixedGCinfo
Jul 18, 2019
Merged

Enable disasm mixed gc info#403
BruceForstall merged 4 commits intodotnet:masterfrom
BruceForstall:EnableDisasmMixedGCinfo

Conversation

@BruceForstall
Copy link
Contributor

Rewrite the "u -gcinfo" code to not require Windows fibers,
and enable it for non-Windows platforms.

Also,

  1. Enable "u -gcinfo" for ARM.
  2. Fix minor issues with EHinfo/GCinfo to ensure end-of-function info is displayed

Rewrite the "u -gcinfo" code to not require Windows fibers,
and enable it for non-Windows platforms.

Also,
1. Enable "u -gcinfo" for ARM.
2. Fix minor issues with EHinfo/GCinfo to ensure end-of-function info is displayed
@BruceForstall BruceForstall requested a review from mikem8361 July 17, 2019 23:45
@BruceForstall
Copy link
Contributor Author

@mikem8361 PTAL.

I've tested this works still on Windows/x64. I'm having trouble building on Ubuntu 16.04 x64, but obviously that's the real test. I'm also trying to test on Windows arm32, since I enabled that for "!gcinfo" as well.


void GCEncodingInfo::Deinitialize()
{
delete[] buf;
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't remember for sure, but does delete check for null? Do you need to check buf for nullptr first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

delete does accept nullptr (presumably it hasn't been overridden by an implementation that does not handle it)

Copy link
Contributor

@mikem8361 mikem8361 left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for doing this.

@BruceForstall
Copy link
Contributor Author

Fixes #54

@BruceForstall
Copy link
Contributor Author

@mikem8361 Does the diagnostics repo have Azure DevOps CI testing for PRs? E.g., something that builds and run tests on all architectures?

@BruceForstall BruceForstall changed the title WIP Enable disasm mixed gc info Enable disasm mixed gc info Jul 18, 2019
@BruceForstall
Copy link
Contributor Author

I was able to build and test this on Linux/x64, so this is no longer WIP.

@BruceForstall BruceForstall merged commit 61597c7 into dotnet:master Jul 18, 2019
@BruceForstall BruceForstall deleted the EnableDisasmMixedGCinfo branch July 18, 2019 19:04
@github-actions github-actions bot locked and limited conversation to collaborators Jan 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants