Fix crash caused by arithmetic underflow in layout2020 #30897
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
🔨 Triggering try run (#7273542356) with platforms=linux,macos,windows and layout=all |
|
Test results for linux-wpt-layout-2013 from try job (#7273542356): Flaky unexpected result (17)
Stable unexpected results that are known to be intermittent (18)
|
|
Test results for linux-wpt-layout-2020 from try job (#7273542356): Flaky unexpected result (15)
Stable unexpected results that are known to be intermittent (14)
|
|
✨ Try run (#7273542356) succeeded. |
|
🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#43745) with upstreamable changes. |
|
✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#43745) title and body. |
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#43745). |
mrobinson
reviewed
Dec 20, 2023
Comment on lines
182
to
185
| self.justification_opportunities = self | ||
| .justification_opportunities | ||
| .checked_sub(spaces_trimmed) | ||
| .unwrap_or(0); |
There was a problem hiding this comment.
I think this might be papering over the issue. We shouldn't be subtracting more justification opportunities than we counted up to this point.
|
Looks like the issue is that when processing the last line, I zeroed out the number of justification opportunities. That's not going to work when subtracting later in diff --git a/components/layout_2020/flow/inline.rs b/components/layout_2020/flow/inline.rs
index 2b99cd63cd..ca5360469a 100644
--- a/components/layout_2020/flow/inline.rs
+++ b/components/layout_2020/flow/inline.rs
@@ -535,23 +535,23 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
// there is no line wrapping, so this forces the segment into the current line.
self.commit_current_segment_to_line();
- // This has the effect of preventing the application of `text-align: justify` to
- // this line because no justification opportunities means no justification.
- self.current_line.justification_opportunities = 0;
-
// Finally we finish the line itself and convert all of the LineItems into
// fragments.
- self.finish_current_line_and_reset();
+ self.finish_current_line_and_reset(true /* last_line */);
}
/// Finish layout of all inline boxes for the current line. This will gather all
/// [`LineItem`]s and turn them into [`Fragment`]s, then reset the
/// [`InlineFormattingContextState`] preparing it for laying out a new line.
- fn finish_current_line_and_reset(&mut self) {
+ fn finish_current_line_and_reset(&mut self, last_line: bool) {
let whitespace_trimmed = self.current_line.trim_trailing_whitespace();
- let (inline_start_position, justification_adjustment) = self
+ let (inline_start_position, mut justification_adjustment) = self
.calculate_current_line_inline_start_and_justification_adjustment(whitespace_trimmed);
+ if last_line {
+ justification_adjustment = Length::zero();
+ }
+
let block_start_position = self
.current_line
.line_block_start_considering_placement_among_floats();
@@ -1029,7 +1029,7 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
&self.current_line,
self.inline_box_state_stack.len(),
);
- self.finish_current_line_and_reset();
+ self.finish_current_line_and_reset(false /* last_line */);
}
/// Process a soft wrap opportunity. This will either commit the current unbreakble |
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#43745). |
mrobinson
approved these changes
Dec 21, 2023
|
🤖 This change no longer contains upstreamable changes to WPT; closed existing upstream pull request (web-platform-tests/wpt#43745). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently we are trying to subtract a non-zero number from 0 in
self.justification_opportunities -= spaces_trimmed, which is resulting in an arithmetic underflow in thetrim_trailing_whitespaceinsideinline.rs, hence the crash.This PR prevents the underflow by using
checked_subfor safe subtraction../mach build -ddoes not report any errors./mach test-tidydoes not report any errors