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

Fix ReadSymbols in a Module that is already created #686

Merged
merged 4 commits into from
Sep 15, 2020

Conversation

thaystg
Copy link
Contributor

@thaystg thaystg commented Aug 12, 2020

When I try to use ReadSymbols in a Module that is already created, for example:

var symbolReader = portablePdbReaderProvider.GetSymbolReader(asm.image, stream);
asm.image.ReadSymbols(symbolReader);

method.debug_info has a list, but it's empty, so it wasn't entering in the if, but it should, maybe we should change the if to method.debug_info == null || method.debug_info.count = 0.

Relates to this PR: dotnet/runtime#40690

…r example:

var symbolReader = portablePdbReaderProvider.GetSymbolReader(asm.image, stream);
asm.image.ReadSymbols(symbolReader);

method.debug_info has a list, but it's empty, so it wasn't entering in the if, but it should, maybe we should change de if to method.debug_info == null || method.debug_info.count = 0.
@jbevain
Copy link
Owner

jbevain commented Aug 12, 2020

Hi @thaystg, good to see you here.

Could you add a unit test that reproduces your scenario?

@thaystg
Copy link
Contributor Author

thaystg commented Aug 14, 2020

Hi @thaystg, good to see you here.

Could you add a unit test that reproduces your scenario?

Done!!! :)

@thaystg
Copy link
Contributor Author

thaystg commented Sep 15, 2020

@jbevain could you please merge and generate a new nuget version? We need this for wasm debug.

@jbevain jbevain merged commit 191f9fc into jbevain:master Sep 15, 2020
@jbevain
Copy link
Owner

jbevain commented Sep 15, 2020

Thanks!

@jbevain
Copy link
Owner

jbevain commented Sep 15, 2020

@thaystg the fix shipped in 0.11.3. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants