Skip to content

REGRESSION (279702@main): Incorrect use of sizeof() in checkJSStringOOBUTF8() from testapi.c #47245

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
Merged
merged 1 commit into from
Jun 27, 2025
Merged

REGRESSION (279702@main): Incorrect use of sizeof() in checkJSStringOOBUTF8() from testapi.c #47245

merged 1 commit into from
Jun 27, 2025
+37 −28

Conversation

ddkilzer
Copy link
Contributor

@ddkilzer ddkilzer commented Jun 26, 2025 •
edited by webkit-early-warning-system
Loading

8f23ef0

REGRESSION (279702@main): Incorrect use of sizeof() in checkJSStringOOBUTF8() from testapi.c
<https://bugs.webkit.org/show_bug.cgi?id=295037>
<rdar://154311531>

Reviewed by Yusuke Suzuki.

Revert 279702@main and add a separate cStringSize variable for testing.

* Source/JavaScriptCore/API/tests/testapi.c:
(checkJSStringOOBUTF8):
(checkJSStringOOBUTF16):
(checkJSStringOOBUTF16AtEnd):
- Switch back to stack-allocated buffers by reverting 279702@main.
- Add IGNORE_WARNINGS_BEGIN("vla")/IGNORE_WARNINGS_END macros to avoid
  warnings in open source clang for the SaferCPP EWS bot.
- Add separate cStringSize variable so outCStringSize matches the size
  of the buffer.

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

8ddd7f1

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@ddkilzer ddkilzer requested a review from a team as a code owner June 26, 2025 17:03
@ddkilzer ddkilzer self-assigned this Jun 26, 2025
@ddkilzer ddkilzer added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label Jun 26, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 26, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 042f715)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
  🛠 🧪 unsafe-merge ✅ 🛠 tv ❌ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@ddkilzer ddkilzer added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing labels Jun 26, 2025
@ddkilzer
Copy link
Contributor Author

Hitting -Wvla warnings-as-errors on the SaferCPP bot:

/Volumes/Data/worker/macOS-Safer-CPP-Checks-EWS/build/Source/JavaScriptCore/API/tests/testapi.c:1198:24: error: variable
 length array used [-Werror,-Wvla]
 1198 |     char sourceCString[sourceCStringSize];
      |                        ^~~~~~~~~~~~~~~~~
/Volumes/Data/worker/macOS-Safer-CPP-Checks-EWS/build/Source/JavaScriptCore/API/tests/testapi.c:1203:21: error: variable
 length array used [-Werror,-Wvla]
 1203 |     char outCString[outCStringSize];
      |                     ^~~~~~~~~~~~~~
/Volumes/Data/worker/macOS-Safer-CPP-Checks-EWS/build/Source/JavaScriptCore/API/tests/testapi.c:1229:24: error: variable
 length array used [-Werror,-Wvla]
 1229 |     char sourceCString[sourceCStringSize];
      |                        ^~~~~~~~~~~~~~~~~
/Volumes/Data/worker/macOS-Safer-CPP-Checks-EWS/build/Source/JavaScriptCore/API/tests/testapi.c:1239:21: error: variable
 length array used [-Werror,-Wvla]
 1239 |     char outCString[outCStringSize];
      |                     ^~~~~~~~~~~~~~
/Volumes/Data/worker/macOS-Safer-CPP-Checks-EWS/build/Source/JavaScriptCore/API/tests/testapi.c:1265:24: error: variable
 length array used [-Werror,-Wvla]
 1265 |     char sourceCString[sourceCStringSize];
      |                        ^~~~~~~~~~~~~~~~~
/Volumes/Data/worker/macOS-Safer-CPP-Checks-EWS/build/Source/JavaScriptCore/API/tests/testapi.c:1275:21: error: variable length array used [-Werror,-Wvla]
 1275 |     char outCString[outCStringSize];
      |                     ^~~~~~~~~~~~~~
6 errors generated.

@ddkilzer ddkilzer force-pushed the eng/REGRESSION-279702-main-Incorrect-use-of-sizeof-in-checkJSStringOOBUTF8-from-testapi-c branch from 042f715 to 8ddd7f1 Compare June 26, 2025 22:25
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 26, 2025 •
edited
Loading

EWS run on current version of this PR (hash 8ddd7f1)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@ddkilzer ddkilzer added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 27, 2025
@ddkilzer
REGRESSION (279702@main): Incorrect use of sizeof() in checkJSStringO…
8f23ef0 
…OBUTF8() from testapi.c

<https://bugs.webkit.org/show_bug.cgi?id=295037>
<rdar://154311531>

Reviewed by Yusuke Suzuki.

Revert 279702@main and add a separate cStringSize variable for testing.

* Source/JavaScriptCore/API/tests/testapi.c:
(checkJSStringOOBUTF8):
(checkJSStringOOBUTF16):
(checkJSStringOOBUTF16AtEnd):
- Switch back to stack-allocated buffers by reverting 279702@main.
- Add IGNORE_WARNINGS_BEGIN("vla")/IGNORE_WARNINGS_END macros to avoid
  warnings in open source clang for the SaferCPP EWS bot.
- Add separate cStringSize variable so outCStringSize matches the size
  of the buffer.

Canonical link: https://commits.webkit.org/296699@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/REGRESSION-279702-main-Incorrect-use-of-sizeof-in-checkJSStringOOBUTF8-from-testapi-c branch from 8ddd7f1 to 8f23ef0 Compare June 27, 2025 01:48
@webkit-commit-queue
Copy link
Collaborator

Committed 296699@main (8f23ef0): https://commits.webkit.org/296699@main

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

@webkit-commit-queue webkit-commit-queue merged commit 8f23ef0 into WebKit:main Jun 27, 2025
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 27, 2025
@ddkilzer ddkilzer deleted the eng/REGRESSION-279702-main-Incorrect-use-of-sizeof-in-checkJSStringOOBUTF8-from-testapi-c branch June 27, 2025 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Constellation Constellation Constellation approved these changes

Assignees

@ddkilzer ddkilzer

Labels
JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ddkilzer @webkit-early-warning-system @webkit-commit-queue @Constellation