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

Make GC stack walking robust in the presence of unmanaged byrefs with extended lifetime #75865

Open
jkotas opened this issue Sep 19, 2022 · 7 comments

Comments

@jkotas
Copy link
Member

jkotas commented Sep 19, 2022

Consider this example:

byte* ptr = Marshal.AllocHGlobal(...);
ref byte b = ref Unsafe.AsRef<byte>(ptr);
Use(ref b);
Marshal.FreeHGlobal(ptr);

// `b` never used again in the code. Let's assume JIT decided to extend the `b` variable lifetime till end of the method.

object o = new object(); // Let's assume that the GC run out of space, allocated a new memory block using virtual alloc, and the OS memory manager decides to give it the memory block that was just freed by `Marshal.FreeHGlobal`.

GC.Collect(); // The GC may assert or crash because of it will see `byref b` pointing into middle of the segment where no valid byrefs are supposed to point to

Context: #75857 (comment)

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 19, 2022
@ghost
Copy link

ghost commented Sep 19, 2022

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Consider this example:

byte* ptr = Marshal.AllocHGlobal(...);
ref byte b = ref Unsafe.AsRef<byte>(ptr);
Use(ref b);
Marshal.FreeHGlobal(ptr);

// `b` never used again in the code. Let's assume JIT decided to extend the `b` variable lifetime till end of the method.

object o = new object(); // Let's assume that the GC run out of space, allocated a new memory block using virtual alloc, and the OS memory manager decides to give it the memory block that was just freed by `Marshal.FreeHGlobal`.

GC.Collect(); // The GC may assert or crash because of it will see `byref b` pointing into middle of the segment where no valid byrefs are supposed to point to

Context: #75857 (comment)

Author: jkotas
Assignees: -
Labels:

area-GC-coreclr

Milestone: -

@jkotas
Copy link
Member Author

jkotas commented Sep 19, 2022

@davidwrighton Do you have thoughts about this scenario? It is related to the clarification made in #71794.

We may want to make GCConfig::GetConservativeGC to be on by default to fix part of this problem.

@Maoni0
Copy link
Member

Maoni0 commented Sep 19, 2022

with regions you don't have this problem since we reserve a big range for regions GC will be acquiring from so you'll never had such overlap.

@Maoni0 Maoni0 added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 19, 2022
@ghost
Copy link

ghost commented Sep 19, 2022

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

Consider this example:

byte* ptr = Marshal.AllocHGlobal(...);
ref byte b = ref Unsafe.AsRef<byte>(ptr);
Use(ref b);
Marshal.FreeHGlobal(ptr);

// `b` never used again in the code. Let's assume JIT decided to extend the `b` variable lifetime till end of the method.

object o = new object(); // Let's assume that the GC run out of space, allocated a new memory block using virtual alloc, and the OS memory manager decides to give it the memory block that was just freed by `Marshal.FreeHGlobal`.

GC.Collect(); // The GC may assert or crash because of it will see `byref b` pointing into middle of the segment where no valid byrefs are supposed to point to

Context: #75857 (comment)

Author: jkotas
Assignees: -
Labels:

area-CodeGen-coreclr, area-GC-coreclr, untriaged

Milestone: -

@jkotas
Copy link
Member Author

jkotas commented Sep 19, 2022

Are you saying that you never ever call virtual alloc to reserve more memory with regions? Or that it is unlikely to run into this with regions?

Also, we still have 32-bit platforms where regions are not enabled.

I do not think codegen is the right area label for this. There is nothing that codegen team can do about this. If there is a change to do for this, it would need to be in the GC.

@jkotas jkotas added area-GC-coreclr and removed area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Sep 19, 2022
@ghost
Copy link

ghost commented Sep 19, 2022

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Consider this example:

byte* ptr = Marshal.AllocHGlobal(...);
ref byte b = ref Unsafe.AsRef<byte>(ptr);
Use(ref b);
Marshal.FreeHGlobal(ptr);

// `b` never used again in the code. Let's assume JIT decided to extend the `b` variable lifetime till end of the method.

object o = new object(); // Let's assume that the GC run out of space, allocated a new memory block using virtual alloc, and the OS memory manager decides to give it the memory block that was just freed by `Marshal.FreeHGlobal`.

GC.Collect(); // The GC may assert or crash because of it will see `byref b` pointing into middle of the segment where no valid byrefs are supposed to point to

Context: #75857 (comment)

Author: jkotas
Assignees: -
Labels:

area-CodeGen-coreclr, area-GC-coreclr, untriaged

Milestone: -

@Maoni0
Copy link
Member

Maoni0 commented Sep 19, 2022

I'm saying with regions that'll never happen.

yes there'll still be 32-bit and segments will continue to be used there which makes it much lower priority.

@mangod9 mangod9 removed the untriaged New issue has not been triaged by the area owner label Sep 20, 2022
@mangod9 mangod9 added this to the 8.0.0 milestone Sep 20, 2022
@mangod9 mangod9 modified the milestones: 8.0.0, Future Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants