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

array appending can create stale memory references #17488

Open
dlangBugzillaToGithub opened this issue Nov 14, 2024 · 4 comments
Open

array appending can create stale memory references #17488

dlangBugzillaToGithub opened this issue Nov 14, 2024 · 4 comments
Labels

Comments

@dlangBugzillaToGithub
Copy link

Steven Schveighoffer (@schveiguy) reported this on 2024-11-14T04:15:07Z

Transferred from https://issues.dlang.org/show_bug.cgi?id=24860

CC List

  • Adam D. Ruppe

Description

Please see issue 24856.

The array append code has the exact same issue. But I made a separate issue for it, because it's in a different component (druntime vs. phobos).
@dlangBugzillaToGithub
Copy link
Author

destructionator commented on 2024-11-14T12:54:12Z

This affect the built in array ~= operator?

@dlangBugzillaToGithub
Copy link
Author

schveiguy (@schveiguy) commented on 2024-11-14T13:40:21Z

Yes, this affects the built-in array operator.

I'll try and see if I can create a test case for it.

@dlangBugzillaToGithub
Copy link
Author

schveiguy (@schveiguy) commented on 2024-11-14T18:07:19Z

So the test case is a bit more convoluted for older compilers (i.e. the release on run.dlang.io), because the builtin append operation uses the exact size for appending for blocks less than PAGE size.

And for more than page size, the GC is already scanning only the "used" array elements. This means, it shouldn't be happening in the wild with the non-appender array runtime. There is one byte that is not zeroed, but that's because of the array metadata size (which isn't exactly correct, but one byte won't be mistaken for a pointer).

However, in the next release I have modified the growth factor for small blocks to still use the algorithm for smaller blocks (similar to Appender), so ironically, this will introduce the problem.

It does mean that for large blocks, we don't need to zero the array data exactly. But this really is GC dependent, and I'd prefer to have this properly handled by the GC.

Really, we need a GC API to allocate N bytes, but notify we will only be using M of those bytes, so the rest should be zeroed by the GC *if it will be scanned*.

@dlangBugzillaToGithub
Copy link
Author

schveiguy (@schveiguy) commented on 2024-11-14T18:17:47Z

(In reply to Steven Schveighoffer from comment #3)
> However, in the next release I have modified the growth factor for small
> blocks to still use the algorithm for smaller blocks (similar to Appender),
> so ironically, this will introduce the problem.

At first I thought 2.110 would be the release that contains this, but it's not. So we can wait a bit on fixing, and maybe the GC changes I'm planning will make this issue moot.

@thewilsonator thewilsonator added the Druntime Specific to druntime label Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants