[runtime] Fix warnings by mbouaziz · Pull Request #138 · SkipLabs/skip

mbouaziz · 2024-03-04T12:32:21Z

In #135, @jberdine found that we used pointer arithmetic on void. This could have been avoided by enabling compiler warnings. So let's do it and fix a few more oddities.

I've run

set -a; SKARGO_PROFILE=dev; STAGE=2; PATH=$PWD/compiler/stage$STAGE/bin:$PATH; set +a; make clean && make -C compiler clean test && make test

as suggested by @jberdine

My VSCode is configured with format-on-save and uses the standard Microsoft C/C++ Extension Pack. The changes looks legit. It should probably be enforced by the CI, not sure it's worth it at this time.

Apparently the noreturn attribute is not enough to silence this warning.

but I didn't get rid of them completely (add -Wsign-conversion if you want to see them)

jberdine

Overall LGTM. There is one change noted inline that I don't know why the warning isn't pointing to a bug. It would be good to understand that before silencing the warning.

jberdine · 2024-03-04T13:31:54Z

@@ -79,14 +79,15 @@ void sk_obstack_attach_page(char* lpage) {
 }


I can't comment on the commit summary, but does "My VSCode is configured with format-on-save and uses the standard Microsoft C/C++ Extension Pack." mean that it runs clang-format, or does its own thing?

I was too lazy to check...
Looks like it's indeed clang-format: https://code.visualstudio.com/docs/cpp/cpp-ide#_code-formatting and it's using the .clang-format file already present.

gregsexton · 2024-03-05T10:31:39Z

@@ -1,8 +1,12 @@
 MEMSIZE32=1073741824

+COMMONFLAGS=-Wall -Wextra -Wno-sign-conversion -Wno-sometimes-uninitialized -Wno-c2x-extensions -Wsign-compare -Wextra-semi-stmt


I would probably throw in -Wconversion too. That tends to catch a lot of useful things.

👍

I actually did -Weverything and looked at warnings that appeared and could be worth fixing later.

gregsexton · 2024-03-05T10:33:05Z

This is great! I've wanted to do this for a long time :D. If you've got cycles and fancy it: I was also thinking of throwing in ubsan and asan for debug builds.

mbouaziz · 2024-03-05T10:43:34Z

I was also thinking of throwing in ubsan and asan for debug builds.

Created https://intern.skiplabs.io/tasks/3bHdAtQiSR_wcnxJtJ9uX_XdsUC for it ;-)

gregsexton · 2024-03-05T10:45:19Z

Do we have -werror somewhere? Or are these all just warnings?

mbouaziz · 2024-03-05T13:08:15Z

They are just warnings for now. werror would be good if people agree.

bennostein · 2024-03-05T13:17:02Z

They are just warnings for now. werror would be good if people agree.

Bring on the -Werror!

jberdine · 2024-03-05T17:29:13Z

They are just warnings for now. werror would be good if people agree.

Bring on the -Werror!

At least for the release profile, but I have to manually set CXXFLAGS when iterating on the compiler anyhow so adding -Wno-error ought to work.

Follow-up of #138: - adds warnings to `runtime64_specific.cpp` that I forgot in #138 - minor fixes in `SKIP_open_file` and `SKIP_notify` - `-Werror`

Closes Dependabot alert #339 (GHSA-v6wh-96g9-6wx3 / CVE-2026-53632, medium — launch-editor NTLMv2 hash disclosure via UNC path handling on Windows). www had launch-editor@2.13.2 (vulnerable range <= 2.14.0). launch-editor is a transitive dep of webpack-dev-server (^2.6.1), so a plain `npm update launch-editor --package-lock-only` bumps it to 2.14.1 within range — no override needed. Single-package, 4-line diff. No breaking changes: 2.14.1 is the UNC-path security fix itself ("fix: reject UNC paths", #138); 2.14.0 adds package types and editor- path detection fixes (Trae casing, vscode path) — all additive. The public API (launch an editor) is unchanged. launch-editor is dev-only tooling — webpack-dev-server's click-to-open- in-editor overlay during `docusaurus start`, and the vuln is Windows/UNC-specific; it never runs in CI or the shipped static site. Closes #339. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

