Bug 254750 - Return value of LineBoxBuilder::lineContent is not a reference
Summary: Return value of LineBoxBuilder::lineContent is not a reference
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-03-30 10:20 PDT by Michael Catanzaro
Modified: 2023-03-30 14:47 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.57 KB, patch)
2023-03-30 11:00 PDT, zalan
no flags Details | Formatted Diff | Diff
[fast-cq]Patch (1.56 KB, patch)
2023-03-30 14:31 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2023-03-30 10:20:57 PDT
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.
Comment 1 zalan 2023-03-30 10:24:31 PDT
hah, indeed. That's very silly. Thank you! Let me quickly fix that.
Comment 2 zalan 2023-03-30 11:00:56 PDT
Created attachment 465683 [details]
Patch
Comment 3 zalan 2023-03-30 14:31:57 PDT
Created attachment 465691 [details]
[fast-cq]Patch
Comment 4 EWS 2023-03-30 14:38:26 PDT
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].
Comment 5 Radar WebKit Bug Importer 2023-03-30 14:39:21 PDT
<rdar://problem/107440230>
Comment 6 zalan 2023-03-30 14:47:12 PDT
Thanks again, Michael!