Bug 249617 – REGRESSION(257434@main): Crash in RenderReplaced::computeIntrinsicSizesConstrainedByTransferredMinMaxSizes on https://gitlab.com/gnutls/gnutls/

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Michael Catanzaro]

Using WebKitGTK 2.39.3 (257841@main), visiting https://gitlab.com/gnutls/gnutls/ causes a web process crash 100% of the time. (But surprisingly, visiting other project pages on gitlab.com does not trigger the crash!) The crashing code was added in 257434@main "Replaced elements with aspect ratio and size in one dimension should respect min-max constraints in opposite dimension" so I'll assume it's a regression from that commit. It's crashing on a libstdc++ assertion inside std::clamp, and the assertion text "!(__hi < __lo)" was helpfully included in the backtrace, so the crash happens because maxLogicalWidth is less than minLogicalWidth at RenderReplaced.cpp:480. The full version of the most interesting frame is:

#7  WebCore::RenderReplaced::computeIntrinsicSizesConstrainedByTransferredMinMaxSizes(WebCore::RenderBox*, WebCore::FloatSize&, WebCore::FloatSize&) const
    (this=this@entry=0x7f3811112880, intrinsicSize=..., intrinsicRatio=..., contentRenderer=0x0)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderReplaced.cpp:460
        minLogicalWidth = <synthetic pointer>
        maxLogicalWidth = <synthetic pointer>
        minLogicalHeight = <synthetic pointer>
        maxLogicalHeight = <synthetic pointer>

Unfortunately, the backtrace does not show what the width and height values are, but note contentRenderer=0x0. The function *does* safely handle nullptr here in the sense that it won't dereference it, because it is only passed to computeAspectRatioInformationForRenderBox and that function checks for nullptr, but maybe there is a new logic error in that case.

Truncated short version of the backtrace:

#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0)
    at pthread_kill.c:44
#1  0x00007f38ab2911f3 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78
#2  0x00007f38ab23f00e in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007f38ab2287fc in __GI_abort () at abort.c:79
#4  0x00007f38a74e200d in std::__glibcxx_assert_fail(char const*, int, char const*, char const*)
    (file=file@entry=0x7f38ae989798 "/usr/include/c++/12.1.0/bits/stl_algo.h", line=line@entry=3623, function=function@entry=0x7f38ae9f9990 "constexpr const _Tp& std::clamp(const _Tp&, const _Tp&, const _Tp&) [with _Tp = WebCore::LayoutUnit]", condition=condition@entry=0x7f38ae989ba8 "!(__hi < __lo)")
    at ../../../../../libstdc++-v3/src/c++11/debug.cc:60
