Bug 1830921 Comment 5 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

This seems to be a bad interaction between the StringConcat stub and the VerifierPre gczeal mode. It looks like what happens is:

1. While running in the C++ interpreter, we trigger a VerifyPreTracer run.
2. This [temporarily disables the nursery](https://searchfox.org/mozilla-central/rev/3563da061ca2b32f7f77f5f68088dbf9b5332a9f/js/src/gc/Verifier.cpp#109).
3. This is an incremental trace, so at some point we return to the interpreter.
4. A script tries to enter the baseline interpreter, so we initialize the JitRealm.
5. At this point, the nursery is not enabled, so we [set the initialStringHeap](https://searchfox.org/mozilla-central/rev/3563da061ca2b32f7f77f5f68088dbf9b5332a9f/js/src/jit/JitRealm.h#102-105) to `gc::TenuredHeap`.
6. When we generate the StringConcat stub, we [allocate a new rope](https://searchfox.org/mozilla-central/rev/3563da061ca2b32f7f77f5f68088dbf9b5332a9f/js/src/jit/CodeGenerator.cpp#11111-11113,11125) in `initialStringHeap`, with a comment saying that we don't need post-barriers if that's the default heap. But in this case, it's the tenured heap, even though the nursery was only temporarily disabled.
7. We concatenate a nursery string to a tenured string. There's no prebarrier. The nursery string is eventually collected, and everything explodes.

My initial instinct is that the problem is in step 6, and we should generate post-barriers if `initialStringHeap` is the tenured heap. I also think this is unlikely to be a real security issue, because AFAICT it should only happen with GC zeal enabled. But I'd appreciate a second pair of eyes. Jon, any thoughts?
This seems to be a bad interaction between the StringConcat stub and the VerifierPre gczeal mode. It looks like what happens is:

1. While running in the C++ interpreter, we trigger a VerifyPreTracer run.
2. This [temporarily disables the nursery](https://searchfox.org/mozilla-central/rev/3563da061ca2b32f7f77f5f68088dbf9b5332a9f/js/src/gc/Verifier.cpp#109).
3. This is an incremental trace, so at some point we return to the interpreter.
4. A script tries to enter the baseline interpreter, so we initialize the JitRealm.
5. At this point, the nursery is not enabled, so we [set the initialStringHeap](https://searchfox.org/mozilla-central/rev/3563da061ca2b32f7f77f5f68088dbf9b5332a9f/js/src/jit/JitRealm.h#102-105) to `gc::TenuredHeap`.
6. When we generate the StringConcat stub, we [allocate a new rope](https://searchfox.org/mozilla-central/rev/3563da061ca2b32f7f77f5f68088dbf9b5332a9f/js/src/jit/CodeGenerator.cpp#11111-11113,11125) in `initialStringHeap`, with a comment saying that we don't need post-barriers if that's the default heap. But in this case, it's the tenured heap, even though the nursery was only temporarily disabled.
7. We concatenate a nursery string. The new rope is allocated in the tenured space. There's no prebarrier. The nursery string is eventually collected, and everything explodes.

My initial instinct is that the problem is in step 6, and we should generate post-barriers if `initialStringHeap` is the tenured heap. I also think this is unlikely to be a real security issue, because AFAICT it should only happen with GC zeal enabled. But I'd appreciate a second pair of eyes. Jon, any thoughts?

PS: I didn't spend any time reducing the testcase, but I did reduce the list of shell flags to `--differential-testing --ion-osr=off --fast-warmup --blinterp-warmup-threshold=1 --gc-zeal=4,230 --baseline-warmup-threshold=100 --no-threads`.

Back to Bug 1830921 Comment 5