Hit MOZ_CRASH(Item found was in the wrong list! type 44 (outer type was 43 at depth 6, now is 70)) at /builds/worker/checkouts/gecko/layout/painting/nsDisplayList.h:2221
Categories
(Core :: Web Painting, defect, P3)
Tracking
()
People
(Reporter: tsmith, Unassigned)
References
Details
(4 keywords, Whiteboard: [bugmon:confirmed,bisected])
Attachments
(1 file, 2 obsolete files)
255 bytes,
text/html
|
Details |
Found while fuzzing 20241208-01166de9b45b (--enable-address-sanitizer --enable-fuzzing)
To reproduce via Grizzly Replay:
$ pip install fuzzfetch grizzly-framework --upgrade
$ python -m fuzzfetch -a --fuzzing -n firefox
$ python -m grizzly.replay.bugzilla ./firefox/firefox <bugid>
Hit MOZ_CRASH(Item found was in the wrong list! type 44 (outer type was 43 at depth 6, now is 70)) at /builds/worker/checkouts/gecko/layout/painting/nsDisplayList.h:2221
#0 0x710f62a0c20f in MOZ_Crash /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:337:3
#1 0x710f62a0c20f in GetOldListIndex /builds/worker/checkouts/gecko/layout/painting/nsDisplayList.h:2218:7
#2 0x710f62a0c20f in mozilla::MergeState::HasMatchingItemInOldList(mozilla::nsDisplayItem*, mozilla::Index<mozilla::OldListUnits>*) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:634:16
#3 0x710f6293a6ec in mozilla::MergeState::ProcessItemFromNewList(mozilla::nsDisplayItem*, mozilla::Maybe<mozilla::Index<mozilla::MergedListUnits>> const&) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:461:9
#4 0x710f6293979c in mozilla::RetainedDisplayListBuilder::MergeDisplayLists(mozilla::nsDisplayList*, mozilla::RetainedDisplayList*, mozilla::RetainedDisplayList*, mozilla::Maybe<mozilla::ActiveScrolledRoot const*>&, mozilla::nsDisplayItem*) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:836:31
#5 0x710f62a0c571 in mozilla::MergeState::MergeChildLists(mozilla::nsDisplayItem*, mozilla::nsDisplayItem*, mozilla::nsDisplayItem*) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:509:37
#6 0x710f6293ab33 in mozilla::MergeState::ProcessItemFromNewList(mozilla::nsDisplayItem*, mozilla::Maybe<mozilla::Index<mozilla::MergedListUnits>> const&) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:481:9
#7 0x710f6293979c in mozilla::RetainedDisplayListBuilder::MergeDisplayLists(mozilla::nsDisplayList*, mozilla::RetainedDisplayList*, mozilla::RetainedDisplayList*, mozilla::Maybe<mozilla::ActiveScrolledRoot const*>&, mozilla::nsDisplayItem*) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:836:31
#8 0x710f62a0c571 in mozilla::MergeState::MergeChildLists(mozilla::nsDisplayItem*, mozilla::nsDisplayItem*, mozilla::nsDisplayItem*) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:509:37
#9 0x710f6293ab33 in mozilla::MergeState::ProcessItemFromNewList(mozilla::nsDisplayItem*, mozilla::Maybe<mozilla::Index<mozilla::MergedListUnits>> const&) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:481:9
#10 0x710f6293979c in mozilla::RetainedDisplayListBuilder::MergeDisplayLists(mozilla::nsDisplayList*, mozilla::RetainedDisplayList*, mozilla::RetainedDisplayList*, mozilla::Maybe<mozilla::ActiveScrolledRoot const*>&, mozilla::nsDisplayItem*) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:836:31
#11 0x710f62a0c571 in mozilla::MergeState::MergeChildLists(mozilla::nsDisplayItem*, mozilla::nsDisplayItem*, mozilla::nsDisplayItem*) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:509:37
#12 0x710f6293ab33 in mozilla::MergeState::ProcessItemFromNewList(mozilla::nsDisplayItem*, mozilla::Maybe<mozilla::Index<mozilla::MergedListUnits>> const&) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:481:9
#13 0x710f6293979c in mozilla::RetainedDisplayListBuilder::MergeDisplayLists(mozilla::nsDisplayList*, mozilla::RetainedDisplayList*, mozilla::RetainedDisplayList*, mozilla::Maybe<mozilla::ActiveScrolledRoot const*>&, mozilla::nsDisplayItem*) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:836:31
#14 0x710f62a0c571 in mozilla::MergeState::MergeChildLists(mozilla::nsDisplayItem*, mozilla::nsDisplayItem*, mozilla::nsDisplayItem*) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:509:37
#15 0x710f6293ab33 in mozilla::MergeState::ProcessItemFromNewList(mozilla::nsDisplayItem*, mozilla::Maybe<mozilla::Index<mozilla::MergedListUnits>> const&) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:481:9
#16 0x710f6293979c in mozilla::RetainedDisplayListBuilder::MergeDisplayLists(mozilla::nsDisplayList*, mozilla::RetainedDisplayList*, mozilla::RetainedDisplayList*, mozilla::Maybe<mozilla::ActiveScrolledRoot const*>&, mozilla::nsDisplayItem*) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:836:31
#17 0x710f62a0c571 in mozilla::MergeState::MergeChildLists(mozilla::nsDisplayItem*, mozilla::nsDisplayItem*, mozilla::nsDisplayItem*) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:509:37
#18 0x710f6293ab33 in mozilla::MergeState::ProcessItemFromNewList(mozilla::nsDisplayItem*, mozilla::Maybe<mozilla::Index<mozilla::MergedListUnits>> const&) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:481:9
#19 0x710f6293979c in mozilla::RetainedDisplayListBuilder::MergeDisplayLists(mozilla::nsDisplayList*, mozilla::RetainedDisplayList*, mozilla::RetainedDisplayList*, mozilla::Maybe<mozilla::ActiveScrolledRoot const*>&, mozilla::nsDisplayItem*) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:836:31
#20 0x710f6294291a in mozilla::RetainedDisplayListBuilder::AttemptPartialUpdate(unsigned int) /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:1668:9
#21 0x710f622a1194 in nsLayoutUtils::PaintFrame(gfxContext*, nsIFrame*, nsRegion const&, unsigned int, mozilla::nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) /builds/worker/checkouts/gecko/layout/base/nsLayoutUtils.cpp:3145:38
#22 0x710f621a11e0 in mozilla::PresShell::PaintInternal(nsView*, mozilla::PaintInternalFlags) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:6564:5
#23 0x710f6193f37f in nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*) /builds/worker/checkouts/gecko/view/nsViewManager.cpp:399:18
#24 0x710f6193e7ed in nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) /builds/worker/checkouts/gecko/view/nsViewManager.cpp:334:22
#25 0x710f61940a97 in nsViewManager::ProcessPendingUpdates() /builds/worker/checkouts/gecko/view/nsViewManager.cpp:837:5
#26 0x710f62113adb in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsRefreshDriver::IsExtraTick) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:2875:11
#27 0x710f62134d22 in operator() /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:1836:25
#28 0x710f62134d22 in mozilla::detail::RunnableFunction<nsRefreshDriver::EnsureTimerStarted(nsRefreshDriver::EnsureTimerStartedFlags)::$_1>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:548:5
#29 0x710f5742ea6a in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:688:16
#30 0x710f57416308 in mozilla::TaskController::RunTask(mozilla::Task*) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:215:19
#31 0x710f5741d44d in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:1015:20
#32 0x710f5741af88 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:838:15
#33 0x710f5741b5a6 in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:624:36
#34 0x710f57443401 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:336:37
#35 0x710f57443401 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.h:548:5
#36 0x710f5746378b in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1159:16
#37 0x710f5746e288 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:480:10
#38 0x710f58a4ae7e in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:85:21
#39 0x710f589301b4 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:369:10
#40 0x710f589301b4 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:362:3
#41 0x710f589301b4 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:344:3
#42 0x710f61a26886 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:148:27
#43 0x710f61bc8b1a in nsAppShell::Run() /builds/worker/checkouts/gecko/widget/gtk/nsAppShell.cpp:469:33
#44 0x710f6386ffbd in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:646:20
#45 0x710f589301b4 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:369:10
#46 0x710f589301b4 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:362:3
#47 0x710f589301b4 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:344:3
#48 0x710f6386e498 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:584:34
#49 0x6431a90420b1 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:397:22
Updated•3 months ago
|
Comment 1•3 months ago
|
||
Item found was in the wrong list! type 44 (outer type was 43 at depth 6, now is 70)
44=PERSPECTIVE
43=OWN_LAYER
70=TRANSFORM
Comment 2•3 months ago
|
||
I'm interested if there is a regression range here (or it's pre-existing for a long time), I assume bugmon will kick in shortly.
So far I haven't been able to reproduce. The closest I've been able to get is following the fuzzfetch instructions in comment 0 I get a different crash deeper in webrender, which might be caused by a messed up display list.
Hit MOZ_CRASH(duplicate key SpatialNodeUid { kind: External { key: SpatialTreeItemKey { key0: 108095740854160, key1: 4294967340 } }, pipeline_id: PipelineId(1, 11), instance_id: PipelineInstanceId(0) }) at gfx/wr/webrender/src/spatial_tree.rs:456 (85fbdff0:1e33cbb4)
I haven't been able to reproduce any assert/crash outside of fuzzfetch so far.
Comment 3•3 months ago
|
||
I suspect one of the prefs that fuzzing flips might explain why fuzzfetch reproduces, how do I get the prefs.js file again?
Reporter | ||
Comment 4•3 months ago
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #3)
I suspect one of the prefs that fuzzing flips might explain why fuzzfetch reproduces, how do I get the prefs.js file again?
The prefs.js file is auto generated but it may randomly selecting the wrong value. Here is what I used to test locally.
Comment 5•3 months ago
|
||
I reproduced this assert with fuzzfetch now, which is getting closer
Assertion failure: !i->IsMergedItem(), at /builds/worker/checkouts/gecko/layout/painting/RetainedDisplayListBuilder.cpp:665
Comment 6•3 months ago
|
||
Hmm, bug 1935561 was opened 4 days ago after there being no open bugs hitting the "Item found was in the wrong list" assert, perhaps suggesting this is a recent regression.
Comment 7•3 months ago
|
||
I can reproduce in a regular build the "duplicate key SpatialNodeUid" crash with the provided prefs.js file. Reducing the important pref was
user_pref("layout.css.overflow-clip-box.enabled", true);
which makes sense given the testcase uses "overflow-clip-box: content-box;"
Comment 8•3 months ago
|
||
With that pref enabled the testcase seems to crash as far back as 2020-06-01 (as far back as I can go on this machine). So doesn't seem like a recent regression.
Comment 9•3 months ago
•
|
||
Seems to be a different regression range on Windows from jul 2021
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0cbc2b32a366dab8b6edf27f20341781841e925a&tochange=eeb3f6140f6910fb78e3942e868a2c39ba25b766
Comment 10•3 months ago
|
||
We switch from |clipCapturedBy = ContainerItemType::Perspective;| to |clipCapturedBy = ContainerItemType::OwnLayerForTransformWithRoundedClip;| here
I assume that happens without marking the frame modified. This controls whether or not we create an OwnLayer item. And then it looks like we stop drawing the caret, do a partial update, and merge the lists. This leads to two copies of the items for the textbox, one of them is inside a OwnLayer item, one which is not.
Comment 11•3 months ago
|
||
We already do a repaintframe changehint, which should mark the frame as modified
for border radius changes. So the border radius must not be changing (which makes sense since it's there from the beginning). So I think we optimize to try to not apply a rounded clip if we can prove that we will be completely inside it and not affected by the rounded part, so my guess would be that something changes to make the frame hit the rounded part. That might be hard to catch all those possibilities, and might affect performance if we did. I'll have to dig more to see what the actual problem is.
Comment 12•3 months ago
|
||
Verified bug as reproducible on mozilla-central 20241209213901-e3b3fa7a28a0.
Unable to bisect testcase (Testcase reproduces on start build!):
Start: dab6b48a2a592a441229e88c88b1aa2570f2171d (20231212043749)
End: 01166de9b45b0edb29c9c97a8d9e2ca9023da37a (20241208090341)
BuildFlags: BuildFlags(asan=True, tsan=False, debug=False, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False, nyx=False, searchfox=False, afl=False)
Successfully recorded a pernosco session. A link to the pernosco session will be added here shortly.
Reporter | ||
Comment 13•3 months ago
•
|
||
A Pernosco session is available here: https://pernos.co/debug/aDXd3VfvOhvApDitP5PhQQ/index.html
Comment 14•3 months ago
•
|
||
A pernosco session for this bug can be found here.
Comment 15•3 months ago
|
||
A pernosco session for this bug can be found here.
Comment 16•3 months ago
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #9)
Seems to be a different regression range on Windows from jul 2021
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0cbc2b32a366dab8b6edf27f20341781841e925a&tochange=eeb3f6140f6910fb78e3942e868a2c39ba25b766
The caret seems key to this, so probably bug 1663475 in here.
Comment 17•3 months ago
|
||
We hit this code
where the presence/absence of a non-zero height caret frame (anywhere in the document!) changes the size of the clip we use. And then I'm assuming we have some code somewhere (I haven't found it yet) that optimizes clips if they can't intersect the content drawn by a frame. So then that changes whether or not there is a rounded rect clip, and that changes what happens here
which changes which display items we create, without marking the frames as modified.
To fix I see two options.
Whenever we hit the overflow clip box content case in the scroll frame we have to disable partial updates (not just when there is a caret because the previous paint could have had a caret that was just removed). This would disable retained display lists whenever a overflow clip box content is on screen, which I believe we use for some form controls. (It is not enabled to be used by content, it's not a standard thing at this point.)
In nsCaret::SchedulePaint whenever it changes if we have a caret frame with non-zero height, we have to disable retained display lists for the next paint only. (It would be better just to mark some nearby frames modified, but due to the scroll frame code we have to look in the whole document for any scroll frames with overflow clip box content.)
I thought of a third option now: nsCaret::SchedulePaint could look for a retained builder and record on it if we changed whether we have a caret frame with non-zero height, and then the scroll frame code could look at that and only disable the partial update if it sees that. But that means we try a partial update, fail and do a full update.
The second option is perhaps the best, and it's still not great. I don't relish forcing a full display list update for the tiny change of showing or hiding a cursor.
From poking around it seems like overflow-clip-box does not have a chance of ever being web exposed. overflow-clip-margin is the potential spec thing that might fill that role.
Giving that the pref layout.css.overflow-clip-box.enabled does not seem like it is ever on track to be enabled, my inclination at this point is to leave this bug unfixed and remove the pref layout.css.overflow-clip-box.enabled from ones that fuzzing flips. But I want to think on it a bit more in case I'm missing anything.
Updated•3 months ago
|
Comment 18•3 months ago
|
||
It appears you figured this out yourself, but that final Pernosco link has working sources.
Comment 19•3 months ago
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #17)
where the presence/absence of a non-zero height caret frame (anywhere in the document!) changes the size of the clip we use.
That seems rather wrong? We should only do that if we're the caret frame (well, if our scrolled frame is...). Wouldn't that fix the bug too?
Comment 20•3 months ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #19)
(In reply to Timothy Nikkel (:tnikkel) from comment #17)
where the presence/absence of a non-zero height caret frame (anywhere in the document!) changes the size of the clip we use.
That seems rather wrong? We should only do that if we're the caret frame (well, if our scrolled frame is...). Wouldn't that fix the bug too?
We would have to check if the caret frame is a descendant of the scroll frame to do it properly, no? We could do that, especially in the case that it is limited to form controls, but in the general case where overflow clip box is fully enabled it's less good.
Unless I'm missing something I don't think it would fix this bug, because the caret is a descendant of the scroll frame, and the caret frame toggles from non-null to null and vice versa (it's not moving around the document, it's either in one place or its not anywhere).
Reporter | ||
Comment 21•3 months ago
|
||
Here is a Pernosco session for the originally reported assertion with working sources: https://pernos.co/debug/NADZVr-mT1UBAfUVazjzrg/index.html
Comment 22•3 months ago
|
||
Do you know why we added layout.css.overflow-clip-box.enabled to the prefs we enable when fuzzing? What do you think about removing it from that set?
My reasoning is that we don't ever plan to enable this pref, the code is only enabled for chrome privileged style sheets, and is only used to implement form controls. As such, it's an implementation detail and not something we ever plan to expose further.
overflow-clip-margin is the thing that seems to be more likely to get specced.
Reporter | ||
Comment 23•3 months ago
|
||
It looks like it was enabled by Jason: https://github.com/MozillaSecurity/prefpicker/commit/a9792af452072151995bd682a24d3efaedfa6cf3
Comment 24•3 months ago
•
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #22)
Do you know why we added layout.css.overflow-clip-box.enabled to the prefs we enable when fuzzing? What do you think about removing it from that set?
I don't recall the initial reasoning for enabling this pref but I also have no problem removing it. See https://github.com/MozillaSecurity/prefpicker/pull/139.
Comment 25•3 months ago
|
||
Thanks. What should we do with this bug, mark it resolved and then see if the fuzzers can hit this asssert without that pref in a new bug?
Reporter | ||
Comment 26•3 months ago
•
|
||
Let's close it and we can always reopen it if the fuzzers find something.
Comment 27•3 months ago
|
||
No valid actions for resolution (WONTFIX).
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.
Reporter | ||
Updated•1 month ago
|
Reporter | ||
Comment 28•1 month ago
|
||
Comment 29•1 month ago
|
||
I hope adding the bugmon keyword back will trigger a bisection.
Reporter | ||
Comment 30•1 month ago
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #29)
I hope adding the bugmon keyword back will trigger a bisection.
Bisection was already reported in comment 12. Bugmon can only go back a year because it uses TC builds.
Comment 31•1 month ago
|
||
Also, just FYI but if the intent is to trigger a new bisection, you'll need to remove bisected
from the whiteboard.
Updated•1 month ago
|
Comment 32•1 month ago
|
||
A new testcase was uploaded, so presumably there could be a different bisect result.
Updated•1 month ago
|
Comment 33•1 month ago
|
||
Unable to bisect testcase (Testcase reproduces on start build!):
Start: 854c462a083b4271435625134f95ce342e243ccb (20240202044327)
End: 01166de9b45b0edb29c9c97a8d9e2ca9023da37a (20241208090341)
BuildFlags: BuildFlags(asan=True, tsan=False, debug=False, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False, nyx=False, searchfox=False, afl=False)
Description
•