## Summary Closes Dependabot alert [#339](https://github.com/SkipLabs/skip/security/dependabot/339) — [GHSA-v6wh-96g9-6wx3](GHSA-v6wh-96g9-6wx3) / CVE-2026-53632 (**medium**: `launch-editor` NTLMv2 hash disclosure via UNC path handling on Windows). `www` had `launch-editor@2.13.2` (vulnerable range `<= 2.14.0`). ## Fix `launch-editor` is a transitive dep of `webpack-dev-server` (`^2.6.1`), so a plain `npm update launch-editor --package-lock-only` bumps it to **2.14.1** within range — no override needed. Single-package, 4-line diff. ## Breaking-change check Changelog 2.13.2 → 2.14.1: - **v2.14.1**: `fix: reject UNC paths` (#138) — the security fix itself — plus non-major dep bumps. - **v2.14.0**: `feat: add and improve package types` (additive), editor-path detection fixes (Trae casing, vscode path). No breaking changes; the public API (launch an editor) is unchanged. ## Verification - Fresh `npm install` → single `launch-editor@2.14.1`; no copy `<= 2.14.0`. - Diff scoped to the one package; no lockfile drift. `launch-editor` is dev-only tooling — `webpack-dev-server`'s click-to-open-in-editor overlay during `docusaurus start` — and the vuln is Windows/UNC-specific. It never runs in CI or reaches the shipped static site. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

mbouaziz added 11 commits March 4, 2024 08:50

[runtime] Autoformat

e36306c

My VSCode is configured with format-on-save and uses the standard Microsoft C/C++ Extension Pack. The changes looks legit. It should probably be enforced by the CI, not sure it's worth it at this time.

[runtime] Disallow void pointer arithmetic

6cb4229

[runtime] Disallow unused variables

9d3a920

[runtime] Trying to disallow uninitialized variables but failed

d30219c

Apparently the noreturn attribute is not enough to silence this warning.

[runtime] Enable all other warnings

2196cff

[runtime] Local context seems dead

13ced26

[runtime] Disallow unused parameters

d3ac1bd

[runtime] Disallow comparison of integers of different signs

68f84c4

[runtime] Reduce implicit sign conversions

dfa1f0d

but I didn't get rid of them completely (add -Wsign-conversion if you want to see them)

[runtime] Remove extra semi-colons

ed32c47

[runtime] Let's do other warnings later

ea02f7c

mbouaziz force-pushed the runtime-warnings branch from 9ffecdd to ea02f7c Compare March 4, 2024 12:32

mbouaziz mentioned this pull request Mar 1, 2024

[runtime] Type for obstack #135

Merged

jberdine reviewed Mar 4, 2024

View reviewed changes

[runtime] Another dead local context

9d83e0f

mbouaziz mentioned this pull request Mar 4, 2024

[skc] Why is 32-bit-specific runtime SKIP_genSym parameter not used? #141

Open

jberdine approved these changes Mar 4, 2024

View reviewed changes

mbouaziz merged commit 0ff7841 into SkipLabs:main Mar 4, 2024

mbouaziz deleted the runtime-warnings branch March 4, 2024 15:32

gregsexton reviewed Mar 5, 2024

View reviewed changes

mbouaziz mentioned this pull request Mar 7, 2024

[runtime] -Werror #148

Merged

mbouaziz added a commit that referenced this pull request Mar 8, 2024

[runtime] -Werror (#148)

ea4d9f0

Follow-up of #138: - adds warnings to `runtime64_specific.cpp` that I forgot in #138 - minor fixes in `SKIP_open_file` and `SKIP_notify` - `-Werror`

mbouaziz mentioned this pull request Jul 10, 2026

Bump launch-editor to 2.14.1 in www (Dependabot #339) #1317

Merged

		@@ -79,14 +79,15 @@ void sk_obstack_attach_page(char* lpage) {
		}

		@@ -1,8 +1,12 @@
		MEMSIZE32=1073741824

		COMMONFLAGS=-Wall -Wextra -Wno-sign-conversion -Wno-sometimes-uninitialized -Wno-c2x-extensions -Wsign-compare -Wextra-semi-stmt

Uh oh!

Conversation

mbouaziz commented Mar 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jberdine left a comment

Choose a reason for hiding this comment

Uh oh!

jberdine Mar 4, 2024

Choose a reason for hiding this comment

Uh oh!

mbouaziz Mar 4, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gregsexton Mar 5, 2024

Choose a reason for hiding this comment

Uh oh!

mbouaziz Mar 5, 2024

Choose a reason for hiding this comment

Uh oh!

gregsexton commented Mar 5, 2024

Uh oh!

mbouaziz commented Mar 5, 2024

Uh oh!

gregsexton commented Mar 5, 2024

Uh oh!

mbouaziz commented Mar 5, 2024

Uh oh!

bennostein commented Mar 5, 2024

Uh oh!

jberdine commented Mar 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mbouaziz commented Mar 4, 2024 •

edited

Loading