#5  0x00007f38ae094467 in std::clamp<WebCore::LayoutUnit>(WebCore::LayoutUnit const&, WebCore::LayoutUnit const&, WebCore::LayoutUnit const&)Python Exception <class 'gdb.error'>: cannot use offset on synthetic pointer to register

    (__val=<optimized out>, __lo=<synthetic pointer>..., __hi=#6  std::clamp<WebCore::LayoutUnit>(WebCore::LayoutUnit const&, WebCore::LayoutUnit const&, WebCore::LayoutUnit const&)
    (__hi=<optimized out>, __lo=<optimized out>, __val=<optimized out>)
    at /usr/include/c++/12.1.0/bits/stl_algo.h:3621
#7  WebCore::RenderReplaced::computeIntrinsicSizesConstrainedByTransferredMinMaxSizes(WebCore::RenderBox*, WebCore::FloatSize&, WebCore::FloatSize&) const
     (this=this@entry=0x7f3811112880, intrinsicSize=..., intrinsicRatio=..., contentRenderer=0x0)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderReplaced.cpp:460
#8  0x00007f38ae096dfa in WebCore::RenderReplaced::computeIntrinsicSizesConstrainedByTransferredMinMaxSizes(WebCore::RenderBox*, WebCore::FloatSize&, WebCore::FloatSize&) const
    (intrinsicRatio=..., intrinsicSize=..., contentRenderer=0x0, this=0x7f3811112880)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderReplaced.cpp:442
#9  WebCore::RenderReplaced::computeReplacedLogicalWidth(WebCore::ShouldComputePreferred) const
    (this=0x7f3811112880, shouldComputePreferred=WebCore::ComputePreferred)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderReplaced.cpp:588
#10 0x00007f38ae093efe in WebCore::RenderReplaced::computePreferredLogicalWidths() (this=0x7f3811112880)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderReplaced.cpp:706

Full backtrace attached.

[Michael Catanzaro]

Hi Sammy, can you please reorder your emails in metadata/contributors.json? The first email listed there is the one that Bugzilla's CC list will autocomplete so your Bugzilla email should be listed first. Currently, trying to CC you results in an error. (You can try it yourself on a different bug and see.) Just swapping the lines should fix that.

[Michael Catanzaro]

Created attachment 464109 [details]
gdb.txt

[Sammy Gill]

Hi Michael, Yes I will fix that issue with the contributors.json file! 

For the regression, could you help me get a reduced test case on that page? I tried navigating to that page (https://gitlab.com/gnutls/gnutls/) on a build that is fairly close to trunk, but it did not seem to reproduce for me. I am assuming that you are just navigating to the page and it crashes? I'll continue try to to reproduce the issue and look into the logic, but a test file or archive would definitely help!

[Radar WebKit Bug Importer]

<rdar://problem/103537352>

[Michael Catanzaro]

Well sadly I just loaded https://gitlab.com/gnutls/gnutls and this time it didn't crash.

I swear it was 100% reproducible a couple hours ago:

Mon 2022-12-19 17:40:00 CST  129533 1000 1000 SIGABRT present  /usr/libexec/webkitgtk-6.0/WebKitWebProcess           >
Mon 2022-12-19 17:40:17 CST  129687 1000 1000 SIGABRT present  /usr/libexec/webkitgtk-6.0/WebKitWebProcess           >
Mon 2022-12-19 17:40:39 CST  130002 1000 1000 SIGABRT present  /usr/libexec/webkitgtk-6.0/WebKitWebProcess           >
Mon 2022-12-19 17:41:10 CST  130388 1000 1000 SIGABRT present  /usr/libexec/webkitgtk-6.0/WebKitWebProcess           >
Mon 2022-12-19 17:58:59 CST  133877 1000 1000 SIGABRT present  /usr/libexec/webkitgtk-6.0/WebKitWebProcess

So it crashed five times in a row over the span of 20 minutes. Who knows what changed....

[Michael Catanzaro]

Eh, now it crashed twice more when I refreshed the page. So I guess it *usually* crashes, but not quite 100%.

[Sammy Gill]

I think style().logicalMinHeight()  and style().logicalMaxHeight()  may be useful inside of the RenderReplaced::computeIntrinsicSizesConstrainedByTransferredMinMaxSizes call.


I would also be curious what blockMinSize and blockMaxSize are being computed to inside RenderBox::computeMinMaxLogicalWidthFromAspectRatio  which gets called from that same RenderReplaced::computeIntrinsicSizesConstrainedByTransferredMinMaxSizes method

[Brent Fulgham]

I added a WebKit assertion, and I see it now:

Thread 0 Crashed::  Dispatch queue: com.apple.main-thread
0   WebCore                       	       0x144cc2244 WTFCrashWithInfo(int, char const*, char const*, int) + 36 (Assertions.h:754)
1   WebCore                       	       0x14a6fb248 WebCore::RenderReplaced::computeIntrinsicSizesConstrainedByTransferredMinMaxSizes(WebCore::RenderBox*, WebCore::FloatSize&, WebCore::FloatSize&) const + 1140 (RenderReplaced.cpp:460)
2   WebCore                       	       0x14a6fca04 WebCore::RenderReplaced::computeReplacedLogicalWidth(WebCore::ShouldComputePreferred) const + 688 (RenderReplaced.cpp:590)
3   WebCore                       	       0x14a5c2080 WebCore::RenderImage::computeReplacedLogicalWidth(WebCore::ShouldComputePreferred) const + 216 (RenderImage.cpp:274)


Our C++ library apparently doesn't include this assertion (or perhaps our build rules don't use the asserting version of the library).

[Sammy Gill]

Pull request: https://github.com/WebKit/WebKit/pull/7946

[Sammy Gill]

Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/37617

[EWS]

Committed 258210@main (28c5d0e82b0f): <https://commits.webkit.org/258210@main>

Reviewed commits have been landed. Closing PR #7946 and removing active labels.