Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

[Local GC] Forbid inclusion of gcscan.h from VM and DAC directories #10332

Merged
merged 2 commits into from
Mar 22, 2017

Conversation

swgillespie
Copy link

This change removes the inclusion of gcscan.h from three places in the VM and once place in the DAC. For the VM uses, IGCHeap::RuntimeStructuresValid was added so that the VM can ask the GC whether structures are in valid state (previously done through GCScan::GetGcRuntimeStructuresValid). For the DAC use, the DAC variable that GCScan::GetGcRuntimeStructuresValid reads is referenced directly (gc_structures_invalid_cnt).

@swgillespie
Copy link
Author

x86 job failed with https://github.com/dotnet/coreclr/issues/10022

@dotnet-bot test Windows_NT x86 Checked Build and Test

@swgillespie
Copy link
Author

@Maoni0 @jkotas PTAL? Most of the work to enable this was done as a part of #9255 - so the functionality of GCScan::GetGcRuntimeStructuresValid can be implemented on the DAC side by reading one of the globals on the GC DAC struct.

@@ -1776,9 +1775,10 @@ VOID Object::ValidateInner(BOOL bDeep, BOOL bVerifyNextHeader, BOOL bVerifySyncB
// try to validate next object's header
if (bDeep
&& bVerifyNextHeader
&& GCScan::GetGcRuntimeStructuresValid ()
&& GCHeapUtilities::IsGCHeapInitialized()
Copy link
Member

Choose a reason for hiding this comment

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

IsGCHeapInitialized call does not look necessary - if we have an object to validate, it sure must have come from GCHeap and thus the GCHeap is initialized.

Copy link
Author

Choose a reason for hiding this comment

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

ok!

…assert, since the heap should definitely be initialized if we are validating objects that ostensibly came from it
@swgillespie
Copy link
Author

@dotnet-bot test Linux ARM Emulator Cross Debug Build
@dotnet-bot test Linux ARM Emulator Cross Release Build

@swgillespie swgillespie merged commit d5906ff into dotnet:master Mar 22, 2017
@swgillespie
Copy link
Author

thanks!

@swgillespie swgillespie deleted the gcscan branch March 22, 2017 05:00
jorive pushed a commit to guhuro/coreclr that referenced this pull request May 4, 2017
…otnet#10332)

* Forbid inclusion of gcscan.h from VM and DAC directories

* Address code review feedback - hoist IsGCHeapInitialized check to an assert, since the heap should definitely be initialized if we are validating objects that ostensibly came from it
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
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.

5 participants