Skip to content

Commit 77a2bad

Browse files
authored
Don't include internal compiler settings with RETAIN_COMPILER_SETTINGS (#25667)
This is technically a breaking change so I mentioned it in the ChangeLog. Fixes: #25663
1 parent 9204ca2 commit 77a2bad

File tree

6 files changed

+31
-14
lines changed

6 files changed

+31
-14
lines changed

ChangeLog.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@ See docs/process.md for more on how version tagging works.
2020

2121
4.0.19 (in development)
2222
-----------------------
23+
- The `RETAIN_COMPILER_SETTINGS` setting and the corresponding
24+
`emscripten_get_compiler_setting` API no longer store or report internal
25+
compiler settings (those listed in `setttings_internal.js`). We made an
26+
exception here for `EMSCRIPTEN_VERSION` which is the only internal setting
27+
where we could find usage of `emscripten_get_compiler_setting` (in a global
28+
GitHub search). (#25667)
2329

2430
4.0.18 - 10/24/25
2531
-----------------

src/parseTools.mjs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -806,17 +806,8 @@ function addAtPostRun(code) {
806806

807807
function makeRetainedCompilerSettings() {
808808
const ret = {};
809-
for (const [name, value] of Object.entries(global)) {
810-
if (name[0] !== '_' && name == name.toUpperCase()) {
811-
if (
812-
typeof value == 'number' ||
813-
typeof value == 'boolean' ||
814-
typeof value == 'string' ||
815-
Array.isArray(name)
816-
) {
817-
ret[name] = value;
818-
}
819-
}
809+
for (const name of PUBLIC_SETTINGS) {
810+
ret[name] = globalThis[name];
820811
}
821812
return ret;
822813
}

src/settings_internal.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,3 +278,6 @@ var OUTPUT_FORMAT = '';
278278
var LOAD_SOURCE_MAP = false;
279279

280280
var ALIASES = [];
281+
282+
// List of public setting names (Used by RETAIN_COMPILER_SETTINGS)
283+
var PUBLIC_SETTINGS = [];

test/core/emscripten_get_compiler_setting.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,25 @@
55
* found in the LICENSE file.
66
*/
77

8-
#include <stdio.h>
98
#include <assert.h>
109
#include <emscripten.h>
10+
#include <stdio.h>
11+
#include <string.h>
1112

1213
int main() {
14+
// Test boolean, int, and string settings
1315
printf("INVOKE_RUN: %ld\n", emscripten_get_compiler_setting("INVOKE_RUN"));
14-
assert((unsigned)emscripten_get_compiler_setting("OPT_LEVEL") <= 3);
15-
assert((unsigned)emscripten_get_compiler_setting("DEBUG_LEVEL") <= 4);
16+
assert((unsigned)emscripten_get_compiler_setting("ASSERTIONS") <= 2);
1617
printf("CLOSURE_WARNINGS: %s\n", (char*)emscripten_get_compiler_setting("CLOSURE_WARNINGS"));
18+
19+
// Internal setting should not be visible.
20+
const char* embind = (char*)emscripten_get_compiler_setting("EMBIND");
21+
printf("EMBIND: %s\n", embind);
22+
assert(strstr(embind, "invalid compiler setting") != NULL);
23+
24+
// EMSCRIPTEN_VERSION should be allowed though..
25+
const char* version = (char*)emscripten_get_compiler_setting("EMSCRIPTEN_VERSION");
26+
printf("EMSCRIPTEN_VERSION: %s\n", version);
27+
assert(strstr(version, "invalid compiler setting") == NULL);
1728
}
1829

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
INVOKE_RUN: 1
22
CLOSURE_WARNINGS: quiet
3+
EMBIND: invalid compiler setting: EMBIND

tools/link.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1793,6 +1793,11 @@ def get_full_import_name(name):
17931793
'emscripten_stack_get_end']
17941794

17951795
settings.EMSCRIPTEN_VERSION = utils.EMSCRIPTEN_VERSION
1796+
if settings.RETAIN_COMPILER_SETTINGS:
1797+
settings.PUBLIC_SETTINGS = [k for k in settings.attrs.keys() if k not in settings.internal_settings]
1798+
# Also include EMSCRIPTEN_VERSION from the internal settings since there are
1799+
# known usage of this with `emscripten_get_compiler_setting`.
1800+
settings.PUBLIC_SETTINGS.append('EMSCRIPTEN_VERSION')
17961801
settings.SOURCE_MAP_BASE = options.source_map_base or ''
17971802

17981803
settings.LINK_AS_CXX = (shared.run_via_emxx or settings.DEFAULT_TO_CXX) and not options.nostdlibxx

0 commit comments

Comments
 (0)