Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WGSL] Type::size can still overflow #29068

Merged
merged 1 commit into from
May 27, 2024
Merged

[WGSL] Type::size can still overflow #29068

merged 1 commit into from
May 27, 2024

Conversation

tadeuzagallo
Copy link
Member

@tadeuzagallo tadeuzagallo commented May 24, 2024 •
edited by webkit-early-warning-system
Loading

2d30b56

[WGSL] Type::size can still overflow
https://bugs.webkit.org/show_bug.cgi?id=274669
rdar://128677742

Reviewed by Mike Wyrzykowski.

In 279204@main I added checks for overflow in Type::size, but I missed the case where
`array.element->size()` returns uint_max, and rounding it up to the alignment returns 0.

* LayoutTests/fast/webgpu/fuzz-128677742-expected.txt: Added.
* LayoutTests/fast/webgpu/fuzz-128677742.html: Added.
* Source/WebGPU/WGSL/Types.cpp:
(WGSL::Type::size const):

Canonical link: https://commits.webkit.org/279343@main

f0c56f7

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@tadeuzagallo tadeuzagallo self-assigned this May 24, 2024
@tadeuzagallo tadeuzagallo added the WebGPU For bugs in WebGPU label May 24, 2024
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 24, 2024 •
edited
Loading

EWS run on current version of this PR (hash f0c56f7)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

mwyrzykowski
mwyrzykowski reviewed May 24, 2024
View reviewed changes
Source/WebGPU/WGSL/Types.cpp
auto stride = WTF::roundUpToMultipleOf(array.element->alignment(), elementSize);
if (stride < elementSize)
return std::numeric_limits<unsigned>::max();
size *= stride;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this run the risk of overflow?

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 24, 2024
@tadeuzagallo tadeuzagallo added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels May 27, 2024
@tadeuzagallo
[WGSL] Type::size can still overflow
2d30b56 
https://bugs.webkit.org/show_bug.cgi?id=274669
rdar://128677742

Reviewed by Mike Wyrzykowski.

In 279204@main I added checks for overflow in Type::size, but I missed the case where
`array.element->size()` returns uint_max, and rounding it up to the alignment returns 0.

* LayoutTests/fast/webgpu/fuzz-128677742-expected.txt: Added.
* LayoutTests/fast/webgpu/fuzz-128677742.html: Added.
* Source/WebGPU/WGSL/Types.cpp:
(WGSL::Type::size const):

Canonical link: https://commits.webkit.org/279343@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/WGSL-Typesize-can-still-overflow branch from f0c56f7 to 2d30b56 Compare May 27, 2024 10:16
@webkit-commit-queue
Copy link
Collaborator

Committed 279343@main (2d30b56): https://commits.webkit.org/279343@main

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

@webkit-commit-queue webkit-commit-queue merged commit 2d30b56 into WebKit:main May 27, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwyrzykowski mwyrzykowski mwyrzykowski approved these changes

@djg djg Awaiting requested review from djg djg is a code owner

Assignees

@tadeuzagallo tadeuzagallo

Labels
WebGPU For bugs in WebGPU
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tadeuzagallo @webkit-early-warning-system @webkit-commit-queue @mwyrzykowski @webkit-ews-buildbot