Skip to content

Commit fa41a9f

Browse files
Consider unexpected undefined symbols to be errors (#86)
## Motivation for the change, related issues We currently live with the following undefined symbol warnings when linking with Emscripten: ``` #84 1.878 warning: undefined symbol: SharpYuvConvert (referenced by root reference (e.g. compiled C/C++ code)) #84 1.878 warning: undefined symbol: SharpYuvGetConversionMatrix (referenced by root reference (e.g. compiled C/C++ code)) #84 1.878 warning: undefined symbol: SharpYuvInit (referenced by root reference (e.g. compiled C/C++ code)) #84 1.880 warning: undefined symbol: getcontext (referenced by root reference (e.g. compiled C/C++ code)) #84 1.880 warning: undefined symbol: getdtablesize (referenced by root reference (e.g. compiled C/C++ code)) #84 1.880 warning: undefined symbol: makecontext (referenced by root reference (e.g. compiled C/C++ code)) #84 1.880 warning: undefined symbol: swapcontext (referenced by root reference (e.g. compiled C/C++ code)) ``` If any new undefined symbols come up, we would not notice them because we set `ERROR_ON_UNDEFINED_SYMBOLS=0`. Let's change this to error so we do not quietly miss new undefined symbols. ## Implementation details Set ERROR_ON_UNDEFINED_SYMBOLS=1 so the Emscripten linker fails if there are any undefined symbols. We have been living with a number of undefined symbols. Some are undefined because Emscripten does not provide them, and others are undefined because we needed to link to an additional library. This PR addresses both cases. In order to be explicit and intentional about undefined symbols we know and accept, this PR adds a JS library for Emscripten that explicitly lists known undefined symbols and declares stubs for them that fail if called. This way, we document the undefined symbols we know about and avoid complaints from Emscripten. ## Testing Instructions (or ideally a Blueprint) - CI
1 parent fea9e99 commit fa41a9f

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

63 files changed

+124973
-111011
lines changed

packages/php-wasm/compile/php/Dockerfile

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ RUN if [ "$WITH_GD" = "yes" ]; \
196196
else \
197197
echo -n ' --with-png-dir=/root/lib --with-jpeg --with-webp --with-gd --enable-gd ' >> /root/.php-configure-flags; \
198198
fi; \
199-
echo -n ' -I /root/lib -I /root/install -lz -lpng16 -lwebp -ljpeg ' >> /root/.emcc-php-wasm-flags; \
199+
echo -n ' -I /root/lib -I /root/install -lz -lpng16 -lwebp -lsharpyuv -ljpeg ' >> /root/.emcc-php-wasm-flags; \
200200
fi;
201201

202202
# Add openssl if needed
@@ -423,7 +423,7 @@ RUN source /root/emsdk/emsdk_env.sh && \
423423
export JSPI_FLAGS=$(if [ "$WITH_JSPI" = "yes" ]; then echo "-sSUPPORT_LONGJMP=wasm -fwasm-exceptions"; else echo ""; fi) && \
424424
EMCC_FLAGS=" -D__x86_64__ -sSIDE_MODULE -Dsetsockopt=wasm_setsockopt -Dphp_exec=wasm_php_exec $JSPI_FLAGS " \
425425
# ...which means we must skip all the libraries - they will be provided in the final linking step.
426-
EMCC_SKIP="-lz -ledit -ldl -lncurses -lzip -lpng16 -lssl -lcrypto -lxml2 -lc -lm -lsqlite3 /root/lib/lib/libxml2.a /root/lib/lib/libsqlite3.so /root/lib/lib/libsqlite3.a /root/lib/lib/libsqlite3.a /root/lib/lib/libpng16.so /root/lib/lib/libwebp.a /root/lib/lib/libjpeg.a" \
426+
EMCC_SKIP="-lz -ledit -ldl -lncurses -lzip -lpng16 -lssl -lcrypto -lxml2 -lc -lm -lsqlite3 /root/lib/lib/libxml2.a /root/lib/lib/libsqlite3.so /root/lib/lib/libsqlite3.a /root/lib/lib/libsqlite3.a /root/lib/lib/libpng16.so /root/lib/lib/libwebp.a /root/lib/lib/libsharpyuv.a /root/lib/lib/libjpeg.a" \
427427
emmake make -j1
428428

429429
RUN cp -v /root/php-src/.libs/libphp*.la /root/lib/libphp.la
@@ -1000,6 +1000,7 @@ export ASYNCIFY_ONLY=$'\
10001000
# Build the final .wasm file
10011001
RUN mkdir /root/output
10021002
COPY ./php/phpwasm-emscripten-library.js /root/phpwasm-emscripten-library.js
1003+
COPY ./php/phpwasm-emscripten-library-known-undefined-functions.js /root/phpwasm-emscripten-library-known-undefined-functions.js
10031004
ARG WITH_SOURCEMAPS
10041005
ARG WITH_DEBUG
10051006
RUN set -euxo pipefail; \
@@ -1046,6 +1047,7 @@ RUN set -euxo pipefail; \
10461047
fi; \
10471048
emcc $OPTIMIZATION_FLAGS \
10481049
--js-library /root/phpwasm-emscripten-library.js \
1050+
--js-library /root/phpwasm-emscripten-library-known-undefined-functions.js \
10491051
-I . \
10501052
-I ext \
10511053
-I ext/json \
@@ -1063,7 +1065,7 @@ RUN set -euxo pipefail; \
10631065
-s INITIAL_MEMORY=1024MB \
10641066
-s ALLOW_MEMORY_GROWTH=1 \
10651067
-s ASSERTIONS=0 \
1066-
-s ERROR_ON_UNDEFINED_SYMBOLS=0 \
1068+
-s ERROR_ON_UNDEFINED_SYMBOLS=1 \
10671069
-s NODEJS_CATCH_EXIT=0 \
10681070
-s NODEJS_CATCH_REJECTION=0 \
10691071
-s INVOKE_RUN=0 \
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* This file is an Emscripten "library" file. It is included in the
3+
* build "php_<major>_<minor>.js" files and implements JavaScript functions
4+
* that can be called from C code.
5+
*
6+
* @see https://emscripten.org/docs/porting/connecting_cpp_and_javascript/Interacting-with-code.html#implement-a-c-api-in-javascript
7+
*/
8+
'use strict';
9+
10+
// Explicitly declare known, accepted undefined functions. By doing this,
11+
// we can enable compiler warnings for unexpected undefined symbols.
12+
var knownUndefinedFunctions = [
13+
// Functions used by PHP for fibers but not supported by Emscripten by default.
14+
// https://github.com/php/php-src/blob/747851724efcdae1502040eec4787fa7b911b6bf/Zend/zend_fibers.c#L517
15+
'getcontext',
16+
'makecontext',
17+
'swapcontext',
18+
19+
// Used by PHP when attempting direct access to file descriptors.
20+
// Not currently included in our build by Emscripten.
21+
// If we encounter a need for it, let's investigate and fix this.
22+
// https://github.com/php/php-src/blob/747851724efcdae1502040eec4787fa7b911b6bf/ext/standard/php_fopen_wrapper.c#L321
23+
'getdtablesize',
24+
];
25+
26+
var LibraryKnownUndefinedFunctions = {};
27+
for (const name of knownUndefinedFunctions) {
28+
LibraryKnownUndefinedFunctions[name] = () => abort('missing function: ${name}');
29+
}
30+
mergeInto(LibraryManager.library, LibraryKnownUndefinedFunctions);

packages/php-wasm/compile/php/phpwasm-emscripten-library.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* This file is an Emscripten "library" file. It is included in the
3-
* build "php-8.0.js" file and implements JavaScript functions that
4-
* called from C code.
3+
* build "php_<major>_<minor>.js" files and implements JavaScript functions
4+
* that can be called from C code.
55
*
66
* @see https://emscripten.org/docs/porting/connecting_cpp_and_javascript/Interacting-with-code.html#implement-a-c-api-in-javascript
77
*/
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.

0 commit comments

Comments
 (0)