From 2ef288148a1a3a16865c6e48b945669c7b9fa11c Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Mon, 18 Nov 2024 18:02:47 -0800 Subject: [PATCH 1/2] Avoid rebuilding `check-symbols` artifacts Like other recent PRs, this change uses true file dependencies to avoid rebuilding the files used to check which symbols are defined or not. The true `git diff` test is still run each time, however. --- Makefile | 87 ++++++++++++++++++++++++++++++-------------------------- 1 file changed, 47 insertions(+), 40 deletions(-) diff --git a/Makefile b/Makefile index 1116c741..ee73d73b 100644 --- a/Makefile +++ b/Makefile @@ -115,7 +115,7 @@ LIBC_BOTTOM_HALF_OMIT_SOURCES := \ LIBC_BOTTOM_HALF_ALL_SOURCES := $(filter-out $(LIBC_BOTTOM_HALF_OMIT_SOURCES),$(LIBC_BOTTOM_HALF_ALL_SOURCES)) # Omit p2-specific headers from include-all.c test. # for exception-handling. -INCLUDE_ALL_CLAUSES := -not -name wasip2.h -not -name descriptor_table.h +INCLUDE_ALL_CLAUSES += -not -name wasip2.h -not -name descriptor_table.h endif ifeq ($(WASI_SNAPSHOT), p2) @@ -921,58 +921,57 @@ finish: check-symbols endif endif -DEFINED_SYMBOLS = $(SYSROOT_SHARE)/defined-symbols.txt -UNDEFINED_SYMBOLS = $(SYSROOT_SHARE)/undefined-symbols.txt +install: finish + mkdir -p "$(INSTALL_DIR)" + cp -r "$(SYSROOT)/lib" "$(SYSROOT)/share" "$(SYSROOT)/include" "$(INSTALL_DIR)" -ifeq ($(WASI_SNAPSHOT),p2) -EXPECTED_TARGET_DIR = expected/wasm32-wasip2 -else -ifeq ($(THREAD_MODEL),posix) -EXPECTED_TARGET_DIR = expected/wasm32-wasip1-threads -else -EXPECTED_TARGET_DIR = expected/wasm32-wasip1 -endif -endif +##### CHECK #################################################################### +# The `check-symbols` target builds up a set of text files in `/share` +# which are compared with known-good output in the `expected` directory. +################################################################################ -check-symbols: startup_files libc - # - # Collect metadata on the sysroot and perform sanity checks. - # +$(SYSROOT_SHARE)/defined-symbols.txt: startup_files libc mkdir -p "$(SYSROOT_SHARE)" - - # - # Collect symbol information. - # + "$(NM)" --defined-only \ + $(SYSROOT_LIB)/libc.a $(SYSROOT_LIB)/libwasi-emulated-*.a $(SYSROOT_LIB)/*.o \ + |grep ' [[:upper:]] ' \ + |sed 's/.* [[:upper:]] //' \ + |LC_ALL=C sort \ + |uniq \ + > "$@" + +$(SYSROOT_SHARE)/undefined-symbols.txt: $(SYSROOT_SHARE)/defined-symbols.txt @# TODO: Use llvm-nm --extern-only instead of grep. This is blocked on @# LLVM PR40497, which is fixed in 9.0, but not in 8.0. @# Ignore certain llvm builtin symbols such as those starting with __mul @# since these dependencies can vary between llvm versions. - "$(NM)" --defined-only "$(SYSROOT_LIB)"/libc.a "$(SYSROOT_LIB)"/libwasi-emulated-*.a "$(SYSROOT_LIB)"/*.o \ - |grep ' [[:upper:]] ' |sed 's/.* [[:upper:]] //' |LC_ALL=C sort |uniq > "$(DEFINED_SYMBOLS)" - for undef_sym in $$("$(NM)" --undefined-only "$(SYSROOT_LIB)"/libc.a "$(SYSROOT_LIB)"/libc-*.a "$(SYSROOT_LIB)"/*.o \ - |grep ' U ' |sed 's/.* U //' |LC_ALL=C sort |uniq); do \ - grep -q '\<'$$undef_sym'\>' "$(DEFINED_SYMBOLS)" || echo $$undef_sym; \ - done | grep -E -v "^__mul|__memory_base|__indirect_function_table|__tls_base" > "$(UNDEFINED_SYMBOLS)" - grep '^_*imported_wasi_' "$(UNDEFINED_SYMBOLS)" \ - > "$(SYSROOT_LIB)/libc.imports" + for undef_sym in $$("$(NM)" --undefined-only $(SYSROOT_LIB)/libc.a $(SYSROOT_LIB)/libc-*.a $(SYSROOT_LIB)/*.o |grep ' U ' |sed 's/.* U //' |LC_ALL=C sort |uniq); do \ + grep -q '\<'$$undef_sym'\>' "$<" || echo $$undef_sym; \ + done | grep -E -v "^__mul|__memory_base|__indirect_function_table|__tls_base" > "$@" +$(SYSROOT_LIB)/libc.imports: $(SYSROOT_SHARE)/undefined-symbols.txt + grep '^_*imported_wasi_' $< > $@ + +INCLUDE_ALL_CLAUSES += -not -name mman.h -not -name signal.h -not -name times.h -not -name resource.h -not -name setjmp.h +$(SYSROOT_SHARE)/include-all.c: $(INCLUDE_DIRS) + mkdir -p "$(SYSROOT_SHARE)" # # Generate a test file that includes all public C header files. # # setjmp.h is excluded because it requires a different compiler option # cd "$(SYSROOT_INC)" && \ - for header in $$(find . -type f -not -name mman.h -not -name signal.h -not -name times.h -not -name resource.h -not -name setjmp.h $(INCLUDE_ALL_CLAUSES) |grep -v /bits/ |grep -v /c++/); do \ + for header in $$(find . -type f $(INCLUDE_ALL_CLAUSES) |grep -v /bits/ |grep -v /c++/); do \ echo '#include <'$$header'>' | sed 's/\.\///' ; \ - done |LC_ALL=C sort >$(SYSROOT_SHARE)/include-all.c ; \ + done |LC_ALL=C sort > $@; \ cd - >/dev/null - # - # Test that it compiles. + # Test that all public C headers compile. # - $(CC) $(CFLAGS) -fsyntax-only "$(SYSROOT_SHARE)/include-all.c" -Wno-\#warnings + $(CC) $(CFLAGS) -fsyntax-only "$@" -Wno-\#warnings +$(SYSROOT_SHARE)/predefined-macros.txt: $(SYSROOT_SHARE)/include-all.c # # Collect all the predefined macros, except for compiler version macros # which we don't need to track here. @@ -1002,7 +1001,7 @@ check-symbols: startup_files libc @# TODO: Undefine __wasm_nontrapping_fptoint__ and __wasm_bulk_memory__, that are @# new to clang 20. @# TODO: As of clang 16, __GNUC_VA_LIST is #defined without a value. - $(CC) $(CFLAGS) "$(SYSROOT_SHARE)/include-all.c" \ + $(CC) $(CFLAGS) "$<" \ -isystem $(SYSROOT_INC) \ -std=gnu17 \ -E -dM -Wno-\#warnings \ @@ -1042,15 +1041,23 @@ check-symbols: startup_files libc | grep -v '^#define __OPTIMIZE__' \ | grep -v '^#define assert' \ | grep -v '^#define __NO_INLINE__' \ - > "$(SYSROOT_SHARE)/predefined-macros.txt" + > "$@" +# Pick which `expected` target to compare against. +ifeq ($(WASI_SNAPSHOT),p2) +EXPECTED_TARGET_DIR = expected/wasm32-wasip2 +else +ifeq ($(THREAD_MODEL),posix) +EXPECTED_TARGET_DIR = expected/wasm32-wasip1-threads +else +EXPECTED_TARGET_DIR = expected/wasm32-wasip1 +endif +endif + +check-symbols: $(SYSROOT_SHARE)/defined-symbols.txt $(SYSROOT_SHARE)/undefined-symbols.txt $(SYSROOT_SHARE)/include-all.c $(SYSROOT_SHARE)/predefined-macros.txt $(SYSROOT_LIB)/libc.imports # Check that the computed metadata matches the expected metadata. # This ignores whitespace because on Windows the output has CRLF line endings. - diff -wur "$(EXPECTED_TARGET_DIR)" "$(SYSROOT_SHARE)" - -install: finish - mkdir -p "$(INSTALL_DIR)" - cp -r "$(SYSROOT)/lib" "$(SYSROOT)/share" "$(SYSROOT)/include" "$(INSTALL_DIR)" + diff -wur $(EXPECTED_TARGET_DIR) $(SYSROOT_SHARE) $(BINDING_WORK_DIR)/wasi-cli: mkdir -p "$(BINDING_WORK_DIR)" From a0cbb41877acfa8a84acf0618b0c116b01a33046 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 20 Nov 2024 15:25:11 -0800 Subject: [PATCH 2/2] Use all `lib` artifacts when checking symbols Previously, we generated the defined and undefined symbol lists from different sets of `sysroot/lib` artifacts. This uses all static artifacts that are built, `*.a` and `*.o`, to increase coverage of the symbols wasi-libc is publishing. --- Makefile | 4 ++-- expected/wasm32-wasip1-threads/defined-symbols.txt | 8 ++++++++ expected/wasm32-wasip1/defined-symbols.txt | 8 ++++++++ expected/wasm32-wasip2/defined-symbols.txt | 8 ++++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index ee73d73b..9ca4ee8b 100644 --- a/Makefile +++ b/Makefile @@ -934,7 +934,7 @@ install: finish $(SYSROOT_SHARE)/defined-symbols.txt: startup_files libc mkdir -p "$(SYSROOT_SHARE)" "$(NM)" --defined-only \ - $(SYSROOT_LIB)/libc.a $(SYSROOT_LIB)/libwasi-emulated-*.a $(SYSROOT_LIB)/*.o \ + $(SYSROOT_LIB)/*.a $(SYSROOT_LIB)/*.o \ |grep ' [[:upper:]] ' \ |sed 's/.* [[:upper:]] //' \ |LC_ALL=C sort \ @@ -946,7 +946,7 @@ $(SYSROOT_SHARE)/undefined-symbols.txt: $(SYSROOT_SHARE)/defined-symbols.txt @# LLVM PR40497, which is fixed in 9.0, but not in 8.0. @# Ignore certain llvm builtin symbols such as those starting with __mul @# since these dependencies can vary between llvm versions. - for undef_sym in $$("$(NM)" --undefined-only $(SYSROOT_LIB)/libc.a $(SYSROOT_LIB)/libc-*.a $(SYSROOT_LIB)/*.o |grep ' U ' |sed 's/.* U //' |LC_ALL=C sort |uniq); do \ + for undef_sym in $$("$(NM)" --undefined-only $(SYSROOT_LIB)/*.a $(SYSROOT_LIB)/*.o |grep ' U ' |sed 's/.* U //' |LC_ALL=C sort |uniq); do \ grep -q '\<'$$undef_sym'\>' "$<" || echo $$undef_sym; \ done | grep -E -v "^__mul|__memory_base|__indirect_function_table|__tls_base" > "$@" diff --git a/expected/wasm32-wasip1-threads/defined-symbols.txt b/expected/wasm32-wasip1-threads/defined-symbols.txt index 2218541b..f7cbd98b 100644 --- a/expected/wasm32-wasip1-threads/defined-symbols.txt +++ b/expected/wasm32-wasip1-threads/defined-symbols.txt @@ -20,6 +20,7 @@ __atexit_lockptr __c_dot_utf8 __c_dot_utf8_locale __c_locale +__c_longjmp __clock __clock_gettime __clock_nanosleep @@ -402,6 +403,9 @@ __wasilibc_tell __wasilibc_unlinkat __wasilibc_utimens __wasm_call_dtors +__wasm_longjmp +__wasm_setjmp +__wasm_setjmp_test __wcscoll_l __wcsftime_l __wcsxfrm_l @@ -575,6 +579,10 @@ difftime dirfd dirname div +dlclose +dlerror +dlopen +dlsym dprintf drand48 drem diff --git a/expected/wasm32-wasip1/defined-symbols.txt b/expected/wasm32-wasip1/defined-symbols.txt index 6e06d64f..83fe8143 100644 --- a/expected/wasm32-wasip1/defined-symbols.txt +++ b/expected/wasm32-wasip1/defined-symbols.txt @@ -17,6 +17,7 @@ __assert_fail __c_dot_utf8 __c_dot_utf8_locale __c_locale +__c_longjmp __clock __clock_gettime __clock_nanosleep @@ -373,6 +374,9 @@ __wasilibc_tell __wasilibc_unlinkat __wasilibc_utimens __wasm_call_dtors +__wasm_longjmp +__wasm_setjmp +__wasm_setjmp_test __wcscoll_l __wcsftime_l __wcsxfrm_l @@ -546,6 +550,10 @@ difftime dirfd dirname div +dlclose +dlerror +dlopen +dlsym dprintf drand48 drem diff --git a/expected/wasm32-wasip2/defined-symbols.txt b/expected/wasm32-wasip2/defined-symbols.txt index 35e5c391..36eaee69 100644 --- a/expected/wasm32-wasip2/defined-symbols.txt +++ b/expected/wasm32-wasip2/defined-symbols.txt @@ -18,6 +18,7 @@ __assert_fail __c_dot_utf8 __c_dot_utf8_locale __c_locale +__c_longjmp __clock __clock_gettime __clock_nanosleep @@ -389,6 +390,9 @@ __wasilibc_tell __wasilibc_unlinkat __wasilibc_utimens __wasm_call_dtors +__wasm_longjmp +__wasm_setjmp +__wasm_setjmp_test __wcscoll_l __wcsftime_l __wcsxfrm_l @@ -568,6 +572,10 @@ difftime dirfd dirname div +dlclose +dlerror +dlopen +dlsym dprintf drand48 drem