Minor false positive fixes in ChakraCore #1581
Conversation
1. BitVectors were not allocated as leaf, and this could cause the GC to interpret it's contents as pointer references. Switched to non-leaf 2. Temporary guest-arenas that were being cached could still get scanned, even if they weren't actively being used. Added a mechanism to unregister said arena when it wasn't used.
| { | ||
| if (isGuestArena) | ||
| { | ||
| if (externalGuestArenaRef == nullptr) |
There was a problem hiding this comment.
If we assert that externalGuestArenaRef != nullptr in
There was a problem hiding this comment.
If we assert that externalGuestArenaRef != nullptr in AdviseNotInUse, should this be an assert externalGuestArenaRef == nullptr too?
| @@ -18,6 +18,8 @@ namespace Js | |||
|
|
|||
| public: | |||
|
|
|||
| void AdviseInUse(); | |||
| void AdviseNotInUse(); | |||
There was a problem hiding this comment.
Were the implementations of these already present?
There was a problem hiding this comment.
No? Newly added
There was a problem hiding this comment.
Derp; I didn't see them above
Merge pull request #1581 from digitalinfinity:min_leaks 1. BitVectors were not allocated as leaf, and this could cause the GC to interpret it's contents as pointer references. Switched to leaf 2. Temporary guest-arenas that were being cached could still get scanned, even if they weren't actively being used. Added a mechanism to unregister said arena when it wasn't used.
| @@ -38,6 +38,8 @@ class NativeCodeData | |||
|
|
|||
| char * Alloc(DECLSPEC_GUARD_OVERFLOW size_t requestedBytes); | |||
| char * AllocZero(DECLSPEC_GUARD_OVERFLOW size_t requestedBytes); | |||
| char * AllocLeaf(__declspec(guard(overflow)) size_t requestedBytes); | |||
There was a problem hiding this comment.
@digitalinfinity Strange, why CI did not complain on this? Should use DECLSPEC_GUARD_OVERFLOW
There was a problem hiding this comment.
derp- I don't know? @dilijev any ideas? @jainchun, i'll make the change- sorry, had ported the change from Threshold and didn't notice the macro conversion 😞
There was a problem hiding this comment.
JIT is still disabled on Linux so this whole block is probably compiled out on Linux
Uh oh!
There was an error while loading. Please reload this page.