Bug 249617 - REGRESSION(257434@main): Crash in RenderReplaced::computeIntrinsicSizesConstrainedByTransferredMinMaxSizes on https://gitlab.com/gnutls/gnutls/
Summary: REGRESSION(257434@main): Crash in RenderReplaced::computeIntrinsicSizesConstr...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-12-19 16:17 PST by Michael Catanzaro
Modified: 2022-12-22 11:40 PST (History)
7 users (show)

See Also:


Attachments
gdb.txt (201.97 KB, text/plain)
2022-12-19 16:26 PST, Michael Catanzaro
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2022-12-19 16:17:36 PST
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.
Comment 1 Michael Catanzaro 2022-12-19 16:20:56 PST
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.
Comment 2 Michael Catanzaro 2022-12-19 16:26:48 PST
Created attachment 464109 [details]
gdb.txt
Comment 3 Sammy Gill 2022-12-19 17:12:38 PST
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!
Comment 4 Radar WebKit Bug Importer 2022-12-19 17:13:24 PST
<rdar://problem/103537352>
Comment 5 Michael Catanzaro 2022-12-19 18:14:25 PST
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....
Comment 6 Michael Catanzaro 2022-12-19 18:16:12 PST
Eh, now it crashed twice more when I refreshed the page. So I guess it *usually* crashes, but not quite 100%.
Comment 7 Sammy Gill 2022-12-19 18:53:19 PST
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
Comment 8 Brent Fulgham 2022-12-20 13:47:48 PST
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).
Comment 9 Sammy Gill 2022-12-20 17:50:16 PST
Pull request: https://github.com/WebKit/WebKit/pull/7946
Comment 10 Sammy Gill 2022-12-20 18:06:33 PST
Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/37617
Comment 11 EWS 2022-12-21 13:10:18 PST
Committed 258210@main (28c5d0e82b0f): <https://commits.webkit.org/258210@main>

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