Parallel change to the diagnostic update for GC Info decoder.#113888
Parallel change to the diagnostic update for GC Info decoder.#113888VSadov merged 5 commits intodotnet:mainfrom
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (5)
- src/coreclr/gcdump/gcdumpnonx86.cpp: Language not supported
- src/coreclr/inc/gcinfo.h: Language not supported
- src/coreclr/inc/gcinfodecoder.h: Language not supported
- src/coreclr/inc/gcinfotypes.h: Language not supported
- src/coreclr/vm/gcinfodecoder.cpp: Language not supported
ghost
added
the
|
Tagging subscribers to this area: @mangod9 |
I assume that we want SOS to work for .NET Framework too. @mikem8361 Is that correct? If it is the case, we want it to be able to parse GCInfo v1. |
|
Yes, we want SOS to work for .NET Framework.
|
How does it get v1? It looks like v3 or v4 is the only option. Same sources used in another repo maybe? (just want to be sure I am not breaking something I do not notice) |
It is possible that this part of SOS is broken on .NET Framework. |
|
I could set version to v1 and shim the parsing for that, assuming the flags bit size is the only difference. If there is a way to detect .Net Fx via I guess I can just debug and see what happens with .Net Fx |
|
This code can tell you if it is .NET Framework: |
VSadov
force-pushed
the
gcDecR
branch
3 times, most recently
from
cb01b3f to
f30f60f
Compare
src/coreclr/inc/gcinfo.h
Outdated
| // Since in SOS we only care about ability to parse the GC Info, | ||
| // we can assume that everything before net10.0 uses GCInfo v3 | ||
| // v2 and v3 had the same format, so for parsing/dumping purposes they are the same. | ||
| // Also, since runtime cannot parse GC info in nondefault format, |
There was a problem hiding this comment.
since runtime cannot parse GC info in nondefault format,
It is the case at the moment. We had a situation for a while where the runtime parsed multiple version (the older version can come from R2R file). It may be the case again in future.
There was a problem hiding this comment.
Anything is possible, I guess. I think the main invariant here, which should hold, is that the default GC Info format can be inferred from runtime`s version. It would be highly unusual for the default format to change without changing at least minor version of the runtime.
There was a problem hiding this comment.
I will make a change to the comment, just to state as a fact that the default GC Info version can be inferred from the runtime/EE version.
There was a problem hiding this comment.
Actually, maybe that does not even need mentioning.
Only the part that we treat v2 and v3 the same needs to be commented as it is a simplification that we can allow because the file format was the same.
|
Since we do not have anything to add on the runtime side, I think this can be merged. |
|
I see the same failures in a NOOP comment only change - #114048
|
|
/ba-g System.Net.XXX tests fail in other runs too. |
|
Thanks for reviewing!! |
Same file content as in dotnet/diagnostics#5369