| Summary: | Return value of LineBoxBuilder::lineContent is not a reference | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> | ||||||
| Component: | Layout and Rendering | Assignee: | zalan <zalan> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bfulgham, koivisto, simon.fraser, webkit-bug-importer, zalan | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=254758 | ||||||||
| Attachments: |
|
||||||||
hah, indeed. That's very silly. Thank you! Let me quickly fix that. Created attachment 465683 [details]
Patch
Created attachment 465691 [details]
[fast-cq]Patch
Committed 262366@main (eff08853244b): <https://commits.webkit.org/262366@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 465691 [details]. Thanks again, Michael! |
The return value of LineBoxBuilder::lineContent looks probably incorrect: const LineBuilder::LineContent lineContent() const { return m_lineContent; } If it was intended to return a copy, then it would surely return a non-const LineBuilder::LineContent, so I think this should probably be changed to: const LineBuilder::LineContent& lineContent() const { return m_lineContent; } But I have not checked to see if this change would be safe. I noticed due to this false-positive GCC 13 warning: In file included from /home/mcatanzaro/Projects/WebKit/WebKitBuild/gtk4/WebCore/DerivedSources/unified-sources/UnifiedSource-207b877e-5.cpp:2: /home/mcatanzaro/Projects/WebKit/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp: In member function ‘void WebCore::Layout::LineBoxBuilder::adjustOutsideListMarkersPosition(WebCore::Layout::LineBox&)’: /home/mcatanzaro/Projects/WebKit/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp:714:15: error: possibly dangling reference to a temporary [-Werror=dangling-reference] 714 | auto& listMarkerRun = lineContent().runs[listMarkerBoxIndex]; | ^~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp:714:68: note: the temporary was destroyed at the end of the full expression ‘(& WebCore::Layout::LineBoxBuilder::lineContent() const().WebCore::Layout::LineBuilder::LineContent::runs)->WTF::Vector<WebCore::Layout::Line::Run, 10>::operator[](listMarkerBoxIndex)’ 714 | auto& listMarkerRun = lineContent().runs[listMarkerBoxIndex]; | ^ It looks bad, but I don't think it's a real problem because although lineContent() is itself invalid after this statement, lineContent().runs[listMarkerBoxIndex] should still be valid.