fix: correct int/int32_t pointer mismatches for -Wincompatible-pointer-types - #1657
Merged
bnoordhuis merged 1 commit intoSep 3, 2026
Merged
Conversation
…r-types Six call sites pass an int* where int32_t* is expected (or the reverse) in find_line_num, js_parseInt, remainingElementsCount_add, js_promise_all_resolve_element, and js_atomics_notify. Both types are 32-bit signed everywhere this project targets, so there is no behavioral change, but on any target where stdint.h defines int32_t as long int (ESP-IDF/ESP32 since v5.0), GCC 14+ treats the mismatch as a hard -Wincompatible-pointer-types error and the file fails to build. Retype the five local variables to match the callee signature already used at every other call site.
Contributor
|
Drop the plan |
Contributor
|
ping @SomSamantray |
SomSamantray
force-pushed
the
fix/int32-pointer-mismatch
branch
from
September 3, 2026 02:21
316e14e to
335ffae
Compare
Contributor
Author
|
Dropped the plan doc from the branch (rebased it out) and pushed — the PR is now scoped to the single |
Contributor
Author
Done — rebased the branch to drop that commit entirely. The PR is now scoped to the single |
Contributor
Author
Responded — see the comment above. Plan doc dropped, fix re-verified (clean build, |
fantomc0der
added a commit
to fantomc0der/esp32-ui-lab
that referenced
this pull request
Sep 19, 2026
## Why The vendored QuickJS-ng carried one local patch: five one-line type fixes in `quickjs.c` where a local's type didn't match the pointer its callee takes. On the Xtensa toolchain `int32_t` is `long int`, so GCC 14 rejects those as incompatible pointer types. Upstream has now made the identical five changes in [quickjs-ng#1657](quickjs-ng/quickjs#1657) (commit `d8e1cc6`, fixing issue #1624), and they first ship in **v0.17.0**. The patch has nothing left to do. ## What changed - `firmware/quickjs-ng/src/` is re-vendored at v0.17.0 (`6d46d07`) and is byte-identical to upstream, with no local modifications. - `firmware/quickjs-ng/patches/` is deleted. - `tools/vendor-quickjs.ps1` loses the `git am` / rebase / `format-patch` replay (and its "no patches found" guard, which was written to be deleted on this exact day). It now clones, checks out the target, checks the manifest, copies the files and updates the pin. The explicit 18-file manifest is unchanged, and v0.17.0 needs no new headers. - The CI doc-link check no longer needs `.patch` in its extension list, so that allowance (added in #32) is reverted. - Docs: `docs/engine-notes.md` Trap 3 is now a short history note (what the mismatch was, that we carried a patch through v0.15.1, and where upstream fixed it). The remaining references in `CLAUDE.md`, `docs/README.md`, `docs/design-rationale.md`, `docs/portability.md`, `firmware/README.md`, `firmware/quickjs-ng/README.md` and the host-test CMake comment now describe an unmodified engine. `tools/lv-mock.mjs` reports the new version. Why keep vendoring at all: QuickJS-ng isn't in the Arduino library index, and upstream's root can't be used as a library folder as-is because it also holds `quickjs-libc.c`, `qjs.c` and others. Only the patch was ever a workaround. ## Verification Off-hardware: - `. lash.ps1 -BuildOnly` builds cleanly with the esp32 3.3.11 core (xtensa GCC 14.2), so there are no incompatible-pointer errors. The sketch is 2,059,086 bytes (65%). - Host build plus ASan (`ctest`, in an ubuntu:24.04 container): 4/4 pass. - `check-js-api`, `test-check-js-api`, `test-jsx`, `test-ui`, `test-parity`, and the doc link check all pass. - Re-running `vendor-quickjs.ps1` against the pinned SHA leaves the tree unchanged. On the board (ESP32-S3-Touch-LCD-1.47, flashed from this branch; `sys.info().quickjs` reports `0.17.0`): - `app/selftest.js`: **84 passed, 0 failed**. - `app/ui-selftest.js`: **19 passed, 0 failed**. - An ad-hoc engine probe (46 checks) passes 45. It covers the five retyped sites (`parseInt` radixes, line numbers in `Error.stack`, `Promise.all` ordering/rejection/100 items, `Atomics.notify`), plus regex (named groups, lookbehind, `\p{}`, sticky, `matchAll`), case mapping and normalization, dtoa formatting, BigInt, classes, Proxy, a 20k-element string join (the `usable_size` trap) and a 20k-entry Map. The one failure is pre-existing and described below. - The launcher and all four apps (tasks, vitals, wifi, weather) evaluate cleanly in 169 to 261 ms. - There's no leak across 15 reloads: PSRAM is identical and internal RAM is within 300 bytes. - Runaway recursion still throws a catchable `RangeError` at the 20 KB stack cap. It gets 50 frames deep here versus 54 on v0.15.1. **Cost:** standing up the VM now takes **155,516 bytes vs 91,292** on v0.15.1. The difference is upstream's new small-block arena allocator (`9de2921`), which carves 4 KB arenas per size class. It comes out of PSRAM (8 MB); free internal RAM is unchanged at about 143.5 KB. The sample boot log in `docs/build-and-deploy.md` is updated to match. ### Pre-existing bug found (not introduced here) `for...of` over a string, `[...str]` and `Array.from(str)` never advance past the first BMP character on this board. They loop forever, which ends in either an OOM or a hung VM. **v0.15.1 on `main` behaves identically**, and upstream v0.17.0 built for x86 (64- and 32-bit, `-O2` and `-Os`) is correct. The cause is in `js_string_iterator_next`: `idx` is `uint32_t`, passed as `string_getc(p, (int *)&idx)`. On Xtensa, `uint32_t` is `unsigned long`, so under strict aliasing GCC treats the write as unable to change `idx` and deletes `it->idx = idx`. The generated assembly confirms the store is gone, and it reappears with `-fno-strict-aliasing`. It's the same `int32_t`-is-`long` root cause as the patch this PR drops, except the explicit cast hides the diagnostic. No shipped app iterates over a string, so nothing on the panel is affected today. The fix belongs in a follow-up, not in this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
social4hyq
pushed a commit
to social4hyq/homebrew-core
that referenced
this pull request
Sep 20, 2026
quickjs-ng 0.17.0 Created-by: HarmonybrewBot Commit-by: HarmonybrewBot Merged-by: HarmonybrewBot Description: Created by `brew bump` --- Created with `brew bump-formula-pr`.<details> <summary>release notes</summary> <pre>## What's Changed * update projects.md to include scriptc project reference (from the Ver… by @lukasMega in quickjs-ng/quickjs#1677 * Fixes segfault from #1661 by @sr5434 in quickjs-ng/quickjs#1675 * docs: add Qbs to projects by @ABBAPOH in quickjs-ng/quickjs#1685 * Add Big Endian architecture support instructions by @saghul in quickjs-ng/quickjs#1681 * Conditional BUILD_DIR by @kapouer in quickjs-ng/quickjs#1686 * Fix String.prototype.normalize() crash from #1683 by @sr5434 in quickjs-ng/quickjs#1688 * Fix reference count bug in Promise.withResolvers by @bnoordhuis in quickjs-ng/quickjs#1692 * Enable test262 host-gc-required feature by @bnoordhuis in quickjs-ng/quickjs#1696 * docs: add Vayu to projects by @athrvk in quickjs-ng/quickjs#1702 * Use mirror for Alpine GHA by @bnoordhuis in quickjs-ng/quickjs#1705 * Fix Iterator.from GetIteratorDirect handling by @jiang1997 in quickjs-ng/quickjs#1706 * Link against libm unconditionally on Unices by @bnoordhuis in quickjs-ng/quickjs#1699 * Fix out-of-bound read in JS_NewTypedArray by @bnoordhuis in quickjs-ng/quickjs#1704 * Preserve old interrupt handler in std.evalScript by @bnoordhuis in quickjs-ng/quickjs#1707 * fix: correct int/int32_t pointer mismatches for -Wincompatible-pointer-types by @SomSamantray in quickjs-ng/quickjs#1657 * Fix JS_FreeCStringUTF16 on slice strings by @bnoordhuis in quickjs-ng/quickjs#1709 * Add react-native-quickjs project to documentation by @saghul in quickjs-ng/quickjs#1710 * Move JSClass out of quickjs.h by @bnoordhuis in quickjs-ng/quickjs#1713 * Optimize JS_DeleteGlobalVar by @bnoordhuis in quickjs-ng/quickjs#1714 * Throw error when converting BigIntArray to Int8Array (#1690) by @sr5434 in quickjs-ng/quickjs#1694 * Fix power-of-two integer hash collisions by @bnoordhuis in quickjs-ng/quickjs#1720 * Store constant atoms more compactly in wire format by @bnoordhuis in quickjs-ng/quickjs#1719 * Prevent Object.preventExtensions when a TypedArray has a resizable ArrayBuffer by @sr5434 in quickjs-ng/quickjs#1695 * Make DOMException a bit more conformant by @goffrie in quickjs-ng/quickjs#1721 * Fix TypedArray.prototype.at() to handle resized ArrayBuffer correctly by @sr5434 in quickjs-ng/quickjs#1718 * Fix JS_SetMemoryLimit byte accounting on WASI by @josefguenther in quickjs-ng/quickjs#1711 * Fix(wasi): enable configurable stack overflow protection by @aayush-kapoor in quickjs-ng/quickjs#1700 * Use a bigger initial atom hash table by @bnoordhuis in quickjs-ng/quickjs#1723 * Add private symbols by @bnoordhuis in quickjs-ng/quickjs#1725 * Peephole-optimize negation + conditional jump by @bnoordhuis in quickjs-ng/quickjs#1726 * Check for property deletion when accessing in `with` by @sr5434 in quickjs-ng/quickjs#1722 ## New Contributors * @lukasMega made their first contribution in quickjs-ng/quickjs#1677 * @kapouer made their first contribution in quickjs-ng/quickjs#1686 * @athrvk made their first contribution in quickjs-ng/quickjs#1702 * @SomSamantray made their first contribution in quickjs-ng/quickjs#1657 * @goffrie made their first contribution in quickjs-ng/quickjs#1721 * @josefguenther made their first contribution in quickjs-ng/quickjs#1711 * @aayush-kapoor made their first contribution in quickjs-ng/quickjs#1700 **Full Changelog**: https://github.com/quickjs-ng/quickjs/compare/v0.16.2...v0.17.0</pre> <p>View the full release notes at <a href="https://github.com/quickjs-ng/quickjs/releases/tag/v0.17.0">https://github.com/quickjs-ng/quickjs/releases/tag/v0.17.0</a>.</p> </details> <hr> See merge request: Harmonybrew/homebrew-core!20596
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
On any target whose
stdint.hdefinesint32_taslong intrather thanint— every ESP-IDF/ESP32 chip since v5.0 —quickjs.cfails to compile under GCC 14+, because-Wincompatible-pointer-typesis now an error by default there. Six call sites across five local variables pass anint *where anint32_t *is expected, or the reverse.Both types are 32-bit signed on every platform this project targets, so there's no behavioral difference — this retypes the five local variables (
find_line_num'sv,js_parseInt'sradix,remainingElementsCount_add'sremainingElementsCount,js_promise_all_resolve_element'sindex, andjs_atomics_notify'scount) to match the callee signature each is already used against everywhere else in the file. No callee signatures changed.Verified locally without ESP32 hardware or a GCC 14 install: shimming
int32_tto a distinct-but-same-width type and compiling withclang -std=gnu11 -Werror=incompatible-pointer-typesreproduces the exact 6 errors at the exact reported lines before the fix, and compiles clean after. Also confirmed the normal (unshimmed) build still succeeds,api-testpasses, and manually exercisedparseInt('ff', 16)andAtomics.notifyto confirm unchanged behavior.Fixes #1624