diff --git a/CLAUDE.md b/CLAUDE.md index 67a821fc3..3787d8285 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -215,7 +215,7 @@ $BUILD_DIR/tests/unit/kv_cached_tests Adding a unit test needs **two** registrations: `add_cdt_unit_test()` in `tests/unit/CMakeLists.txt` (builds it) and `add_unit_test()` in `tests/CMakeLists.txt` (registers it with ctest). Miss the second and the test still compiles but `ctest` never executes it — a silent gap, not a failure. `basic_name_tests` sat that way until it was registered; when adding a test, check both lists match. -A few unit tests are shell scripts registered directly in `tests/CMakeLists.txt`: `version_tests`, `abi_version_tests`, `multidir_contract_tests`, `postpass_tests`. +A few unit tests are shell scripts registered directly in `tests/CMakeLists.txt`: `version_tests`, `abi_version_tests`, `abidiff_tests`, `multidir_contract_tests`, `postpass_tests`, `staged_headers_tests`. ### Toolchain tests diff --git a/cmake/CDTMacros.cmake.in b/cmake/CDTMacros.cmake.in index b40ebbe50..ddb1ba275 100644 --- a/cmake/CDTMacros.cmake.in +++ b/cmake/CDTMacros.cmake.in @@ -182,7 +182,7 @@ endmacro() # ) # # The resulting module exports an `apply(uint64_t, uint64_t, uint64_t)` -# function. Intrinsic symbols (db_store_i64, etc.) are left undefined and +# function. Intrinsic symbols (kv_set, kv_get, etc.) are left undefined and # resolved at dlopen time against symbols exported by the host executable. function(add_native_contract) cmake_parse_arguments(ARG "" "TARGET;CONTRACT_CLASS;ABI_FILE" diff --git a/cmake/InstallCDT.cmake b/cmake/InstallCDT.cmake index 36716064c..e82102fa8 100644 --- a/cmake/InstallCDT.cmake +++ b/cmake/InstallCDT.cmake @@ -46,9 +46,14 @@ macro( cdt_libraries_install) install(DIRECTORY ${CMAKE_BINARY_DIR}/lib/ DESTINATION lib COMPONENT base PATTERN "libnative*" EXCLUDE PATTERN "cmake" EXCLUDE) - install(DIRECTORY ${CMAKE_BINARY_DIR}/lib/ DESTINATION lib COMPONENT dev - FILES_MATCHING PATTERN "libnative*" - PATTERN "cmake" EXCLUDE) + # Guarded on the option, not merely on what happens to be sitting in lib/: a tree + # reconfigured from native ON to OFF can still hold archives from the previous build. + # stage_cdt_tree prunes those, and this makes packaging one impossible regardless. + if(ENABLE_NATIVE_COMPILER) + install(DIRECTORY ${CMAKE_BINARY_DIR}/lib/ DESTINATION lib COMPONENT dev + FILES_MATCHING PATTERN "libnative*" + PATTERN "cmake" EXCLUDE) + endif() install(DIRECTORY ${CMAKE_BINARY_DIR}/include/ DESTINATION include COMPONENT base) endmacro( cdt_libraries_install ) diff --git a/cmake/package.cmake b/cmake/package.cmake index ab18e443f..e45d4bd34 100644 --- a/cmake/package.cmake +++ b/cmake/package.cmake @@ -178,11 +178,17 @@ set(CPACK_WIRE_PUBLIC_ENTRY_POINTS "${CDT_PUBLIC_ENTRY_POINTS}") # versioned name on its own (see the archive-name/root decoupling in # cmake/cpack-project-config.cmake), so this no longer renames anything -- it is # a plain alias, kept because CI and the docs invoke it by name. +# Depends on CDTWasmLibraries because header staging (and its pruning) happens in that +# nested build -- see cmake/stage_cdt_tree.cmake. The generated `package` and `install` +# targets are ordered after `all` and so pick it up for free, but this convenience +# target is standalone: without the dependency, `cmake --build . --target package-tgz` +# on a reused tree could run CPack over a stale staged header a prior build deleted. add_custom_target(package-tgz COMMAND "${CMAKE_CPACK_COMMAND}" -G TGZ WORKING_DIRECTORY "${CMAKE_BINARY_DIR}" COMMENT "Packaging ${CPACK_PACKAGE_FILE_NAME}.tar.gz (portable toolchain)" VERBATIM) +add_dependencies(package-tgz CDTWasmLibraries) set(CPACK_SET_DESTDIR OFF) set(CPACK_PACKAGE_RELOCATABLE OFF) diff --git a/cmake/stage_cdt_tree.cmake b/cmake/stage_cdt_tree.cmake new file mode 100644 index 000000000..7eff6eaf0 --- /dev/null +++ b/cmake/stage_cdt_tree.cmake @@ -0,0 +1,99 @@ +# Stage the CDT-owned parts of the build tree -- headers into /include and the +# native archives in /lib -- pruning first. +# +# Run in script mode (`cmake -P`) from the `stage_cdt_tree` build target, not at +# configure time. Configure-time `file(COPY)` is additive: it never removes a staged +# copy whose source has been deleted, so a removed header stayed in /include +# forever -- shipped by install/CPack, and visible to native consumers whose compiled +# view could then disagree with the rebuilt library. Reusing a build tree across such +# a deletion is the case this exists to handle, and a configure-time copy cannot, +# because the ExternalProject's configure step is stamped and does not re-run. +# +# The two destinations OVERLAP: sysiolib owns include/sysiolib, and native's second +# copy lands in include/sysiolib/native, a subdirectory of it. Pruning and copying +# from one ordered script is what makes that safe -- two independent steps would race +# to delete each other's output. +# +# `file(COPY)` preserves source timestamps, so re-running every build neither churns +# mtimes nor triggers downstream rebuilds. (It also skips files already current at the +# destination, but that never applies here: both destinations are REMOVE_RECURSE'd +# below before either is repopulated.) +# +# EVERY tree staged into /include is handled here. The four vendored ones -- libc, +# libcxx, boost/preprocessor and bluegrass -- were left as configure-time copies in an +# earlier revision, which meant deleting a header from the cdt-musl or cdt-libcxx submodule +# left the staged copy shipping forever, exactly the bug this script exists to fix. They are +# pruned and recopied on the same schedule now. +# +# Inputs (via -D): +# STAGE_SOURCE_DIR - the repo's libraries/ directory +# STAGE_BINARY_DIR - BASE_BINARY_DIR, whose include/ subtree is staged into +# STAGE_NATIVE - truthy when ENABLE_NATIVE_COMPILER is on + +foreach(var STAGE_SOURCE_DIR STAGE_BINARY_DIR) + if(NOT DEFINED ${var}) + message(FATAL_ERROR "stage_cdt_tree.cmake: ${var} is required") + endif() +endforeach() + +set(header_patterns FILES_MATCHING PATTERN "*.h" PATTERN "*.hpp") + +# Prune BOTH trees before either is repopulated, and prune the native tree whether or +# not native mode is on. Pruning it inside the STAGE_NATIVE branch left the previous +# build's native headers staged when a reused tree flipped ENABLE_NATIVE_COMPILER from +# ON to OFF -- and since InstallCDT.cmake installs the whole include tree, an OFF build +# then packaged an API it was configured not to build. (include/sysiolib/native happens +# to vanish with its parent; include/sysio/native has no such parent.) +file(REMOVE_RECURSE "${STAGE_BINARY_DIR}/include/sysiolib") +file(REMOVE_RECURSE "${STAGE_BINARY_DIR}/include/sysio/native") +file(REMOVE_RECURSE "${STAGE_BINARY_DIR}/include/libc") +file(REMOVE_RECURSE "${STAGE_BINARY_DIR}/include/libcxx") +file(REMOVE_RECURSE "${STAGE_BINARY_DIR}/include/boost/preprocessor") +file(REMOVE_RECURSE "${STAGE_BINARY_DIR}/include/bluegrass") + +# sysiolib -> include/sysiolib +file(COPY "${STAGE_SOURCE_DIR}/sysiolib" + DESTINATION "${STAGE_BINARY_DIR}/include" + ${header_patterns}) + +# The native-host archives are copied into lib/ by POST_BUILD commands that exist only while +# ENABLE_NATIVE_COMPILER is on. Reconfiguring a reused tree to OFF removes those targets but +# not the files they already copied, and InstallCDT.cmake installs lib/ wholesale -- so an OFF +# build packaged archives its own configuration never produced, still carrying whatever symbols +# the last ON build put in them. +# +# libnative* ONLY. libsf.a is a WebAssembly archive, not a native-host one: cdt-ld links it +# with -lsf for --use-rt and every --fquery mode (compiler_options.hpp.in), and the base +# install ships it. It is declared in libraries/native/CMakeLists.txt but OUTSIDE that file's +# native-only guard, so every configuration builds and stages it -- deleting it here would +# strip the copy an OFF package needs to link those modes. +if(NOT STAGE_NATIVE) + file(GLOB stale_native "${STAGE_BINARY_DIR}/lib/libnative*") + if(stale_native) + file(REMOVE ${stale_native}) + endif() +endif() + +if(STAGE_NATIVE) + # native -> include/sysio/native + file(COPY "${STAGE_SOURCE_DIR}/native" + DESTINATION "${STAGE_BINARY_DIR}/include/sysio" + ${header_patterns} PATTERN "softfloat" EXCLUDE) + + # native/native -> include/sysiolib/native (inside the tree pruned above) + file(COPY "${STAGE_SOURCE_DIR}/native/native" + DESTINATION "${STAGE_BINARY_DIR}/include/sysiolib" + ${header_patterns} PATTERN "softfloat" EXCLUDE) +endif() + +# The vendored trees. libc and libcxx copy whole directories rather than header-matching, +# because musl and libc++ both ship extensionless headers (, , ...) that a +# "*.h;*.hpp" filter would drop. +file(COPY "${STAGE_SOURCE_DIR}/libc/cdt-musl/include/" DESTINATION "${STAGE_BINARY_DIR}/include/libc/") +file(COPY "${STAGE_SOURCE_DIR}/libc/cdt-musl/src/internal/" DESTINATION "${STAGE_BINARY_DIR}/include/libc/") +file(COPY "${STAGE_SOURCE_DIR}/libc/cdt-musl/arch/eos/" DESTINATION "${STAGE_BINARY_DIR}/include/libc/") +file(COPY "${STAGE_SOURCE_DIR}/libc++/cdt-libcxx/include/" DESTINATION "${STAGE_BINARY_DIR}/include/libcxx") +file(COPY "${STAGE_SOURCE_DIR}/boost/include/boost/preprocessor" + DESTINATION "${STAGE_BINARY_DIR}/include/boost") +file(COPY "${STAGE_SOURCE_DIR}/meta_refl/include/bluegrass" + DESTINATION "${STAGE_BINARY_DIR}/include") diff --git a/imports/cdt.imports.in b/imports/cdt.imports.in index 3a3584e9b..6f3f735cd 100644 --- a/imports/cdt.imports.in +++ b/imports/cdt.imports.in @@ -4,7 +4,6 @@ memcmp memset abort action_data_size -add_security_group_participants alt_bn128_add alt_bn128_mul alt_bn128_pair @@ -29,70 +28,9 @@ check_permission_authorization check_transaction_authorization current_receiver current_time -db_end_i64 -db_find_i64 -db_get_i64 -db_idx128_end -db_idx128_find_primary -db_idx128_find_secondary -db_idx128_lowerbound -db_idx128_next -db_idx128_previous -db_idx128_remove -db_idx128_store -db_idx128_update -db_idx128_upperbound -db_idx256_end -db_idx256_find_primary -db_idx256_find_secondary -db_idx256_lowerbound -db_idx256_next -db_idx256_previous -db_idx256_remove -db_idx256_store -db_idx256_update -db_idx256_upperbound -db_idx64_end -db_idx64_find_primary -db_idx64_find_secondary -db_idx64_lowerbound -db_idx64_next -db_idx64_previous -db_idx64_remove -db_idx64_store -db_idx64_update -db_idx64_upperbound -db_idx_double_end -db_idx_double_find_primary -db_idx_double_find_secondary -db_idx_double_lowerbound -db_idx_double_next -db_idx_double_previous -db_idx_double_remove -db_idx_double_store -db_idx_double_update -db_idx_double_upperbound -db_idx_long_double_end -db_idx_long_double_find_primary -db_idx_long_double_find_secondary -db_idx_long_double_lowerbound -db_idx_long_double_next -db_idx_long_double_previous -db_idx_long_double_remove -db_idx_long_double_store -db_idx_long_double_update -db_idx_long_double_upperbound -db_lowerbound_i64 -db_next_i64 -db_previous_i64 -db_remove_i64 -db_store_i64 -db_update_i64 -db_upperbound_i64 expiration get_action get_active_producers -get_active_security_group get_block_num get_blockchain_parameters_packed get_code_hash @@ -102,7 +40,6 @@ get_ram_usage get_resource_limits get_sender has_auth -in_active_security_group is_account is_feature_activated k1_recover @@ -124,7 +61,6 @@ publication_time read_action_data read_transaction recover_key -remove_security_group_participants require_auth require_auth2 require_recipient @@ -134,7 +70,6 @@ send_inline set_action_return_value set_blockchain_parameters_packed set_finalizers -set_kv_parameters_packed set_privileged set_proposed_producers set_proposed_producers_ex diff --git a/libraries/CMakeLists.txt b/libraries/CMakeLists.txt index 3bd42e7bf..6d19a8175 100644 --- a/libraries/CMakeLists.txt +++ b/libraries/CMakeLists.txt @@ -19,14 +19,51 @@ endif() set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_EXTENSIONS ON) +# Stage the CDT-owned headers into ${BASE_BINARY_DIR}/include at BUILD time, pruning +# any whose source has been deleted. See cmake/stage_cdt_tree.cmake for why this cannot +# be a configure-time file(COPY). +add_custom_target(stage_cdt_tree ALL + COMMAND ${CMAKE_COMMAND} + -DSTAGE_SOURCE_DIR=${CMAKE_CURRENT_SOURCE_DIR} + -DSTAGE_BINARY_DIR=${BASE_BINARY_DIR} + -DSTAGE_NATIVE=$ + -P ${CMAKE_CURRENT_SOURCE_DIR}/../cmake/stage_cdt_tree.cmake + COMMENT "Staging CDT build tree into ${BASE_BINARY_DIR}/include" + VERBATIM) + add_subdirectory(libc) add_subdirectory(libc++) add_subdirectory(sysiolib) add_subdirectory(rt) -if (ENABLE_NATIVE_COMPILER) - add_subdirectory(native) -endif() +# Added unconditionally. The directory defines the WebAssembly softfloat archive `sf`, which +# cdt-ld links with -lsf for --use-rt and every --fquery mode, alongside the native-host +# targets -- and only the latter depend on ENABLE_NATIVE_COMPILER. Skipping the whole +# directory meant a clean OFF build produced no libsf.a at all, so an OFF package could not +# link those Wasm modes. The native-host targets are gated inside the file instead. +add_subdirectory(native) + +# Anything compiled against the staged tree must see the pruned copy, not a leftover. +# +# Enumerated, not listed by hand. The staging step REMOVE_RECURSE's its destinations and every +# compile in this tree carries -I/include/... baked in by the driver, so a target that +# is not ordered after it can race the deletion. A three-name allowlist covered `sysio`, +# `native` and `native_sysio` and silently missed sysio_malloc, sysio_dsm, sysio_cmem, c, c++, +# rt, sf and the native_* variants -- and would miss the next one added. +function(cdt_order_after_staging dir) + get_property(dir_targets DIRECTORY "${dir}" PROPERTY BUILDSYSTEM_TARGETS) + foreach(tgt IN LISTS dir_targets) + if(NOT tgt STREQUAL "stage_cdt_tree") + get_target_property(tgt_type ${tgt} TYPE) + if(NOT tgt_type STREQUAL "INTERFACE_LIBRARY") + add_dependencies(${tgt} stage_cdt_tree) + endif() + endif() + endforeach() + get_property(subdirs DIRECTORY "${dir}" PROPERTY SUBDIRECTORIES) + foreach(subdir IN LISTS subdirs) + cdt_order_after_staging("${subdir}") + endforeach() +endfunction() +cdt_order_after_staging("${CMAKE_CURRENT_SOURCE_DIR}") -file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/boost/include/boost/preprocessor DESTINATION ${BASE_BINARY_DIR}/include/boost) -file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/meta_refl/include/bluegrass DESTINATION ${BASE_BINARY_DIR}/include) diff --git a/libraries/libc++/CMakeLists.txt b/libraries/libc++/CMakeLists.txt index b06da22b8..bb121b408 100644 --- a/libraries/libc++/CMakeLists.txt +++ b/libraries/libc++/CMakeLists.txt @@ -44,4 +44,3 @@ if (ENABLE_NATIVE_COMPILER) add_custom_command( TARGET native_c++ POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy $ ${BASE_BINARY_DIR}/lib ) endif() -file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/cdt-libcxx/include/ DESTINATION ${BASE_BINARY_DIR}/include/libcxx) diff --git a/libraries/libc/CMakeLists.txt b/libraries/libc/CMakeLists.txt index f2be30d12..77c20666f 100644 --- a/libraries/libc/CMakeLists.txt +++ b/libraries/libc/CMakeLists.txt @@ -92,6 +92,3 @@ if (ENABLE_NATIVE_COMPILER) add_custom_command( TARGET native_c POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy $ ${BASE_BINARY_DIR}/lib ) endif() -file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/cdt-musl/include/ DESTINATION ${BASE_BINARY_DIR}/include/libc/) -file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/cdt-musl/src/internal/ DESTINATION ${BASE_BINARY_DIR}/include/libc/) -file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/cdt-musl/arch/eos/ DESTINATION ${BASE_BINARY_DIR}/include/libc/) diff --git a/libraries/native/CMakeLists.txt b/libraries/native/CMakeLists.txt index 8941e8939..e0bb9758a 100644 --- a/libraries/native/CMakeLists.txt +++ b/libraries/native/CMakeLists.txt @@ -355,15 +355,18 @@ list( APPEND native_softfloat_sources ${native_softfloat_headers} ) add_library ( sf STATIC ${softfloat_sources} ) target_include_directories( sf PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/softfloat/source/include" "${CMAKE_CURRENT_SOURCE_DIR}/${SOFTFLOAT_SPECIALIZE_DIR}" "${CMAKE_CURRENT_SOURCE_DIR}/${SOFTFLOAT_PLATFORM_DIR}" ${CMAKE_SOURCE_DIR}) -add_native_library ( native STATIC ${native_softfloat_sources} intrinsics.cpp crt.cpp ${CRT_ASM} ) -target_include_directories( native PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/softfloat/source/include" "${CMAKE_CURRENT_SOURCE_DIR}/${NATIVE_SOFTFLOAT_SPECIALIZE_DIR}" "${CMAKE_CURRENT_SOURCE_DIR}/${NATIVE_SOFTFLOAT_PLATFORM_DIR}" ${CMAKE_SOURCE_DIR} ${CMAKE_SOURCE_DIR}/sysiolib/capi ${CMAKE_SOURCE_DIR}/sysiolib/contracts ${CMAKE_SOURCE_DIR}/sysiolib/core) +# The native-HOST library. Everything above this point -- including the `sf` archive -- is +# WebAssembly and is built in every configuration, because cdt-ld links -lsf for --use-rt and +# the --fquery modes regardless of whether the native tester is enabled. +if (ENABLE_NATIVE_COMPILER) + add_native_library ( native STATIC ${native_softfloat_sources} intrinsics.cpp crt.cpp ${CRT_ASM} ) + target_include_directories( native PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/softfloat/source/include" "${CMAKE_CURRENT_SOURCE_DIR}/${NATIVE_SOFTFLOAT_SPECIALIZE_DIR}" "${CMAKE_CURRENT_SOURCE_DIR}/${NATIVE_SOFTFLOAT_PLATFORM_DIR}" ${CMAKE_SOURCE_DIR} ${CMAKE_SOURCE_DIR}/sysiolib/capi ${CMAKE_SOURCE_DIR}/sysiolib/contracts ${CMAKE_SOURCE_DIR}/sysiolib/core) -add_dependencies(native native_sysio) + add_dependencies(native native_sysio) -add_custom_command( TARGET native POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy $ ${BASE_BINARY_DIR}/lib ) + add_custom_command( TARGET native POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy $ ${BASE_BINARY_DIR}/lib ) +endif() add_custom_command( TARGET sf POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy $ ${BASE_BINARY_DIR}/lib ) -file(COPY ${CMAKE_CURRENT_SOURCE_DIR} DESTINATION ${BASE_BINARY_DIR}/include/sysio FILES_MATCHING PATTERN "*.h" PATTERN "*.hpp" PATTERN "softfloat" EXCLUDE) - -file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/native DESTINATION ${BASE_BINARY_DIR}/include/sysiolib FILES_MATCHING PATTERN "*.h" PATTERN "*.hpp" PATTERN "softfloat" EXCLUDE) +# Header staging lives in the stage_cdt_tree target (cmake/stage_cdt_tree.cmake). diff --git a/libraries/native/intrinsics.cpp b/libraries/native/intrinsics.cpp index 2b797e3ea..5c9c7c307 100644 --- a/libraries/native/intrinsics.cpp +++ b/libraries/native/intrinsics.cpp @@ -704,22 +704,6 @@ extern "C" { } #pragma clang diagnostic pop - int64_t add_security_group_participants(const char* data, uint32_t datalen) { - return intrinsics::get().call(data, datalen); - } - - int64_t remove_security_group_participants(const char* data, uint32_t datalen){ - return intrinsics::get().call(data, datalen); - } - - bool in_active_security_group(const char* data, uint32_t datalen){ - return intrinsics::get().call(data, datalen); - } - - uint32_t get_active_security_group(char* data, uint32_t datalen){ - return intrinsics::get().call(data, datalen); - } - void set_finalizers(uint64_t packed_finalizer_format, const char* data, uint32_t len) { intrinsics::get().call(packed_finalizer_format, data, len); } diff --git a/libraries/native/native/sysio/intrinsics_def.hpp b/libraries/native/native/sysio/intrinsics_def.hpp index 560d2ed70..1a536e19b 100644 --- a/libraries/native/native/sysio/intrinsics_def.hpp +++ b/libraries/native/native/sysio/intrinsics_def.hpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include @@ -102,10 +101,6 @@ intrinsic_macro(send_context_free_inline) \ intrinsic_macro(get_context_free_data) \ intrinsic_macro(get_sender) \ intrinsic_macro(set_action_return_value) \ -intrinsic_macro(add_security_group_participants) \ -intrinsic_macro(remove_security_group_participants) \ -intrinsic_macro(in_active_security_group) \ -intrinsic_macro(get_active_security_group) \ intrinsic_macro(blake2_f) \ intrinsic_macro(blake2b_256) \ intrinsic_macro(sha3) \ diff --git a/libraries/sysiolib/CMakeLists.txt b/libraries/sysiolib/CMakeLists.txt index 1d014d1f0..70c037903 100644 --- a/libraries/sysiolib/CMakeLists.txt +++ b/libraries/sysiolib/CMakeLists.txt @@ -48,4 +48,4 @@ if (ENABLE_NATIVE_COMPILER) add_custom_command( TARGET native_sysio POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy $ ${BASE_BINARY_DIR}/lib ) endif() -file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/../sysiolib DESTINATION ${BASE_BINARY_DIR}/include FILES_MATCHING PATTERN "*.h" PATTERN "*.hpp") +# Header staging lives in the stage_cdt_tree target (cmake/stage_cdt_tree.cmake). diff --git a/libraries/sysiolib/capi/sysio/privileged.h b/libraries/sysiolib/capi/sysio/privileged.h index 262c9b7a7..54606c04f 100644 --- a/libraries/sysiolib/capi/sysio/privileged.h +++ b/libraries/sysiolib/capi/sysio/privileged.h @@ -105,16 +105,6 @@ void set_blockchain_parameters_packed( char* data, uint32_t datalen ); __attribute__((sysio_wasm_import)) uint32_t get_blockchain_parameters_packed( char* data, uint32_t datalen ); -/** - * Set the KV parameters - * - * @param data - pointer to KV parameters packed as bytes - * @param datalen - size of the packed KV parameters - * @pre `data` is a valid pointer to a range of memory at least `datalen` bytes long that contains packed KV params data - */ -__attribute__((sysio_wasm_import)) -void set_kv_parameters_packed( const char* data, uint32_t datalen ); - /** * Pre-activate protocol feature * diff --git a/libraries/sysiolib/capi/sysio/security_group.h b/libraries/sysiolib/capi/sysio/security_group.h deleted file mode 100644 index acd53bc8f..000000000 --- a/libraries/sysiolib/capi/sysio/security_group.h +++ /dev/null @@ -1,56 +0,0 @@ -#pragma once -#include "types.h" -#ifdef __cplusplus -extern "C" { -#endif - -/** - * Propose new participants to the security group. - * - * @param data - the buffer containing the packed participants. - * @param datalen - size of the packed participants - * @pre `data` is a valid pointer to a range of memory at least `datalen` bytes long that contains packed participants data - * - * @return -1 if proposing a new security group was unsuccessful, otherwise returns 0. -*/ -__attribute__((sysio_wasm_import)) -int64_t add_security_group_participants(const char* data, uint32_t datalen); - -/** - * Propose to remove participants from the security group. - * - * @param data - the buffer containing the packed participants. - * @param datalen - size of the packed participants - * @pre `data` is a valid pointer to a range of memory at least `datalen` bytes long that contains packed participants data - * - * @return -1 if proposing a new security group was unsuccessful, otherwise returns 0. -*/ -__attribute__((sysio_wasm_import)) -int64_t remove_security_group_participants(const char* data, uint32_t datalen); - -/** - * Check if the specified accounts are all in the active security group. - * - * @param data - the buffer containing the packed participants. - * @param datalen - size of the packed participants - * - * @return Returns true if the specified accounts are all in the active security group. -*/ -__attribute__((sysio_wasm_import)) -bool in_active_security_group(const char* data, uint32_t datalen); - -/** - * Gets the active security group - * - * @param[out] data - the output buffer containing the packed security group. - * @param datalen - size of the `data` buffer - * - * @return Returns the size required in the buffer (if the buffer is too small, nothing is written). - * -*/ -__attribute__((sysio_wasm_import)) -uint32_t get_active_security_group(char* data, uint32_t datalen); - -#ifdef __cplusplus -} -#endif diff --git a/libraries/sysiolib/contracts/sysio/security_group.hpp b/libraries/sysiolib/contracts/sysio/security_group.hpp deleted file mode 100644 index 7cedc01b9..000000000 --- a/libraries/sysiolib/contracts/sysio/security_group.hpp +++ /dev/null @@ -1,86 +0,0 @@ -#pragma once -#include -#include "../../core/sysio/name.hpp" -#include "../../core/sysio/serialize.hpp" - -namespace sysio { - -namespace internal_use_do_not_use { -extern "C" { -__attribute__((sysio_wasm_import)) int64_t add_security_group_participants(const char* data, uint32_t datalen); - -__attribute__((sysio_wasm_import)) int64_t remove_security_group_participants(const char* data, uint32_t datalen); - -__attribute__((sysio_wasm_import)) bool in_active_security_group(const char* data, uint32_t datalen); - -__attribute__((sysio_wasm_import)) uint32_t get_active_security_group(char* data, uint32_t datalen); -} -} // namespace internal_use_do_not_use - -/** - * @defgroup security_group Security Group - * @ingroup contracts - * @brief Defines C++ security group API - */ - -struct security_group { - uint32_t version; - std::set participants; - CDT_REFLECT(version, participants); -}; - -/** - * Propose new participants to the security group. - * - * @ingroup security_group - * @param participants - the participants. - * - * @return -1 if proposing a new security group was unsuccessful, otherwise returns 0. - */ -inline int64_t add_security_group_participants(const std::set& participants) { - auto packed_participants = sysio::pack( participants ); - return internal_use_do_not_use::add_security_group_participants( packed_participants.data(), packed_participants.size() ); -} - -/** - * Propose to remove participants from the security group. - *å - * @ingroup security_group - * @param participants - the participants. - *å - * @return -1 if proposing a new security group was unsuccessful, otherwise returns 0. - */ -inline int64_t remove_security_group_participants(const std::set& participants){ - auto packed_participants = sysio::pack( participants ); - return internal_use_do_not_use::remove_security_group_participants( packed_participants.data(), packed_participants.size() ); -} - -/** - * Check if the specified accounts are all in the active security group. - * - * @ingroup security_group - * @param participants - the participants. - * - * @return Returns true if the specified accounts are all in the active security group. - */ -inline bool in_active_security_group(const std::set& participants){ - auto packed_participants = sysio::pack( participants ); - return internal_use_do_not_use::in_active_security_group( packed_participants.data(), packed_participants.size() ); -} - -/** - * Gets the active security group - * - * @ingroup security_group - * @param[out] packed_security_group - the buffer containing the packed security_group. - * - * @return Returns the size required in the buffer (if the buffer is too small, nothing is written). - * - */ -inline security_group get_active_security_group() { - size_t buffer_size = internal_use_do_not_use::get_active_security_group(0, 0); - std::vector buffer(buffer_size); - internal_use_do_not_use::get_active_security_group(buffer.data(), buffer_size); - return sysio::unpack(buffer); -} -} // namespace sysio \ No newline at end of file diff --git a/plugins/sysio/abigen.hpp b/plugins/sysio/abigen.hpp index 5e7882a40..eb1f4c715 100644 --- a/plugins/sysio/abigen.hpp +++ b/plugins/sysio/abigen.hpp @@ -1072,7 +1072,7 @@ namespace sysio { namespace cdt { o["variants"].push_back(variant_to_json( v )); } o["abi_extensions"] = ojson::array(); - if (_abi.version_major == 1 && _abi.version_minor >= 2) { + if (abi_version::supports_action_results(_abi.version_major, _abi.version_minor)) { o["action_results"] = ojson::array(); for ( auto ar : _abi.action_results ) { o["action_results"].push_back(action_result_to_json( ar )); @@ -1461,9 +1461,13 @@ namespace sysio { namespace cdt { output = arg.substr(arg.find("=")+1); } else if (sysio::cdt::starts_with(arg, "abi_version=")) { auto str = arg.substr(arg.find("=")+1); - float tmp; - int abi_version_major = std::stoi(str); - int abi_version_minor = (int)(std::modf(std::stof(str), &tmp) * 10); + int abi_version_major = abi_version::default_major; + int abi_version_minor = abi_version::default_minor; + if (!abi_version::parse(str, abi_version_major, abi_version_minor)) { + llvm::errs() << "sysio_abigen: invalid abi_version '" << str + << "': expected [.]\n"; + return false; + } abigen::get().set_abi_version(abi_version_major, abi_version_minor); } else if (arg == "no_abigen") { abigen::get().no_abigen = true; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0ed05d0bd..ac1efd630 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -33,13 +33,25 @@ add_test(NAME version_tests COMMAND ${CMAKE_BINARY_DIR}/tests/unit/version_tests set_property(TEST version_tests PROPERTY LABELS unit_tests) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/unit/abi_version_tests.sh ${CMAKE_BINARY_DIR}/tests/unit/abi_version_tests.sh COPYONLY) -add_test(NAME abi_version_tests COMMAND ${CMAKE_BINARY_DIR}/tests/unit/abi_version_tests.sh "${CMAKE_BINARY_DIR}" WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) +add_test(NAME abi_version_tests COMMAND ${CMAKE_BINARY_DIR}/tests/unit/abi_version_tests.sh "${CMAKE_BINARY_DIR}" "${CMAKE_SOURCE_DIR}" "${VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/include" WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST abi_version_tests PROPERTY LABELS unit_tests) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/unit/multidir_contract_tests.sh ${CMAKE_BINARY_DIR}/tests/unit/multidir_contract_tests.sh COPYONLY) add_test(NAME multidir_contract_tests COMMAND ${CMAKE_BINARY_DIR}/tests/unit/multidir_contract_tests.sh "${CMAKE_BINARY_DIR}" WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST multidir_contract_tests PROPERTY LABELS unit_tests) +# cdt-abidiff ABI-version handling — the version must be read from parsed components, not a +# fixed-width suffix read that mis-scores a two-digit minor. +configure_file(${CMAKE_CURRENT_SOURCE_DIR}/unit/abidiff_tests.sh ${CMAKE_BINARY_DIR}/tests/unit/abidiff_tests.sh COPYONLY) +add_test(NAME abidiff_tests COMMAND ${CMAKE_BINARY_DIR}/tests/unit/abidiff_tests.sh "${CMAKE_BINARY_DIR}/bin" WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) +set_property(TEST abidiff_tests PROPERTY LABELS unit_tests) + +# Staged-header hygiene — every header under /include must still exist in +# libraries/. Catches a stale copy left behind when a source header is deleted. +configure_file(${CMAKE_CURRENT_SOURCE_DIR}/unit/staged_headers_tests.sh ${CMAKE_BINARY_DIR}/tests/unit/staged_headers_tests.sh COPYONLY) +add_test(NAME staged_headers_tests COMMAND ${CMAKE_BINARY_DIR}/tests/unit/staged_headers_tests.sh "${CMAKE_BINARY_DIR}" "${CMAKE_SOURCE_DIR}" "$" WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) +set_property(TEST staged_headers_tests PROPERTY LABELS unit_tests) + # sysio-pp (WABT post-pass) regression — guards the WSA-020 / SEC-10 # FillFromSegments out-of-bounds write and basic post-pass correctness. configure_file(${CMAKE_CURRENT_SOURCE_DIR}/unit/postpass_tests.sh ${CMAKE_BINARY_DIR}/tests/unit/postpass_tests.sh COPYONLY) diff --git a/tests/unit/abi_version_tests.sh b/tests/unit/abi_version_tests.sh index 25231dd7c..a33de9944 100755 --- a/tests/unit/abi_version_tests.sh +++ b/tests/unit/abi_version_tests.sh @@ -1,9 +1,11 @@ #!/bin/bash # Test ABI version and protobuf_types generation -# Usage: abi_version_tests.sh +# Usage: abi_version_tests.sh [source_dir] [magic_enum_include_dir] set -euo pipefail BUILD_DIR="$1" +SOURCE_DIR="${2:-}" +MAGIC_ENUM_INC="${3:-}" CONTRACTS_DIR="${BUILD_DIR}/tests/unit/test_contracts" PASS=0 FAIL=0 @@ -21,6 +23,9 @@ check() { fi } +fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } +pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } + check_absent() { local desc="$1" file="$2" pattern="$3" if grep -q "$pattern" "$file" 2>/dev/null; then @@ -85,6 +90,421 @@ check "proto syntax is proto3" \ "${CONTRACTS_DIR}/pb_tests.abi" \ '"syntax": "proto3"' +# --- -abi-version parsing ------------------------------------------------------ +# +# The driver used to derive the minor by float round-trip: +# +# float tmp = std::stof(v); minor = (int)((tmp - (int)tmp) * 10); +# +# which truncates whenever the decimal has no exact binary expansion. "1.3" +# parsed as minor 2 and "1.4" as minor 3, so `-abi-version 1.3` silently emitted +# sysio::abi/1.2. Both driver and cdt-codegen now parse the components as +# integers from a single shared implementation (abi_version::parse). These cases +# pin that: they FAIL on the float arithmetic and pass on integer parsing. +echo "-- -abi-version parsing --" + +CDT_CPP="${BUILD_DIR}/bin/cdt-cpp" +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +cat > "${WORK}/verparse.cpp" <<'CONTRACT' +#include +class [[sysio::contract("verparse")]] verparse : public sysio::contract { +public: + using contract::contract; + [[sysio::action]] void hi(sysio::name nm) { (void)nm; } +}; +CONTRACT + +check_emitted_version() { + local requested="$1" expected="$2" + local abi="${WORK}/v${expected}.abi" + local desc="-abi-version ${requested} emits sysio::abi/${expected}" + local args=(-abigen -contract verparse "-abigen_output=${abi}" + "${WORK}/verparse.cpp" -o "${WORK}/v${expected}.wasm") + [ -n "$requested" ] && args=(-abi-version "$requested" "${args[@]}") + + if ! "$CDT_CPP" "${args[@]}" > "${WORK}/build.log" 2>&1; then + fail "$desc (build failed)" + sed 's/^/ /' "${WORK}/build.log" + return + fi + check "$desc" "$abi" "\"version\": \"sysio::abi/${expected}\"" +} + +check_emitted_version "" "1.2" # no flag -> the toolchain baseline +check_emitted_version "1.2" "1.2" +check_emitted_version "1.3" "1.3" # float parse gave 1.2 here +check_emitted_version "1.4" "1.4" # float parse gave 1.3 here +check_emitted_version "1.10" "1.10" # two-digit minor: see below + +# A two-digit minor exercised three separate parsers, each of which got it wrong: +# the driver's stof/modf, the abigen plugin's stof/modf, and ABIMerger deriving the +# version from the string's last three characters (".10" -> 0.10). The last one also +# silently dropped action_results, because 0.10*10 failed its >= 12 gate. All three +# now share abi_version::parse / parse_version_string, so the section must survive. +check "1.10 keeps action_results (ABIMerger no longer parses the suffix)" \ + "${WORK}/v1.10.abi" \ + '"action_results"' + +# There is no ABI 0.x, so 0.1 must be rejected rather than coerced. cdt-cpp no longer treats +# a zero major as "option absent" -- it records and forwards the parsed version +# unconditionally -- and this rejection is what lets it do that, so the case still guards the +# driver/codegen divergence, now by keeping the sentinel unnecessary rather than by feeding it. +# 2.0 and 10.2 are rejected rather than accepted: abigen's to_json only serializes +# action_results when major == 1, so a higher major would be stamped onto an ABI missing +# the sections that version implies, and the merger would rank it above 1.2 regardless. +for bad in "not-a-version" "0.1" "1.2.3" "1x" "2.0" "10.2"; do + if "$CDT_CPP" -abi-version "$bad" -abigen -contract verparse \ + "-abigen_output=${WORK}/bad.abi" "${WORK}/verparse.cpp" \ + -o "${WORK}/bad.wasm" > "${WORK}/bad.log" 2>&1; then + fail "-abi-version ${bad} is rejected" + elif grep -q "invalid -abi-version" "${WORK}/bad.log"; then + echo " PASS: -abi-version ${bad} is rejected with a diagnostic" + PASS=$((PASS + 1)) + else + fail "-abi-version ${bad} rejected, but without the expected diagnostic" + sed 's/^/ /' "${WORK}/bad.log" + fi +done + +# --- protobuf version promotion --------------------------------------------------- +# +# A contract with protobuf files is stamped at 1.3. That promotion used to happen after +# the abigen plugin had already run, so `-abi-version 1.1` made the plugin suppress +# action_results under its own 1.2 gate, and codegen then stamped the incomplete output +# as 1.3 -- a version promising a section the descriptors no longer carried. The +# promotion now happens before the plugin is told the version, so a non-void protobuf +# action keeps its result entry. +echo "-- protobuf version promotion --" + +# The magic_enum include directory is passed in from CMake, which already resolves it. +# Searching for it here found `vcpkg_installed//share/magic_enum` as readily as +# the include/ one -- find(1) does not order its matches -- and the share copy has no +# headers under it. +PB_SRC="${SOURCE_DIR:-}" +PB_GEN="${BUILD_DIR}/tests/unit/test_contracts" + +if [ -z "$PB_SRC" ] || [ ! -f "${PB_SRC}/tests/unit/test_contracts/pb_tests.cpp" ] \ + || [ ! -d "${PB_GEN}/test" ] \ + || [ -z "$MAGIC_ENUM_INC" ] || [ ! -f "${MAGIC_ENUM_INC}/magic_enum/magic_enum.hpp" ]; then + echo " SKIP: protobuf inputs not locatable in this build tree" +else + if "$CDT_CPP" -abigen -abi-version 1.1 -contract pb_tests \ + -protobuf-dir "${PB_SRC}/tests/unit/test_contracts" -protobuf-files test.proto \ + -I "$PB_GEN" -I "${PB_SRC}/tests/unit/test_contracts" -I "$MAGIC_ENUM_INC" \ + "-abigen_output=${WORK}/pb11.abi" "${PB_SRC}/tests/unit/test_contracts/pb_tests.cpp" \ + -o "${WORK}/pb11.wasm" > "${WORK}/pb11.log" 2>&1; then + check "1.1 + protobuf is promoted to 1.3" \ + "${WORK}/pb11.abi" '"version": "sysio::abi/1.3"' + # Must key on result_type: "name": "hiproto" also appears in the top-level actions + # array, so asserting the name passed even when the old late-promotion path had + # suppressed action_results entirely -- i.e. it passed on the regression it exists + # to catch. result_type appears only under action_results. + check "1.1 + protobuf keeps the non-void action's result" \ + "${WORK}/pb11.abi" '"result_type": "protobuf::test.ActResult"' + else + fail "1.1 + protobuf builds" + sed 's/^/ /' "${WORK}/pb11.log" + fi +fi + +# --- mixed-version descriptor merge ----------------------------------------------- +# +# Sections enter the format at a version, so a valid 1.1 descriptor omits action_results. +# Once the capability gate consults the MERGED version, such a descriptor merged with a +# newer one reaches merge_action_results, and indexing the older side unconditionally threw +# `Key 'action_results' not found` -- failing the very mixed-version case the gate enables. +# Driven through `cdt-codegen --finalize`, which is the real ABIMerger entry point. +echo "-- mixed-version descriptor merge --" + +CDT_CODEGEN="${BUILD_DIR}/bin/cdt-codegen" +MERGE_COMMON='"types":[],"tables":[],"ricardian_clauses":[],"variants":[],"abi_extensions":[],"pb_types":[],"wasm_actions":[],"wasm_entries":[],"wasm_notifies":[]' + +cat > "${WORK}/old.desc" < "${WORK}/new.desc" < "${dir}/mix.log" 2>&1; then + check "${label}: emits the newer version" "${dir}/mix.abi" '"version": "sysio::abi/1.10"' + # result_type, not name: "actb" is in the actions array too, so asserting the name + # would pass even with action_results dropped entirely. + check "${label}: retains the newer side's action_result" "${dir}/mix.abi" '"result_type": "uint64"' + else + fail "${label}: descriptors merge" + sed 's/^/ /' "${dir}/mix.log" + fi +} + +merge_case "older-first" old new 1.1 +merge_case "newer-first" new old 1.1 + +# --- matcher identity --------------------------------------------------------------- +# +# variant_is_same asked only whether every type in one variant appeared somewhere in the +# other, with no length check, so ["uint64"] and ["uint64","string"] compared equal. Merging +# them kept the accumulator's shorter list and dropped the `string` alternative outright -- +# or, with the descriptors in the other order, failed the build with "v already defined". +# Which of the two you got was decided by sorted .desc filename order. +# +# struct_is_same matched fields by set membership plus size, so the same struct declared with +# reordered fields merged as identical and the alphabetically-first .desc silently won. ABI +# field order is serialization order, so that is a wire-layout change decided by a filename. +VAR_COMMON='"types":[],"tables":[],"ricardian_clauses":[],"abi_extensions":[],"pb_types":[],"wasm_actions":[],"wasm_entries":[],"wasm_notifies":[],"action_results":[]' + +mkdesc_variant() { # $1=path $2=types-json + cat > "$1" < "$1" < "${dir}/mix.log" 2>&1; then + fail "${label} (${order} order merged instead of refusing)" + sed 's/^/ /' "${dir}/mix.abi" + else + pass "${label} (${order} order)" + fi + done +} + +# A populated gated section PROMOTES the emitted version rather than being dropped. Master +# emitted `variants` unconditionally, so a 1.0 build produced a document stamped 1.0 that +# carried a section the format introduced at 1.1 -- inconsistent, not lossy. Gating the section +# on the requested version would have made it lossy instead, since the struct field keeps +# referencing `variant_uint64_string` after the array defining it is dropped. Promoting the +# stamp is the only option that is neither. Asserted end to end, because the interesting part +# is abigen and the merger agreeing. +promo_dir="${WORK}/promote_1_0"; mkdir -p "$promo_dir" +cat > "${promo_dir}/v.cpp" <<'EOF' +#include +#include +#include +using namespace sysio; +class [[sysio::contract]] v : public contract { public: using contract::contract; + [[sysio::action]] void go(std::variant p) { (void)p; } +}; +EOF +if (cd "$promo_dir" && "${BUILD_DIR}/bin/cdt-cpp" -abigen -abigen_output=v.abi -contract=v \ + -abi-version 1.0 -o v.wasm v.cpp) > "${promo_dir}/build.log" 2>&1; then + check "a variant at -abi-version 1.0 promotes to 1.1" \ + "${promo_dir}/v.abi" '"version": "sysio::abi/1.1"' + check "the promoted document still defines the variant it references" \ + "${promo_dir}/v.abi" '"name": "variant_uint64_string"' +else + fail "a variant at -abi-version 1.0 builds" + sed 's/^/ /' "${promo_dir}/build.log" +fi + +# "version" is the first key of every ABI this toolchain emits and ojson preserves insertion +# order, so assigning it after the sections moved it to the end of the object -- changing the +# bytes of every contract's ABI. The abigen-pass fixtures pin this too; asserted here as well +# because the merger is where the ordering is decided. +if [ "$(head -3 "${promo_dir}/v.abi" | grep -c '"version"')" -eq 1 ]; then + pass "version stays the leading key of the emitted ABI" +else + fail "version stays the leading key of the emitted ABI" + sed 's/^/ /' <<< "$(head -3 "${promo_dir}/v.abi")" +fi + +# --- merger: tables ------------------------------------------------------------------ +# +# The merger's table path had NO coverage at all -- every merge fixture above uses +# "tables":[] -- so a regression there was invisible to ctest. +# +# The case that matters is a multi-file contract. abigen writes `secondary_indexes` only when +# non-empty and `key_names` only when the indexed instantiation is visible, so a translation +# unit that sees a table's [[sysio::table]] but not its `kv::index` instantiation emits a +# descriptor with those keys absent or empty. That must merge with the richer one, in either +# order, and keep the richer metadata. It must NOT depend on which .desc sorts first. +TBL_COMMON='"types":[],"actions":[],"ricardian_clauses":[],"variants":[],"abi_extensions":[],"pb_types":[],"wasm_actions":[],"wasm_entries":[],"wasm_notifies":[],"action_results":[],"structs":[]' + +cat > "${WORK}/t_rich.desc" < "${WORK}/t_poor.desc" < "${dir}/mix.log" 2>&1; then + # Each of the three lists is copied independently, so each needs its own assertion. + # Checking only key_names and secondary_indexes left key_types pinned by nothing: + # dropping it from the merge loop kept every case green while "uint64" vanished. + check "${label}: keeps key_names" "${dir}/mix.abi" '"id"' + check "${label}: keeps key_types" "${dir}/mix.abi" '"uint64"' + check "${label}: keeps secondary_indexes" "${dir}/mix.abi" '"byowner"' + else + fail "${label}: descriptors merge" + sed 's/^/ /' "${dir}/mix.log" + fi +} +merge_table_case "partial table desc, rich first" "${WORK}/t_rich.desc" "${WORK}/t_poor.desc" +merge_table_case "partial table desc, poor first" "${WORK}/t_poor.desc" "${WORK}/t_rich.desc" + +# Split richness: each descriptor carries one optional list the other lacks. Replacing the +# accumulator wholesale -- as an earlier revision did on any of the three keys -- discarded +# whichever list the accumulator was richer in, so the result depended on which .desc sorted +# first. Merging per key gives the union either way. +cat > "${WORK}/t_keys_only.desc" < "${WORK}/t_idx_only.desc" < "${WORK}/t_otherid.desc" +merge_refuses_both_orders "tables with different table_ids conflict" \ + "${WORK}/t_rich.desc" "${WORK}/t_otherid.desc" + +# A descriptor declaring a foreign ABI namespace must not produce one. Ordering ignores the +# prefix by design, so an eosio:: descriptor can be ingested -- but merge_version returned the +# winning document's RAW string, so the merged ABI was stamped eosio::abi/1.10, which the +# runtime's abi_serializer rejects outright. Nothing reached ABIMerger with a foreign prefix +# before this: the eosio:: fixture in abidiff_tests exercises only the differ. +printf '{"version":"sysio::abi/1.2",%s,"tables":[]}\n' "$TBL_COMMON" > "${WORK}/ns_local.desc" +printf '{"version":"eosio::abi/1.10",%s,"tables":[]}\n' "$TBL_COMMON" > "${WORK}/ns_foreign.desc" +ns_dir="${WORK}/ns"; mkdir -p "$ns_dir" +cp "${WORK}/ns_local.desc" "${ns_dir}/a_first.desc" +cp "${WORK}/ns_foreign.desc" "${ns_dir}/b_second.desc" +if "$CDT_CODEGEN" --finalize --contract ns --output-dir "$ns_dir" \ + --abi-output "${ns_dir}/ns.abi" \ + --desc-file "${ns_dir}/a_first.desc" --desc-file "${ns_dir}/b_second.desc" \ + > "${ns_dir}/ns.log" 2>&1; then + check "a foreign ABI namespace is canonicalised on merge" \ + "${ns_dir}/ns.abi" '"version": "sysio::abi/1.10"' + check_absent "the merged ABI carries no foreign namespace" \ + "${ns_dir}/ns.abi" 'eosio::abi' +else + fail "a descriptor with a foreign namespace merges" + sed 's/^/ /' "${ns_dir}/ns.log" +fi + +mkdesc_variant "${WORK}/v_short.desc" '["uint64"]' +mkdesc_variant "${WORK}/v_long.desc" '["uint64","string"]' +merge_refuses_both_orders "variants of differing length conflict" \ + "${WORK}/v_short.desc" "${WORK}/v_long.desc" + +mkdesc_struct "${WORK}/s_ab.desc" '[{"name":"a","type":"uint64"},{"name":"b","type":"string"}]' +mkdesc_struct "${WORK}/s_ba.desc" '[{"name":"b","type":"string"},{"name":"a","type":"uint64"}]' +merge_refuses_both_orders "reordered struct fields conflict" \ + "${WORK}/s_ab.desc" "${WORK}/s_ba.desc" + +# A descriptor missing a REQUIRED section is truncated, not merely older, and must be +# rejected rather than merged as empty -- otherwise contract interface content is dropped +# silently. Baseline sections are required at every version; `variants` and `action_results` +# are required from the version that introduced them (1.1 and 1.2) and may be absent only +# below it; `enums` alone is optional everywhere, being emitted only when non-empty. +cat > "${WORK}/truncated.desc" < "${WORK}/trunc.log" 2>&1; then + fail "a descriptor missing a required section is rejected" +else + pass "a descriptor missing a required section is rejected" +fi + +# Each validation rule needs its own case: without these, reverting either the versionless +# rejection or the per-version thresholds leaves the suite green. +# +# $1 label, $2 expected outcome (accept|reject), $3 seed version, $4 descriptor JSON, +# $5 expected diagnostic fragment when rejecting. +validation_case() { + local label="$1" expect="$2" seed="$3" body="$4" diag="${5:-}" + local dir="${WORK}/v_${label// /_}" + mkdir -p "$dir" + printf '%s\n' "$body" > "${dir}/d.desc" + if "$CDT_CODEGEN" --finalize --contract vcase --output-dir "$dir" \ + --abi-version "$seed" --abi-output "${dir}/out.abi" \ + --desc-file "${dir}/d.desc" > "${dir}/log" 2>&1; then + if [ "$expect" = "accept" ]; then pass "$label"; else fail "$label (accepted)"; fi + else + if [ "$expect" = "reject" ]; then + if [ -z "$diag" ] || grep -q "$diag" "${dir}/log"; then + pass "$label" + else + fail "$label (rejected, but not with the expected diagnostic)" + sed 's/^/ /' "${dir}/log" + fi + else + fail "$label (rejected)" + sed 's/^/ /' "${dir}/log" + fi + fi +} + +BASE='"structs":[],"types":[],"actions":[],"tables":[],"ricardian_clauses":[],"abi_extensions":[],"pb_types":[],"wasm_actions":[],"wasm_entries":[],"wasm_notifies":[]' + +validation_case "a versionless descriptor is rejected" reject 1.2 \ + "{${BASE},\"variants\":[],\"action_results\":[]}" "missing its version" + +validation_case "1.0 may omit variants and action_results" accept 1.0 \ + "{\"version\":\"sysio::abi/1.0\",${BASE}}" + +validation_case "1.1 requires variants" reject 1.1 \ + "{\"version\":\"sysio::abi/1.1\",${BASE},\"action_results\":[]}" "missing section : variants" + +validation_case "1.1 may omit action_results" accept 1.1 \ + "{\"version\":\"sysio::abi/1.1\",${BASE},\"variants\":[]}" + +validation_case "1.2 requires action_results" reject 1.2 \ + "{\"version\":\"sysio::abi/1.2\",${BASE},\"variants\":[]}" "missing section : action_results" + echo "" echo "Results: ${PASS} passed, ${FAIL} failed" [ "$FAIL" -eq 0 ] \ No newline at end of file diff --git a/tests/unit/abidiff_tests.sh b/tests/unit/abidiff_tests.sh new file mode 100755 index 000000000..ad506ecc2 --- /dev/null +++ b/tests/unit/abidiff_tests.sh @@ -0,0 +1,642 @@ +#!/bin/bash +# Regression tests for cdt-abidiff's ABI version handling. +# +# get_version used to be `stod(ver.substr(ver.size() - 3)) * 10`, a fixed-width suffix +# read that returns 1 for "sysio::abi/1.10" (it sees ".10"). Version gates in diff() +# compared that against 11 and 12, so for any two-digit minor the variant and action-result +# diffs were silently skipped -- a real difference reported as none. The same suffix read +# also collapsed "eosio::abi/1.2" and "sysio::abi/1.2" to one number. +# +# Both halves of that are fixed. The version is now read with the shared +# abi_version::parse_version_string, and the gates are gone entirely: every section is +# diffed unconditionally, so a section one document carries and the other does not is +# reported whatever version either side declares. The parse survives because the version +# string is itself compared, and because an unreadable one stops the run. +# +# Usage: abidiff_tests.sh +set -euo pipefail + +BIN_DIR="$1" +ABIDIFF="${BIN_DIR}/cdt-abidiff" +PASS=0 +FAIL=0 + +pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } + +# Valid input: cdt-abidiff must exit 0 whether or not it reports differences. A non-zero +# status is a crash or a rejection, never "found a difference", so it propagates to the +# caller instead of being folded into the output text with `|| true`. +run_abidiff() { "$ABIDIFF" "$@" 2>&1; } + +# Capture output, failing the named case outright if the process did not exit 0. +# Sets `out`; returns non-zero when the case has already been failed. +capture() { + local desc="$1"; shift + out="$(run_abidiff "$@")" && return 0 + fail "${desc} (cdt-abidiff exited non-zero)" + sed 's/^/ /' <<< "$out" + return 1 +} +fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } + +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +# Two ABIs identical but for one action_result entry. At any version that supports the +# section, the difference must be reported. +write_pair() { + local version="$1" + cat > "${WORK}/a.abi" < "${WORK}/b.abi" < "${WORK}/v1.abi" <<'EOF' +{ "version": "sysio::abi/1.2", "types": [], "structs": [], "actions": [], "tables": [], "ricardian_clauses": [], "variants": [], "action_results": [] } +EOF +cat > "${WORK}/v2.abi" <<'EOF' +{ "version": "sysio::abi/1.10", "types": [], "structs": [], "actions": [], "tables": [], "ricardian_clauses": [], "variants": [], "action_results": [] } +EOF +if capture "1.2 vs 1.10 reports a version difference" "${WORK}/v1.abi" "${WORK}/v2.abi" && + grep -q "version" <<< "$out"; then + pass "1.2 vs 1.10 reports a version difference" +else + fail "1.2 vs 1.10 reports a version difference" + sed 's/^/ /' <<< "$out" +fi + +# Identical inputs must stay quiet. +if ! capture "identical ABIs report no difference" "${WORK}/v1.abi" "${WORK}/v1.abi"; then + : +elif grep -qE "^[<>] (version|struct|type|action|table|clause|variant|action_result)" <<< "$out"; then + fail "identical ABIs report no difference" + sed 's/^/ /' <<< "$out" +else + pass "identical ABIs report no difference" +fi + +# Reordered but equivalent action_results must NOT report a difference. find_action_results +# compared the matched entry against abi2[...].at(i) instead of .at(j), so once a name matched +# at a different index the result_type comparison read the wrong entry and reported all four +# sides as changed. Pre-existing, but this PR routes 1.10 through that path. +cat > "${WORK}/r1.abi" <<'EOF' +{ "version": "sysio::abi/1.10", "types": [], "structs": [], "actions": [], "tables": [], "ricardian_clauses": [], "variants": [], + "action_results": [ { "name": "geta", "result_type": "uint64" }, { "name": "getb", "result_type": "uint32" } ] } +EOF +cat > "${WORK}/r2.abi" <<'EOF' +{ "version": "sysio::abi/1.10", "types": [], "structs": [], "actions": [], "tables": [], "ricardian_clauses": [], "variants": [], + "action_results": [ { "name": "getb", "result_type": "uint32" }, { "name": "geta", "result_type": "uint64" } ] } +EOF +if ! capture "reordered equivalent action_results report no difference" \ + "${WORK}/r1.abi" "${WORK}/r2.abi"; then + : +elif grep -qE "geta|getb" <<< "$out"; then + fail "reordered equivalent action_results report no difference" + sed 's/^/ /' <<< "$out" +else + pass "reordered equivalent action_results report no difference" +fi + +# An unsupported or unparsable version must be refused, not silently read as the 1.2 default. +# parse() rejects majors above 1, so seeding the outputs with 1.2 and ignoring the result made +# a 2.0 document compare equal to a 1.2 one. +cat > "${WORK}/v20.abi" <<'EOF' +{ "version": "sysio::abi/2.0", "types": [], "structs": [], "actions": [], "tables": [], "ricardian_clauses": [], "variants": [], "action_results": [] } +EOF +if "$ABIDIFF" "${WORK}/v20.abi" "${WORK}/v1.abi" > "${WORK}/v20.log" 2>&1; then + fail "an unsupported ABI version is refused" +elif grep -q "unsupported ABI version" "${WORK}/v20.log"; then + pass "an unsupported ABI version is refused with a diagnostic" +else + fail "unsupported ABI version refused, but without the expected diagnostic" + sed 's/^/ /' "${WORK}/v20.log" +fi + +# Variants at 1.10. find_variants broke out of the element loop on a type mismatch and then +# set found unconditionally, so a same-named variant counted as unchanged however its types +# differed; with no length check, at(k) threw on a shorter right-hand side. This PR is what +# routes 1.10 into that matcher. +mkvariant() { # file, types-json + cat > "$1" < "$1" <&1)"; then + fail "$1" + echo " compared cleanly instead of refusing" + sed 's/^/ /' <<< "$out" + elif grep -q "$4" <<< "$out"; then + pass "$1" + else + fail "$1" + sed 's/^/ /' <<< "$out" + fi +} + +expect_quiet() { # $1=desc $2=a $3=b $4=needle + if ! capture "$1" "$2" "$3"; then : + elif grep -q "$4" <<< "$out"; then fail "$1"; sed 's/^/ /' <<< "$out" + else pass "$1"; fi +} + +mkstruct "${WORK}/s_ab_int.abi" '[{"name":"a","type":"uint64"},{"name":"b","type":"uint64"}]' +mkstruct "${WORK}/s_ab_str.abi" '[{"name":"a","type":"uint64"},{"name":"b","type":"string"}]' +expect_reports "a change in a non-first struct field is reported" \ + "${WORK}/s_ab_int.abi" "${WORK}/s_ab_str.abi" "struct" + +mkstruct "${WORK}/s_first.abi" '[{"name":"z","type":"uint64"},{"name":"b","type":"uint64"}]' +expect_reports "a change in the first struct field is still reported" \ + "${WORK}/s_ab_int.abi" "${WORK}/s_first.abi" "struct" + +expect_quiet "an identical multi-field struct reports no difference" \ + "${WORK}/s_ab_int.abi" "${WORK}/s_ab_int.abi" "struct" + +mkstruct "${WORK}/s_empty1.abi" '[]' +mkstruct "${WORK}/s_empty2.abi" '[]' +expect_quiet "an identical zero-field struct reports no difference" \ + "${WORK}/s_empty1.abi" "${WORK}/s_empty2.abi" "struct" + +mkstruct "${WORK}/s_reorder.abi" '[{"name":"b","type":"uint64"},{"name":"a","type":"uint64"}]' +expect_reports "reordered struct fields are reported (order is serialization order)" \ + "${WORK}/s_ab_int.abi" "${WORK}/s_reorder.abi" "struct" + +# --- table matching -------------------------------------------------------------------- +# +# find_tables compared only name and type, so index_type, key_names, key_types and table_id +# could all change with no difference reported -- the metadata a contract upgrade turns on. + +mktable() { # $1=path $2=index_type $3=key_names $4=key_types $5=table_id + cat > "$1" < "${WORK}/t_type.abi" < "${WORK}/t_sec_a.abi" <<'EOF' +{ + "version": "sysio::abi/1.2", + "types": [], "structs": [], "actions": [], "ricardian_clauses": [], "variants": [], + "action_results": [], + "tables": [ { "name": "t", "type": "row", "index_type": "i64", + "key_names": ["id"], "key_types": ["uint64"], "table_id": 100, + "secondary_indexes": [ { "name": "byowner", "type": "name", "table_id": 37799 } ] } ] +} +EOF +sed 's/37799/60481/' "${WORK}/t_sec_a.abi" > "${WORK}/t_sec_b.abi" +expect_reports "a changed secondary index table_id is reported" \ + "${WORK}/t_sec_a.abi" "${WORK}/t_sec_b.abi" "table" +expect_quiet "an identical table with secondary indexes reports no difference" \ + "${WORK}/t_sec_a.abi" "${WORK}/t_sec_a.abi" "table" + +# --- optional keys --------------------------------------------------------------------- +# +# table_id and secondary_indexes are Wire's additions to table_def: a stock Antelope/eosio-cdt +# ABI omits those two. index_type, key_names and key_types are STANDARD table_def fields and +# are present there -- the t_antelope fixture below carries all three, which is what makes it +# a stock ABI rather than an empty one. Reading an absent key through jsoncons' const +# operator[] throws, so comparing them naively aborted the tool (exit 255) on every such ABI -- +# including two byte-identical ones. capture() already fails a case whose process exits +# non-zero, so these assert the comparison happens at all, not merely that it is quiet. +cat > "${WORK}/t_antelope.abi" <<'EOF' +{ + "version": "sysio::abi/1.2", + "types": [], "structs": [], "actions": [], "ricardian_clauses": [], "variants": [], + "action_results": [], + "tables": [ { "name": "t", "type": "row", "index_type": "i64", + "key_names": ["id"], "key_types": ["uint64"] } ] +} +EOF +expect_quiet "an ABI with no table_id diffs cleanly against itself" \ + "${WORK}/t_antelope.abi" "${WORK}/t_antelope.abi" "table" +expect_reports "an ABI with no table_id differs from one that has it" \ + "${WORK}/t_antelope.abi" "${WORK}/t_base.abi" "table" + +cat > "${WORK}/t_minimal.abi" <<'EOF' +{ + "version": "sysio::abi/1.2", + "types": [], "structs": [], "actions": [], "ricardian_clauses": [], "variants": [], + "action_results": [], + "tables": [ { "name": "t", "type": "row" } ] +} +EOF +expect_quiet "a name+type-only table diffs cleanly against itself" \ + "${WORK}/t_minimal.abi" "${WORK}/t_minimal.abi" "table" + +# A struct with no "base" key, as ABIs from other toolchains emit. +cat > "${WORK}/s_nobase.abi" <<'EOF' +{ + "version": "sysio::abi/1.2", + "types": [], "actions": [], "tables": [], "ricardian_clauses": [], "variants": [], + "action_results": [], + "structs": [ { "name": "s", "fields": [ {"name":"a","type":"uint64"} ] } ] +} +EOF +expect_quiet "a struct with no base key diffs cleanly against itself" \ + "${WORK}/s_nobase.abi" "${WORK}/s_nobase.abi" "struct" + +# --- ricardian clauses ----------------------------------------------------------------- +# +# find_clauses iterates "ricardian_clauses" but print_clause read "clauses", so the tool +# aborted the moment it had a clause difference to report -- it could never report one. +mkclause() { # $1=path $2=body + cat > "$1" < "$1" < "$1" < "${WORK}/upstream.abi" <<'EOF' +{ + "version": "eosio::abi/1.2", + "types": [], "structs": [], "actions": [], "tables": [], "ricardian_clauses": [] +} +EOF +expect_quiet "an ABI missing whole sections diffs cleanly against itself" \ + "${WORK}/upstream.abi" "${WORK}/upstream.abi" "." + +cat > "${WORK}/upstream2.abi" <<'EOF' +{ + "version": "eosio::abi/1.2", + "types": [], "structs": [], "tables": [], "ricardian_clauses": [], + "actions": [ { "name": "act", "type": "act", "ricardian_contract": "" } ] +} +EOF +expect_reports "an ABI missing whole sections still reports a real difference" \ + "${WORK}/upstream2.abi" "${WORK}/upstream.abi" "action" + +# An upstream ABI omits an empty `base` rather than writing ""; absent and "" mean the same +# thing, so that must not read as a difference. +cat > "${WORK}/s_blank_a.abi" <<'EOF' +{ + "version": "sysio::abi/1.2", + "types": [], "actions": [], "tables": [], "ricardian_clauses": [], "variants": [], + "action_results": [], + "structs": [ { "name": "s", "fields": [ {"name":"a","type":"uint64"} ] } ] +} +EOF +sed 's/"name": "s",/"name": "s", "base": "",/' "${WORK}/s_blank_a.abi" > "${WORK}/s_blank_b.abi" +expect_quiet "an omitted base and an empty base are the same struct" \ + "${WORK}/s_blank_a.abi" "${WORK}/s_blank_b.abi" "struct" + +# --- legacy versions --------------------------------------------------------------------- +# +# The variant and action-result diffs were gated on the declared version, which suppressed +# real content: abigen emits `variants` at every version, so two 1.0 documents whose variant +# changed reported nothing. A version stamp says which sections a document must CARRY, not +# which it may contain. +mk_legacy() { # $1=path $2=version $3=extra-json + cat > "$1" < that default-constructs empty, so the chain reads +# an omitted key and an explicit empty array identically. A diff tool that reported them as +# different would be describing a difference the runtime does not see. An earlier revision +# required five sections to be present, which rejected schema-valid minimal documents. +python3 - "${WORK}/upstream.abi" "${WORK}/no_actions.abi" <<'PYEOF' +import json, sys +a = json.load(open(sys.argv[1])); a.pop("actions", None) +json.dump(a, open(sys.argv[2], "w")) +PYEOF +expect_quiet "an omitted section equals an explicit empty one" \ + "${WORK}/no_actions.abi" "${WORK}/upstream.abi" "action" + +cat > "${WORK}/minimal_ok.abi" <<'EOF' +{ "version": "sysio::abi/1.2", "structs": [], "actions": [] } +EOF +expect_quiet "a minimal schema-valid document diffs cleanly against itself" \ + "${WORK}/minimal_ok.abi" "${WORK}/minimal_ok.abi" "." + +# The namespace prefix is deployment-relevant: the runtime accepts only "sysio::abi/1.". +cat > "${WORK}/minimal_eos.abi" <<'EOF' +{ "version": "eosio::abi/1.2", "structs": [], "actions": [] } +EOF +expect_reports "a differing ABI namespace is reported" \ + "${WORK}/minimal_ok.abi" "${WORK}/minimal_eos.abi" "version" + +# --- remaining payload sections ------------------------------------------------------------ +mk_legacy "${WORK}/em1.abi" "sysio::abi/1.2" '"error_messages": []' +mk_legacy "${WORK}/em2.abi" "sysio::abi/1.2" '"error_messages": [ { "error_code": 1, "error_msg": "boom" } ]' +expect_reports "a changed error_messages is reported" "${WORK}/em1.abi" "${WORK}/em2.abi" "error_message" +expect_quiet "an identical error_messages reports no difference" \ + "${WORK}/em1.abi" "${WORK}/em1.abi" "error_message" + +# Consumed into a map keyed by error_code, so order is not significant and omission equals []. +mk_legacy "${WORK}/em_ab.abi" "sysio::abi/1.2" '"error_messages": [ { "error_code": 1, "error_msg": "a" }, { "error_code": 2, "error_msg": "b" } ]' +mk_legacy "${WORK}/em_ba.abi" "sysio::abi/1.2" '"error_messages": [ { "error_code": 2, "error_msg": "b" }, { "error_code": 1, "error_msg": "a" } ]' +expect_quiet "reordered error_messages report no difference" \ + "${WORK}/em_ab.abi" "${WORK}/em_ba.abi" "error_message" +mk_legacy "${WORK}/em_absent.abi" "sysio::abi/1.2" '"variants": []' +expect_quiet "an omitted error_messages equals an empty one" \ + "${WORK}/em_absent.abi" "${WORK}/em1.abi" "error_message" + +mk_legacy "${WORK}/ax1.abi" "sysio::abi/1.2" '"abi_extensions": []' +mk_legacy "${WORK}/ax2.abi" "sysio::abi/1.2" '"abi_extensions": [ [ 1, "00" ] ]' +expect_reports "a changed abi_extensions is reported" "${WORK}/ax1.abi" "${WORK}/ax2.abi" "abi_extensions" +# Omitted equals empty. Without this, reverting diff_opaque_section from section_or_empty back +# to field_or_null leaves every other case green while `null` is printed against `[]`. +mk_legacy "${WORK}/ax_absent.abi" "sysio::abi/1.2" '"variants": []' +expect_quiet "an omitted abi_extensions equals an empty one" \ + "${WORK}/ax_absent.abi" "${WORK}/ax1.abi" "abi_extensions" + +# --- protobuf_types spellings --------------------------------------------------------------- +# +# may_not_exist holding a FileDescriptorSet as JSON. The chain's from_variant accepts +# either a JSON object or a string containing that object, and absent equals an empty string -- +# so all three pairs below are the same ABI and a raw node comparison reported each as changed. +PB_BASE='"version":"sysio::abi/1.3","structs":[],"actions":[]' +printf '{%s,"protobuf_types":{"file":[{"name":"a.proto","package":"t"}]}}\n' "$PB_BASE" > "${WORK}/pb_obj.abi" +printf '{%s,"protobuf_types":"{\\"file\\":[{\\"name\\":\\"a.proto\\",\\"package\\":\\"t\\"}]}"}\n' "$PB_BASE" > "${WORK}/pb_str.abi" +printf '{%s,"protobuf_types":""}\n' "$PB_BASE" > "${WORK}/pb_empty.abi" +printf '{%s}\n' "$PB_BASE" > "${WORK}/pb_absent.abi" +expect_quiet "an object and its JSON-string encoding are the same protobuf_types" \ + "${WORK}/pb_obj.abi" "${WORK}/pb_str.abi" "protobuf_types" +expect_quiet "an omitted protobuf_types equals an empty string" \ + "${WORK}/pb_absent.abi" "${WORK}/pb_empty.abi" "protobuf_types" +expect_reports "a present protobuf_types differs from an absent one" \ + "${WORK}/pb_obj.abi" "${WORK}/pb_absent.abi" "protobuf_types" + +# Invalid roots. JsonStringToMessage requires a message, so a string holding "null" or an +# array is content the chain REJECTS -- decoding it and comparing the result would equate it +# with absence, or with a raw array the chain reads quite differently. Only an object root is +# adopted; everything else stays the string it is. +printf '{%s,"protobuf_types":"null"}\n' "$PB_BASE" > "${WORK}/pb_strnull.abi" +printf '{%s,"protobuf_types":[1,2]}\n' "$PB_BASE" > "${WORK}/pb_rawarr.abi" +printf '{%s,"protobuf_types":"[1,2]"}\n' "$PB_BASE" > "${WORK}/pb_strarr.abi" +expect_reports "a protobuf_types string of \"null\" differs from an absent one" \ + "${WORK}/pb_absent.abi" "${WORK}/pb_strnull.abi" "protobuf_types" +expect_reports "a raw array differs from a string containing that array" \ + "${WORK}/pb_rawarr.abi" "${WORK}/pb_strarr.abi" "protobuf_types" + +# Duplicate object members. jsoncons keeps only the last, so this document would compare equal +# to one carrying just the second -- while fc preserves both and protobuf merges duplicate +# repeated fields, making it [a,b] on chain and [b] here. Refused rather than mis-compared. +printf '{%s,"protobuf_types":{"file":[{"name":"a.proto"}],"file":[{"name":"b.proto"}]}}\n' \ + "$PB_BASE" > "${WORK}/pb_dup.abi" +printf '{%s,"protobuf_types":"{\\"file\\":[{\\"name\\":\\"b.proto\\"}]}"}\n' \ + "$PB_BASE" > "${WORK}/pb_onlyb.abi" +expect_refused "a document with duplicate object members is refused" \ + "${WORK}/pb_dup.abi" "${WORK}/pb_onlyb.abi" "duplicate object member" + +# The same duplicate one level down, inside the STRING spelling -- where the outer walk sees +# only an opaque string value. This passed the check, was then parsed by jsoncons (which keeps +# the last member), and compared EQUAL to a string carrying just b.proto: no output, exit 0. +# JsonStringToMessage reads the original as BOTH descriptors, so that is exactly the runtime- +# visible false negative the detector exists to prevent, and it survived until the check was +# run on the string's own text. +printf '{%s,"protobuf_types":"{\\"file\\":[{\\"name\\":\\"a.proto\\"}],\\"file\\":[{\\"name\\":\\"b.proto\\"}]}"}\n' \ + "$PB_BASE" > "${WORK}/pb_dup_str.abi" +expect_refused "a duplicate member inside a protobuf_types string is refused" \ + "${WORK}/pb_dup_str.abi" "${WORK}/pb_onlyb.abi" "duplicate object member" + +# ...but only for the spelling canonical_protobuf ADOPTS. A string whose JSON root is not an +# object is compared verbatim, as the string it is -- the assertion above pins that -- so +# jsoncons drops nothing from the compared value and this refusal's reason does not hold. +# Refusing it would be stricter than the object spelling rather than equal to it, and would +# take away a document that diffs faithfully. (The chain rejects such content either way, +# JsonStringToMessage needing a message root, which is exactly why it stays a verbatim string.) +printf '{%s,"protobuf_types":"[{\\"a\\":1,\\"a\\":2}]"}\n' "$PB_BASE" > "${WORK}/pb_arrdup.abi" +printf '{%s,"protobuf_types":"[{\\"a\\":9}]"}\n' "$PB_BASE" > "${WORK}/pb_arrother.abi" +expect_reports "a duplicate in a non-object-root protobuf_types string still compares" \ + "${WORK}/pb_arrdup.abi" "${WORK}/pb_arrother.abi" "protobuf_types" + +# --- strict JSON ---------------------------------------------------------------------------- +# +# jsoncons's default handler silently accepts and DISCARDS C/C++ comments; fc rejects them, so +# a commented document is one the chain will not load. Parsing leniently made it compare equal +# to the uncommented document the chain does load -- the difference gone before anything +# compared it, exactly like a dropped duplicate member. +printf '{%s,"protobuf_types":{"file":[]} /* a comment fc rejects */ }\n' "$PB_BASE" \ + > "${WORK}/commented.abi" +printf '{%s,"protobuf_types":{"file":[]}}\n' "$PB_BASE" > "${WORK}/uncommented.abi" +expect_refused "a commented document is refused, not read as equal" \ + "${WORK}/commented.abi" "${WORK}/uncommented.abi" "not strict JSON" + +# A trailing comma is refused too -- and NOT because the chain would refuse it. fc's parser +# consumes any comma it meets (libfc/src/io/json.cpp: the array loop `if (in.peek() == ',') +# { in.get(); continue; }`), so it loads one happily. Refusing is this tool's policy, and it +# predates strict parsing: jsoncons's default handler already propagated `extra_comma`, +# swallowing `illegal_comment` alone, so a lenient build rejects this identically. Pinned so +# the policy is on record as a decision rather than surviving as an accident, and so the +# diagnostic keeps saying whose rule it is. +printf '{%s,}\n' "$PB_BASE" > "${WORK}/trailing_comma.abi" +expect_refused "a trailing comma is refused as tool policy" \ + "${WORK}/trailing_comma.abi" "${WORK}/uncommented.abi" "not strict JSON" +expect_refused "...and the diagnostic does not claim the chain rejects it" \ + "${WORK}/trailing_comma.abi" "${WORK}/uncommented.abi" "this tool's policy" + +# ...and inside the string spelling, where the chain'"'"'s verdict flips between the two: protobuf +# rejects the commented string and accepts the equivalent object, so they are not the same ABI. +# canonical_protobuf adopts a string only when it parses STRICTLY, so the commented one stays +# the string it is and differs from the object. +printf '{%s,"protobuf_types":"{\\"file\\":[]/*c*/}"}\n' "$PB_BASE" > "${WORK}/pb_strcomment.abi" +printf '{%s,"protobuf_types":{"file":[]}}\n' "$PB_BASE" > "${WORK}/pb_fileobj.abi" +expect_reports "a commented protobuf_types string differs from the equivalent object" \ + "${WORK}/pb_strcomment.abi" "${WORK}/pb_fileobj.abi" "protobuf_types" + +# ...while the uncommented string and that object remain the same ABI, as before. +printf '{%s,"protobuf_types":"{\\"file\\":[]}"}\n' "$PB_BASE" > "${WORK}/pb_strplain.abi" +expect_quiet "an uncommented protobuf_types string still equals the object" \ + "${WORK}/pb_strplain.abi" "${WORK}/pb_fileobj.abi" "protobuf_types" + +echo "" +echo "Results: ${PASS} passed, ${FAIL} failed" +[ "$FAIL" -eq 0 ] diff --git a/tests/unit/staged_headers_tests.sh b/tests/unit/staged_headers_tests.sh new file mode 100755 index 000000000..38ec8b5c3 --- /dev/null +++ b/tests/unit/staged_headers_tests.sh @@ -0,0 +1,440 @@ +#!/bin/bash +# Guards the staged header tree under /include against stale files. +# +# Header staging used to be a configure-time file(COPY), which is additive: deleting a +# source header left the staged copy behind forever, so install/CPack kept shipping a +# removed API and native consumers could compile against a header that disagreed with +# the rebuilt library. Reusing a build tree across such a deletion never recovered, +# because the ExternalProject's configure step is stamped and does not re-run. +# +# stage_cdt_tree (cmake/stage_cdt_tree.cmake) now prunes before it copies. This test +# pins the resulting invariant -- every staged header has a source counterpart -- so it +# catches ANY future stale staging, not just the deletion that prompted it. +# +# Usage: staged_headers_tests.sh +# +# The caller passes $, the same canonicalization the stage +# target uses. Comparing the raw cache spelling here meant a valid setting like +# -DENABLE_NATIVE_COMPILER=TRUE staged the native trees while this script took its OFF branch. +set -euo pipefail + +BUILD_DIR="$1" +SOURCE_DIR="$2" +NATIVE_ENABLED="${3:-1}" +INCLUDE_DIR="${BUILD_DIR}/include" +PASS=0 +FAIL=0 + +pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } +fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } + +# Map a staged path back to the source file it was copied from. The three staged trees +# come from two source trees, and one destination nests inside another: +# +# include/sysiolib/native/ <- libraries/native/native/ +# include/sysiolib/ <- libraries/sysiolib/ +# include/sysio/native/ <- libraries/native/ +# +# The sysiolib/native rule is tested first because it is the more specific prefix. +# Maps a staged path back to the source it was copied from, for every tree +# stage_cdt_tree.cmake owns. The vendored four were added when staging took them over from +# their configure-time copies; leaving them out of this map is what let that half of the +# rework ship untested. +source_for() { + local staged="$1" + case "$staged" in + sysiolib/native/*) echo "${SOURCE_DIR}/libraries/native/native/${staged#sysiolib/native/}" ;; + sysiolib/*) echo "${SOURCE_DIR}/libraries/${staged}" ;; + sysio/native/*) echo "${SOURCE_DIR}/libraries/native/${staged#sysio/native/}" ;; + libcxx/*) echo "${SOURCE_DIR}/libraries/libc++/cdt-libcxx/include/${staged#libcxx/}" ;; + bluegrass/*) echo "${SOURCE_DIR}/libraries/meta_refl/include/${staged}" ;; + boost/preprocessor/*) echo "${SOURCE_DIR}/libraries/boost/include/${staged}" ;; + # libc is stitched together from three source roots, so a staged file legitimately + # matches any one of them; first hit wins. + libc/*) + local rest="${staged#libc/}" root + for root in "libraries/libc/cdt-musl/include" \ + "libraries/libc/cdt-musl/src/internal" \ + "libraries/libc/cdt-musl/arch/eos"; do + [ -f "${SOURCE_DIR}/${root}/${rest}" ] && { echo "${SOURCE_DIR}/${root}/${rest}"; return; } + done + # Not found under any root -- report the first so the failure names a real path. + echo "${SOURCE_DIR}/libraries/libc/cdt-musl/include/${rest}" + ;; + *) echo "" ;; + esac +} + +echo "=== Staged Header Tests ===" + +if [ ! -d "$INCLUDE_DIR" ]; then + fail "staged include directory exists (${INCLUDE_DIR})" + echo "Results: ${PASS} passed, ${FAIL} failed" + exit 1 +fi + +# Every tree stage_cdt_tree.cmake owns, vendored ones included. A staged file with no source +# counterpart is one the pruning step failed to remove -- the whole point of the rework. +staged_count=0 +stale=() +while IFS= read -r abs; do + rel="${abs#${INCLUDE_DIR}/}" + src="$(source_for "$rel")" + [ -z "$src" ] && continue + staged_count=$((staged_count + 1)) + [ -f "$src" ] || stale+=("$rel") +# No extension filter on the vendored trees: libc++ ships extensionless headers (, +# ), which a '*.h;*.hpp' find would skip entirely -- and those are exactly the files +# a whole-directory copy stages and an extension-filtered one would have dropped. +done < <( { find "${INCLUDE_DIR}/sysiolib" "${INCLUDE_DIR}/sysio" \ + \( -name '*.h' -o -name '*.hpp' \) -type f 2>/dev/null + find "${INCLUDE_DIR}/libc" "${INCLUDE_DIR}/libcxx" \ + "${INCLUDE_DIR}/boost/preprocessor" "${INCLUDE_DIR}/bluegrass" \ + -type f 2>/dev/null; } ) + +if [ "$staged_count" -eq 0 ]; then + fail "found staged CDT headers to check (none under ${INCLUDE_DIR})" +else + pass "found ${staged_count} staged CDT headers" +fi + +# Per destination, not just in aggregate. A single total cannot show that every tree was +# staged: dropping the bluegrass copy from stage_cdt_tree.cmake leaves the count nonzero on +# the strength of the other five, and the test stays green. +# count_files: 0 for a missing directory rather than a non-zero find. Under `set -euo +# pipefail` an unguarded `find` on an absent path aborts the suite mid-run -- the failure this +# check exists to report is exactly when the path is absent, so it would never be printed. +count_files() { [ -d "$1" ] || { echo 0; return 0; }; find "$1" -type f 2>/dev/null | wc -l; } + +# Every destination stage_cdt_tree.cmake populates in EVERY configuration. Named once because +# three checks walk it -- this build tree, a fresh scratch staging, and the isolated OFF build +# below -- and a tree added to the script but missed in one of them is a gap the others cannot +# report. +readonly NON_NATIVE_DESTS=(sysiolib libc libcxx boost/preprocessor bluegrass) + +# Every native-host archive a native-enabled build stages into lib/, from the POST_BUILD copies +# in libraries/{native,sysiolib,libc,libc++,rt}/CMakeLists.txt. The ON -> OFF prune must remove +# ALL of them: seeding only a couple left `file(GLOB ... libnative*)` free to narrow to those +# names while the rest survived a reconfigure to OFF and were packaged, still carrying the +# previous build's symbols. libnative_c++.a is the one that matters most -- its plus signs are +# what a hand-written character class drops. +readonly NATIVE_ARCHIVES=(libnative.a libnative_sysio.a libnative_c.a libnative_c++.a libnative_rt.a) + +# Count the files under a NON-NATIVE destination, with the native subtree that nests inside +# one of them excluded. Staging copies native/native into include/sysiolib/native, so a plain +# recursive count of include/sysiolib is satisfied by the four native headers alone: gating the +# main sysiolib copy on `NOT STAGE_NATIVE` dropped all 63 regular headers from a native-ON +# build and every assertion stayed green. +count_non_native_files() { + [ -d "$1" ] || { echo 0; return 0; } + find "$1" -type f -not -path '*/sysiolib/native/*' 2>/dev/null | wc -l +} + +# Require each of those to be present AND non-empty under the include root $1, suffixing each +# result with $2. The file count, not the directory: a staging regression that created the +# destinations and copied nothing would ship a package with no public headers at all while a +# directory-existence check stayed green. +require_non_native_dests() { # $1=include root $2=label suffix + local root="$1" label="$2" dest n + for dest in "${NON_NATIVE_DESTS[@]}"; do + n="$(count_non_native_files "${root}/${dest}")" + if [ "$n" -gt 0 ]; then + pass "${dest} is staged${label} (${n} files)" + else + fail "${dest} is staged${label}" + echo " nothing under ${root}/${dest}" + fi + done +} + +require_non_native_dests "$INCLUDE_DIR" "" + +if [ "${#stale[@]}" -eq 0 ]; then + pass "every staged header has a source counterpart" +else + fail "every staged header has a source counterpart" + echo " ${#stale[@]} staged header(s) no longer exist in libraries/:" + for f in "${stale[@]}"; do echo " include/${f}"; done + echo " stage_cdt_tree should have pruned these; see cmake/stage_cdt_tree.cmake" +fi + +# Pruning, per destination -- in an ISOLATED tree. An earlier version planted sentinels in the +# live ${INCLUDE_DIR} and re-ran the staging script there, which wipes and repopulates the very +# headers other tests are compiling against; under `ctest -j` that raced toolchain_tests and +# abi_version_tests. Staging into a scratch destination proves the same property and touches +# nothing shared. +if ! command -v cmake > /dev/null 2>&1; then + echo " SKIP: cmake not on PATH (per-tree prune)" +else + PRUNE_SCRATCH="$(mktemp -d)" + if ! cmake -DSTAGE_SOURCE_DIR="${SOURCE_DIR}/libraries" \ + -DSTAGE_BINARY_DIR="${PRUNE_SCRATCH}" \ + -DSTAGE_NATIVE="${NATIVE_ENABLED}" \ + -P "${SOURCE_DIR}/cmake/stage_cdt_tree.cmake" > "${PRUNE_SCRATCH}/stage.log" 2>&1; then + fail "the staging script populates a fresh tree" + sed 's/^/ /' "${PRUNE_SCRATCH}/stage.log" + else + # What the script produces IN THIS MODE, before any sentinel is planted. The live tree + # checked above is a snapshot of whatever the last build left, so a staging regression + # is invisible there until someone rebuilds; this runs the script and looks at its + # actual output. Gating the main sysiolib copy on `NOT STAGE_NATIVE` -- which drops all + # 63 regular headers from a native-ON build and leaves only the four under + # sysiolib/native -- is caught here and nowhere else. + require_non_native_dests "${PRUNE_SCRATCH}/include" " (fresh staging)" + + planted=0 + for dest in "${NON_NATIVE_DESTS[@]}"; do + if [ -d "${PRUNE_SCRATCH}/include/${dest}" ]; then + : > "${PRUNE_SCRATCH}/include/${dest}/zz_stale_probe.hpp" && planted=$((planted + 1)) + else + fail "fresh staging created ${dest}" + fi + done + if [ "$planted" -ne "${#NON_NATIVE_DESTS[@]}" ]; then + fail "planted a stale sentinel in each staged tree (planted ${planted}, expected ${#NON_NATIVE_DESTS[@]})" + elif ! cmake -DSTAGE_SOURCE_DIR="${SOURCE_DIR}/libraries" \ + -DSTAGE_BINARY_DIR="${PRUNE_SCRATCH}" \ + -DSTAGE_NATIVE="${NATIVE_ENABLED}" \ + -P "${SOURCE_DIR}/cmake/stage_cdt_tree.cmake" > /dev/null 2>&1; then + fail "the staging script re-runs cleanly" + else + survivors="$(find "${PRUNE_SCRATCH}/include" -name 'zz_stale_probe.hpp' 2>/dev/null | wc -l)" + if [ "$survivors" -eq 0 ]; then + pass "a stale file is pruned from every staged tree" + else + fail "a stale file is pruned from every staged tree" + find "${PRUNE_SCRATCH}/include" -name 'zz_stale_probe.hpp' \ + | sed "s|${PRUNE_SCRATCH}/include/| |" + fi + fi + fi + rm -rf "$PRUNE_SCRATCH" +fi + +# With native mode off, the native headers must not be staged at all. They are pruned +# unconditionally rather than inside the STAGE_NATIVE branch, because a build tree whose +# ENABLE_NATIVE_COMPILER flipped ON -> OFF would otherwise keep the previous build's copy +# -- and InstallCDT.cmake installs the whole include tree, so the OFF package would ship +# an API it was configured not to build. The counterpart check above cannot catch that: +# those files still have source counterparts, they simply should not be there. +if [ "$NATIVE_ENABLED" = "1" ]; then + # BOTH destinations: staging owns include/sysio/native and include/sysiolib/native, and + # checking only the first left the second free to disappear with the suite still green. + # A nonzero file count, not merely the directory. A staging-pattern regression that + # created the destinations and copied nothing would ship a package with no native API + # while a directory-existence check stayed green. + missing_native=() + for d in "${INCLUDE_DIR}/sysio/native" "${INCLUDE_DIR}/sysiolib/native"; do + n="$(count_files "$d")" + [ "$n" -gt 0 ] || missing_native+=("$d ($n files)") + done + if [ "${#missing_native[@]}" -eq 0 ]; then + pass "both native header trees are staged and non-empty (native enabled)" + else + fail "both native header trees are staged and non-empty (native enabled)" + for d in "${missing_native[@]}"; do echo " empty or absent: $d"; done + fi +else + leftovers=() + for d in "${INCLUDE_DIR}/sysio/native" "${INCLUDE_DIR}/sysiolib/native"; do + [ -d "$d" ] && leftovers+=("$d") + done + # The native-HOST archives are copied into lib/ by POST_BUILD commands that only exist + # while native mode is on. They survive a reconfigure to OFF, and InstallCDT installs lib/ + # wholesale, so a stale one gets packaged carrying the previous build's symbols. + # + # libnative* only. libsf.a is WebAssembly and is built in every configuration, so it is + # required below rather than forbidden here -- listing it as a leftover contradicted the + # isolated probe, which requires the same file to survive. + for f in "${BUILD_DIR}"/lib/libnative*; do + [ -e "$f" ] && leftovers+=("$f") + done + if [ "${#leftovers[@]}" -eq 0 ]; then + pass "native headers and archives are absent (native disabled)" + else + fail "native headers and archives are absent (native disabled)" + for d in "${leftovers[@]}"; do echo " still staged: $d"; done + fi + + # ...and the wasm softfloat archive must be PRESENT, in this mode as in any other. + if [ -e "${BUILD_DIR}/lib/libsf.a" ]; then + pass "libsf.a is present (native disabled)" + else + fail "libsf.a is present (native disabled)" + echo " cdt-ld links -lsf for --use-rt and the --fquery modes" + fi +fi + +# --- native-disabled configuration, always run ------------------------------------------ +# +# Both workflows leave ENABLE_NATIVE_COMPILER at its ON default, so every OFF assertion above +# is dead in CI -- and the scratch prune probe seeds a fake libsf.a rather than building one, +# so it proves only that pruning spares the file. Re-gating add_subdirectory(native), or the sf +# target inside it, would leave all of that green while a clean OFF package again shipped no +# softfloat archive. +# +# Configuring is enough to catch that and costs seconds: the generated build graph either +# contains the `sf` target or it does not. Building it is left to the OFF matrix leg. +echo "-- native-disabled configuration --" + +if ! command -v cmake > /dev/null 2>&1; then + echo " SKIP: cmake not on PATH" +elif [ ! -f "${BUILD_DIR}/lib/cmake/cdt/CDTWasmToolchain.cmake" ]; then + echo " SKIP: no staged CDT toolchain file to configure against" +else + OFFDIR="$(mktemp -d)" + # out/lib must exist before the build: the archives are staged by POST_BUILD + # `cmake -E copy /lib`, which writes a FILE named lib when the + # directory is absent. The real build tree always has it; an isolated probe must make it. + mkdir -p "${OFFDIR}/out/lib" + if ! cmake -S "${SOURCE_DIR}/libraries" -B "${OFFDIR}" -G Ninja \ + -DCMAKE_BUILD_TYPE=Release \ + -DCMAKE_TOOLCHAIN_FILE="${BUILD_DIR}/lib/cmake/cdt/CDTWasmToolchain.cmake" \ + -DCDT_BIN="${BUILD_DIR}/lib/cmake/cdt/" \ + -DBASE_BINARY_DIR="${OFFDIR}/out" \ + -D__APPLE=FALSE \ + -DENABLE_NATIVE_COMPILER=OFF > "${OFFDIR}/cfg.log" 2>&1; then + fail "the libraries project configures with ENABLE_NATIVE_COMPILER=OFF" + sed 's/^/ /' "${OFFDIR}/cfg.log" + else + pass "the libraries project configures with ENABLE_NATIVE_COMPILER=OFF" + + # BUILD it, do not merely list the targets. `ninja -t targets` shows a declared target + # even when it is EXCLUDE_FROM_ALL, and listing never runs the POST_BUILD copy into + # lib/ -- so either change would leave a listing check green while a default OFF + # package still shipped no softfloat archive. Building the default graph proves the + # target is reachable from `all` AND that the archive is staged. It costs a few + # seconds: these objects are already in the compiler cache from the main build. + if ! ninja -C "${OFFDIR}" > "${OFFDIR}/build.log" 2>&1; then + fail "the libraries project builds with ENABLE_NATIVE_COMPILER=OFF" + tail -20 "${OFFDIR}/build.log" | sed 's/^/ /' + else + pass "the libraries project builds with ENABLE_NATIVE_COMPILER=OFF" + + if [ -f "${OFFDIR}/out/lib/libsf.a" ]; then + pass "an OFF build stages libsf.a" + # ...and it is WebAssembly, not a host archive. cdt-ld links it with -lsf for + # --use-rt and the --fquery modes, so a host-built one would be useless. + probe_dir="${OFFDIR}/probe"; mkdir -p "$probe_dir" + ( cd "$probe_dir" && "${BUILD_DIR}/bin/llvm-ar" x "${OFFDIR}/out/lib/libsf.a" ) \ + > /dev/null 2>&1 || true + # -print -quit, not `| head -1`: head closes the pipe after one line, find + # takes SIGPIPE, and under `set -o pipefail` the assignment fails with 141 -- + # which `set -e` turns into a silent early exit mid-suite. Also parenthesised, + # so the -o binds to the two -name tests rather than to -print. + first_obj="$(find "$probe_dir" \( -name '*.obj' -o -name '*.o' \) -print -quit 2>/dev/null)" + if [ -n "$first_obj" ] && file -b "$first_obj" | grep -qi "webassembly"; then + pass "the staged libsf.a contains WebAssembly objects" + else + fail "the staged libsf.a contains WebAssembly objects" + echo " got: $(file -b "${first_obj:-}" 2>/dev/null)" + fi + else + fail "an OFF build stages libsf.a" + echo " cdt-ld links -lsf for --use-rt and the --fquery modes" + ls "${OFFDIR}/out/lib" 2>/dev/null | sed 's/^/ staged: /' + fi + + # The required header trees ARE staged. Everything else this probe asserts is + # negative -- what an OFF build must not produce -- so removing the OFF sysiolib, + # libc, libcxx, boost/preprocessor and bluegrass outputs altogether left every + # assertion green while the package shipped no public headers. The live tree above + # cannot cover this: it is configured ON, so a regression gated on the OFF branch + # stages them there and is invisible. + require_non_native_dests "${OFFDIR}/out/include" " (native disabled)" + + # ...and no native HEADER tree is staged. Checking lib/ alone does not pin the + # CMake-to-staging wiring: forcing STAGE_NATIVE=1 in libraries/CMakeLists.txt + # leaves these populated in an OFF build, and InstallCDT installs the whole + # include tree, so they would ship. + stray_hdrs=() + for d in "${OFFDIR}/out/include/sysio/native" "${OFFDIR}/out/include/sysiolib/native"; do + n="$(count_files "$d")" + [ "$n" -eq 0 ] || stray_hdrs+=("$d ($n files)") + done + if [ "${#stray_hdrs[@]}" -eq 0 ]; then + pass "an OFF build stages no native header tree" + else + fail "an OFF build stages no native header tree" + for d in "${stray_hdrs[@]}"; do echo " staged: $d"; done + fi + + # ...while no native-HOST archive is produced. [^[:space:]]* rather than [a-z_]*: + # the real targets include libnative_c++.a, whose plus signs a + # letters-and-underscores class silently excludes. + stray_native="$(ls "${OFFDIR}/out/lib" 2>/dev/null | grep -E "^libnative[^[:space:]]*\.a$" || true)" + if [ -z "$stray_native" ]; then + pass "an OFF build stages no libnative* archive" + else + fail "an OFF build stages no libnative* archive" + sed 's/^/ staged: /' <<< "$stray_native" + fi + fi + fi + rm -rf "${OFFDIR}" +fi + +# --- ON -> OFF prune, in an isolated tree ------------------------------------------ +# +# The checks above only describe the mode this build was configured in, and +# ENABLE_NATIVE_COMPILER defaults ON with neither workflow overriding it -- so the OFF +# assertions never ran in CI. A clean OFF build would not prove the prune either: it has no +# stale native outputs to remove. So drive the staging script directly against a scratch tree +# seeded the way a previous ON build leaves one, which is mode-independent and always runs. +echo "-- ON -> OFF prune (isolated tree) --" + +if ! command -v cmake > /dev/null 2>&1; then + echo " SKIP: cmake not on PATH" +else + SCRATCH="$(mktemp -d)" + trap 'rm -rf "$SCRATCH"' EXIT + mkdir -p "${SCRATCH}/lib" "${SCRATCH}/include/sysio/native" "${SCRATCH}/include/sysiolib/native" + for f in "${NATIVE_ARCHIVES[@]}" libsf.a libc.a; do echo stale > "${SCRATCH}/lib/${f}"; done + : > "${SCRATCH}/include/sysio/native/sentinel.hpp" + : > "${SCRATCH}/include/sysiolib/native/sentinel.hpp" + + if cmake -DSTAGE_SOURCE_DIR="${SOURCE_DIR}/libraries" -DSTAGE_BINARY_DIR="${SCRATCH}" \ + -DSTAGE_NATIVE=0 -P "${SOURCE_DIR}/cmake/stage_cdt_tree.cmake" \ + > "${SCRATCH}/stage.log" 2>&1; then + # libnative* and the native header trees only. libsf.a is asserted separately, and + # positively: it is a WebAssembly archive built in every configuration. + leftovers=() + for f in "${NATIVE_ARCHIVES[@]}"; do + [ -e "${SCRATCH}/lib/${f}" ] && leftovers+=("${SCRATCH}/lib/${f}") + done + for f in "${SCRATCH}/include/sysio/native" "${SCRATCH}/include/sysiolib/native"; do + [ -e "$f" ] && leftovers+=("$f") + done + if [ "${#leftovers[@]}" -eq 0 ]; then + pass "STAGE_NATIVE=0 prunes stale native archives and header trees" + else + fail "STAGE_NATIVE=0 prunes stale native archives and header trees" + for f in "${leftovers[@]}"; do echo " survived: $f"; done + fi + + # libsf.a must SURVIVE. It is the WebAssembly softfloat archive cdt-ld links with + # -lsf for --use-rt and the --fquery modes, not a native-host archive. It is declared + # under libraries/native/ but outside that file's native-only guard, so every + # configuration builds it -- the clean OFF probe above asserts exactly that. Pruning + # it here would leave an OFF package unable to link those modes. + if [ -e "${SCRATCH}/lib/libsf.a" ]; then + pass "STAGE_NATIVE=0 keeps libsf.a (a wasm archive, not a native one)" + else + fail "STAGE_NATIVE=0 keeps libsf.a (a wasm archive, not a native one)" + fi + + # An unrelated archive must be left alone -- the prune is targeted, not a wipe. + if [ -e "${SCRATCH}/lib/libc.a" ]; then + pass "the prune leaves unrelated archives alone" + else + fail "the prune leaves unrelated archives alone" + fi + else + fail "stage_cdt_tree.cmake runs with STAGE_NATIVE=0" + sed 's/^/ /' "${SCRATCH}/stage.log" + fi +fi + +echo "" +echo "Results: ${PASS} passed, ${FAIL} failed" +[ "$FAIL" -eq 0 ] diff --git a/tools/abidiff/cdt-abidiff.cpp.in b/tools/abidiff/cdt-abidiff.cpp.in index c4b8654d4..a812dffad 100644 --- a/tools/abidiff/cdt-abidiff.cpp.in +++ b/tools/abidiff/cdt-abidiff.cpp.in @@ -4,7 +4,11 @@ #include #include #include +#include #include +#include + +#include #include @@ -19,26 +23,177 @@ struct abidiff_exception : public std::exception { } abidiff_ex; +// Rejects a document containing duplicate object members. +// +// jsoncons keeps only the last of a repeated key, so a duplicate is gone before any comparison +// sees it -- and it is gone for EVERY section, not only protobuf_types. The runtime does not +// agree: fc preserves both members and protobuf merges duplicate repeated fields, so a +// descriptor set written with two `file` members is [a,b] on chain and [b] here. Comparing such +// a document would silently answer about a value the chain never sees, so it is refused +// instead. The vendored jsoncons has no option to reject duplicates at parse time; this walks +// the same text through its SAX reader and tracks the keys of each open object. +class duplicate_member_detector final : public jsoncons::json_content_handler +{ +public: + bool found() const { return found_; } + const std::string& duplicate() const { return dup_; } + +private: + std::vector> stack_; + std::string dup_; + bool found_ = false; + + void do_begin_document() override {} + void do_end_document() override {} + void do_begin_object(const jsoncons::serializing_context&) override { stack_.emplace_back(); } + void do_end_object(const jsoncons::serializing_context&) override { if (!stack_.empty()) stack_.pop_back(); } + void do_begin_array(const jsoncons::serializing_context&) override {} + void do_end_array(const jsoncons::serializing_context&) override {} + void do_name(const string_view_type& name, const jsoncons::serializing_context&) override { + if (stack_.empty()) + return; + const std::string key(name.data(), name.size()); + if (!stack_.back().insert(key).second && !found_) { + found_ = true; + dup_ = key; + } + } + void do_null_value(const jsoncons::serializing_context&) override {} + void do_string_value(const string_view_type&, const jsoncons::serializing_context&) override {} + void do_byte_string_value(const uint8_t*, size_t, const jsoncons::serializing_context&) override {} + void do_bignum_value(int, const uint8_t*, size_t, const jsoncons::serializing_context&) override {} + void do_double_value(double, const jsoncons::floating_point_options&, const jsoncons::serializing_context&) override {} + void do_integer_value(int64_t, const jsoncons::serializing_context&) override {} + void do_uinteger_value(uint64_t, const jsoncons::serializing_context&) override {} + void do_bool_value(bool, const jsoncons::serializing_context&) override {} +}; + +/// @return the duplicated key, or empty when the text has none. +inline std::string first_duplicate_member(const std::string& text) { + duplicate_member_detector det; + try { + std::istringstream is(text); + jsoncons::json_reader reader(is, det); + reader.read(); + } catch (const std::exception&) { + return {}; // malformed input is diagnosed by the ordinary parse + } + return det.found() ? det.duplicate() : std::string{}; +} + +namespace { + /// The one ABI member whose value may itself be a JSON document: a FileDescriptorSet + /// written either as an object, or as a string containing that object's JSON. + constexpr auto protobuf_types_key = "protobuf_types"; + + /// Parse JSON strictly, refusing anything this tool would otherwise have to normalise. + /// + /// The bug this closes is comments. jsoncons's DEFAULT handler silently accepts and DISCARDS + /// them -- see `default_parse_error_handler`, which swallows `illegal_comment` and nothing + /// else -- while fc rejects them outright. A commented document therefore compared EQUAL to + /// the uncommented one the chain accepts: the same class of false equality as a dropped + /// duplicate member, invisible for the same reason, the difference gone before anything + /// compared it. + /// + /// Strictness beyond that is this TOOL'S POLICY and not a claim about the runtime, which is + /// laxer in places: fc's parser consumes any comma it meets (libfc/src/io/json.cpp), so it + /// loads a trailing one happily. Refusing to compare is the conservative direction -- it + /// never answers "equal" about a document it had to rewrite to read -- and it costs nothing + /// that was working, since the default handler already propagated every error but the + /// comment. Only the comment case changed behaviour here. + /// + /// There is also no single parser to mirror even if that were wanted: the outer document is + /// fc's to read, while a string-valued protobuf_types is JsonStringToMessage's, and their + /// leniencies are not the same. One uniform policy is deliberate. + /// + /// @throws jsoncons::parse_error on a comment, or on any other input strict JSON rejects. + inline ojson parse_strict(const std::string& text) { + jsoncons::strict_parse_error_handler strict; + return ojson::parse(text, strict); + } + + /// Report a document whose comparison would be unfaithful, and exit non-zero. + /// + /// @param which the document as named on the command line -- "file1" or "file2". + /// @param dup the member name that appears more than once. + /// @param where where it appears, empty for the document itself. + [[noreturn]] void refuse_duplicate(const char* which, const std::string& dup, + const std::string& where) { + std::cerr << "Error: " << which << " has a duplicate object member '" << dup << "'" + << where << ". jsoncons keeps only the last, while the chain preserves both and\n" + "protobuf merges duplicate repeated fields -- so any comparison here\n" + "would describe a value the runtime never sees.\n"; + exit(1); + } +} + class abidiff { private: ojson abi_1, abi_2; std::string fn_1, fn_2; + + // Read a document, refusing one whose comparison would be unfaithful. + static ojson load_checked(const std::string& path, const char* which) { + std::ifstream in(path); + std::stringstream buf; + buf << in.rdbuf(); + const std::string text = buf.str(); + const std::string dup = first_duplicate_member(text); + if (!dup.empty()) + refuse_duplicate(which, dup, ""); + + ojson parsed; + try { + parsed = parse_strict(text); + } catch (const std::exception& e) { + std::cerr << "Error: " << which << " is not strict JSON: " << e.what() << "\n" + << "cdt-abidiff refuses what it cannot read strictly, rather than let a\n" + "lenient parse normalise a difference away before anything compares\n" + "it. The runtime's parsers are laxer in places -- fc tolerates a stray\n" + "comma -- so this is this tool's policy, not a verdict on whether the\n" + "chain would load the file.\n"; + exit(1); + } + + // ...and again one level down, inside protobuf_types when it is written as a STRING. + // The walk above sees that spelling as a single opaque string value, so a descriptor + // set carrying two `file` members passes it -- and canonical_protobuf then parses the + // string with jsoncons, which keeps only the last. The document compares EQUAL to one + // carrying just that member, while JsonStringToMessage reads the original as both + // descriptors: the same false negative the outer check exists to prevent, in the one + // place the outer check cannot see. Refused here rather than at the comparison, so a + // document is rejected on the same terms whichever spelling it uses. + // + // ONLY for the spelling canonical_protobuf actually ADOPTS. A string whose JSON root + // is not an object is compared verbatim, as the string it is, so nothing is dropped + // from the compared value and this refusal's own reason would not hold -- refusing it + // would be stricter than the object spelling rather than equal to it, and would take + // away a document that diffs faithfully today. The adoption rule is asked for rather + // than restated, so the two cannot drift apart. + if (parsed.is_object() && parsed.count(protobuf_types_key)) { + const ojson& pb = parsed.at(protobuf_types_key); + if (pb.is_string() && canonical_protobuf(parsed).is_object()) { + const std::string nested = first_duplicate_member(pb.as()); + if (!nested.empty()) + refuse_duplicate(which, nested, " inside protobuf_types"); + } + } + return parsed; + } public: abidiff( const std::string& fn1, const std::string& fn2) { llvm::SmallString<128> _fn1, _fn2; if (!llvm::sys::fs::real_path(fn1, _fn1, true)) { - std::ifstream in(_fn1.str().str()); fn_1 = _fn1.str().str(); - abi_1 = ojson::parse(in); + abi_1 = load_checked(fn_1, "file1"); } else { std::cerr << "Error, invalid filepath { " << _fn1.str().str() << " }\n"; throw abidiff_ex; } if (!llvm::sys::fs::real_path(fn2, _fn2, true)) { - std::ifstream in(_fn2.str().str()); fn_2 = _fn2.str().str(); - abi_2 = ojson::parse(in); + abi_2 = load_checked(fn_2, "file2"); } else { std::cerr << "Error, invalid filepath { " << _fn2.str().str() << " }\n"; throw abidiff_ex; @@ -46,13 +201,44 @@ class abidiff { } - int get_version(const ojson& abi) { - std::string ver = abi["version"].as(); - return (std::stod(ver.substr(ver.size()-3))*10); + /// The (major, minor) an ABI declares. + /// + /// The previous form -- stod over the string's last three characters -- read + /// "sysio::abi/1.10" as ".10" -> 1. Version-gated diffs then skipped the variant and + /// action-result sections for any two-digit minor, reporting a real difference as none. + /// Those gates are gone: every section is diffed unconditionally, since a section one + /// document carries and the other does not is precisely the difference this tool exists + /// to report. The parse remains because the version is itself compared, and because an + /// unreadable one is a reason to stop. + /// + /// A failed parse is refused, not defaulted. Seeding the outputs with 1.2 and ignoring + /// the result meant an unparsable or unsupported version -- including `sysio::abi/2.0`, + /// which the shared parser rejects -- silently compared equal to a 1.2 document. + std::pair get_version(const ojson& abi, const std::string& which) { + if (!abi.has_key("version")) { + std::cerr << "cdt-abidiff: " << which << " has no \"version\" field\n"; + exit(1); + } + const auto text = abi["version"].as(); + int major_v = 0; + int minor_v = 0; + if (!abi_version::parse_version_string(text, major_v, minor_v)) { + std::cerr << "cdt-abidiff: " << which << " declares an unsupported ABI version '" + << text << "'\n"; + exit(1); + } + return {major_v, minor_v}; } + // The FULL string, not just the parsed components. parse_version_string ignores + // everything through the last '/', which is right for ORDERING two versions and wrong + // here: Wire's abi_serializer accepts only a "sysio::abi/1." prefix, so an otherwise + // identical eosio::abi/1.2 is a deployment-relevant difference. The numeric parse still + // runs, to reject an unsupported version. void diff_version() { - if (get_version(abi_1) != get_version(abi_2)) { + (void)get_version(abi_1, "file1"); + (void)get_version(abi_2, "file2"); + if (field_or_null(abi_1, "version") != field_or_null(abi_2, "version")) { std::cout << "< version\n\t"; std::cout << abi_1["version"] << "\n"; std::cout << "> version\n\t"; @@ -62,35 +248,84 @@ class abidiff { void print_struct(const ojson& abi, int index, char direction) { std::cout << direction << " struct\n"; - std::cout << pretty_print(abi["structs"].at(index)) << "\n"; + std::cout << pretty_print(section_or_empty(abi, "structs").at(index)) << "\n"; } - ojson get_base_type(const ojson& abi, const ojson& type) { - for (int i=0; i < abi["types"].size(); i++) { - if (abi["types"].at(i)["new_type_name"] == type) - return abi["types"].at(i)["new_type_name"]; - } - return type; + // An absent key read through the CONST operator[] throws, and a whole SECTION may be + // legitimately absent -- a stock Antelope 1.0/1.1 ABI omits `variants` and + // `action_results` -- so every section is read through this. Absent reads as an empty + // array, which is what "this ABI declares none" means. + static const ojson& section_or_empty(const ojson& o, const char* key) { + static const ojson none = ojson::array(); + return o.count(key) ? o.at(key) : none; + } + + // No section is required. Every list member of abi_def is a vector<> that + // default-constructs empty, so the chain reads an absent key and an explicit empty array + // identically -- reporting them as different would describe a difference the runtime does + // not see. `version` is the exception, and get_version diagnoses a missing or unparsable + // one. + // + // Individual FIELDS are optional too. `table_id` and `secondary_indexes` are Wire's + // additions to `table_def`, so a stock Antelope ABI omits them -- while `index_type`, + // `key_names` and `key_types` are standard and present there, as the t_antelope fixture + // in abidiff_tests.sh carries. Every optional field is read through this, so absent + // compares equal to absent and never equal to present. + + static const ojson& field_or_null(const ojson& o, const char* key) { + static const ojson absent = ojson::null(); + return o.count(key) ? o.at(key) : absent; } + + // Element-wise equality over a JSON array. Used for the ABI's list-valued fields, where + // both length and order are significant -- order is serialization order. + // Absent and empty mean the same thing for these scalars, so an ABI that omits an + // empty `base` or `index_type` is not reported as differing from one that writes "". + static bool scalars_equal(const ojson& a, const ojson& b) { + const auto blank = [](const ojson& v) { + return v.is_null() || (v.is_string() && v.as().empty()); + }; + return a == b || (blank(a) && blank(b)); + } + + static bool arrays_equal(const ojson& a, const ojson& b) { + if (a.size() != b.size()) + return false; + for (size_t k = 0; k < a.size(); ++k) + if (a.at(k) != b.at(k)) + return false; + return true; + } + + // Same base, same fields, in the same order. + // + // The previous form had two defects, both reproduced against real ABIs. It seeded a flag + // false and set it only INSIDE the field loop, so two byte-identical zero-field structs + // -- which every parameterless action generates -- compared as different. And on a field + // mismatch it broke out of the loop WITHOUT clearing the flag, so a difference in any + // field but the first left the flag set from the previous iteration and the changed + // struct was reported as unchanged. + static bool structs_match(const ojson& a, const ojson& b) { + if (!scalars_equal(field_or_null(a, "base"), field_or_null(b, "base"))) + return false; + const auto& fa = field_or_null(a, "fields"); + const auto& fb = field_or_null(b, "fields"); + if (fa.size() != fb.size()) + return false; + for (size_t k = 0; k < fa.size(); ++k) + if (fa.at(k)["name"] != fb.at(k)["name"] || fa.at(k)["type"] != fb.at(k)["type"]) + return false; + return true; + } + void find_structs(const ojson& abi1, const ojson& abi2, char direction) { - for ( int i=0; i < abi1["structs"].size(); i++ ) { + for ( int i=0; i < section_or_empty(abi1, "structs").size(); i++ ) { bool found = false; - for ( int j=0; j < abi2["structs"].size(); j++ ) { - if (abi1["structs"].at(i)["name"] == abi2["structs"].at(j)["name"]) { - if (abi1["structs"].at(i)["fields"].size() != abi2["structs"].at(j)["fields"].size()) - break; - if (abi1["structs"].at(i)["base"] != abi2["structs"].at(j)["base"]) - break; - - bool _found = false; - for (int k=0; k < abi1["structs"].at(i)["fields"].size(); k++) { - if (abi1["structs"].at(i)["fields"].at(k)["name"] != abi2["structs"].at(j)["fields"].at(k)["name"] || - abi1["structs"].at(i)["fields"].at(k)["type"] != abi2["structs"].at(j)["fields"].at(k)["type"]) - break; - _found = true; - } - found = _found; - } + for ( int j=0; j < section_or_empty(abi2, "structs").size(); j++ ) { + if (section_or_empty(abi1, "structs").at(i)["name"] != section_or_empty(abi2, "structs").at(j)["name"]) + continue; + found = structs_match(section_or_empty(abi1, "structs").at(i), section_or_empty(abi2, "structs").at(j)); + break; // the name matched; that entry alone decides } if (!found) print_struct(abi1, i, direction); @@ -99,18 +334,19 @@ class abidiff { void print_type(const ojson& abi, int index, char direction) { std::cout << direction << " type\n"; - std::cout << pretty_print(abi["types"].at(index)) << "\n"; + std::cout << pretty_print(section_or_empty(abi, "types").at(index)) << "\n"; } void find_types(const ojson& abi1, const ojson& abi2, char direction) { - for ( int i=0; i < abi1["types"].size(); i++ ) { + for ( int i=0; i < section_or_empty(abi1, "types").size(); i++ ) { bool found = false; - for ( int j=0; j < abi2["types"].size(); j++ ) { - if (abi1["types"].at(i)["new_type_name"] == abi2["types"].at(j)["new_type_name"]) { - if (abi1["types"].at(i)["type"] != abi2["types"].at(j)["type"]) - break; - found = true; - } + for ( int j=0; j < section_or_empty(abi2, "types").size(); j++ ) { + if (section_or_empty(abi1, "types").at(i)["new_type_name"] != section_or_empty(abi2, "types").at(j)["new_type_name"]) + continue; + if (field_or_null(section_or_empty(abi1, "types").at(i), "type") != field_or_null(section_or_empty(abi2, "types").at(j), "type")) + break; + found = true; + break; } if (!found) print_type(abi1, i, direction); @@ -119,20 +355,21 @@ class abidiff { void print_action(const ojson& abi, int index, char direction) { std::cout << direction << " action\n"; - std::cout << pretty_print(abi["actions"].at(index)) << "\n"; + std::cout << pretty_print(section_or_empty(abi, "actions").at(index)) << "\n"; } void find_actions(const ojson& abi1, const ojson& abi2, char direction) { - for ( int i=0; i < abi1["actions"].size(); i++ ) { + for ( int i=0; i < section_or_empty(abi1, "actions").size(); i++ ) { bool found = false; - for ( int j=0; j < abi2["actions"].size(); j++ ) { - if (abi1["actions"].at(i)["name"] == abi2["actions"].at(j)["name"]) { - if (abi1["actions"].at(i)["type"] != abi2["actions"].at(j)["type"]) - break; - if (abi1["actions"].at(i)["ricardian_contract"] != abi2["actions"].at(j)["ricardian_contract"]) - break; - found = true; - } + for ( int j=0; j < section_or_empty(abi2, "actions").size(); j++ ) { + if (section_or_empty(abi1, "actions").at(i)["name"] != section_or_empty(abi2, "actions").at(j)["name"]) + continue; + if (field_or_null(section_or_empty(abi1, "actions").at(i), "type") != field_or_null(section_or_empty(abi2, "actions").at(j), "type")) + break; + if (field_or_null(section_or_empty(abi1, "actions").at(i), "ricardian_contract") != field_or_null(section_or_empty(abi2, "actions").at(j), "ricardian_contract")) + break; + found = true; + break; } if (!found) print_action(abi1, i, direction); @@ -141,18 +378,35 @@ class abidiff { void print_table(const ojson& abi, int index, char direction) { std::cout << direction << " table\n"; - std::cout << pretty_print(abi["tables"].at(index)) << "\n"; + std::cout << pretty_print(section_or_empty(abi, "tables").at(index)) << "\n"; + } + + // Every field a table entry can carry. Comparing only name and type -- as this did -- + // made the tool silent about the metadata it exists to check: index_type, key_names, + // key_types, table_id and secondary_indexes could all change and it reported no + // difference. table_id is where the row physically lives and each secondary index + // carries its own, so a change to either is a migration. + // + // All of these are optional: read through field_or_null so an ABI that omits them is + // compared, not aborted on. + static bool tables_match(const ojson& a, const ojson& b) { + for (const char* key : {"type", "index_type", "table_id"}) + if (!scalars_equal(field_or_null(a, key), field_or_null(b, key))) + return false; + for (const char* key : {"key_names", "key_types", "secondary_indexes"}) + if (!arrays_equal(field_or_null(a, key), field_or_null(b, key))) + return false; + return true; } void find_tables(const ojson& abi1, const ojson& abi2, char direction) { - for ( int i=0; i < abi1["tables"].size(); i++ ) { + for ( int i=0; i < section_or_empty(abi1, "tables").size(); i++ ) { bool found = false; - for ( int j=0; j < abi2["tables"].size(); j++ ) { - if (abi1["tables"].at(i)["name"] == abi2["tables"].at(j)["name"]) { - if (abi1["tables"].at(i)["type"] != abi2["tables"].at(j)["type"]) - break; - found = true; - } + for ( int j=0; j < section_or_empty(abi2, "tables").size(); j++ ) { + if (section_or_empty(abi1, "tables").at(i)["name"] != section_or_empty(abi2, "tables").at(j)["name"]) + continue; + found = tables_match(section_or_empty(abi1, "tables").at(i), section_or_empty(abi2, "tables").at(j)); + break; } if (!found) print_table(abi1, i, direction); @@ -161,18 +415,19 @@ class abidiff { void print_clause(const ojson& abi, int index, char direction) { std::cout << direction << " clause\n"; - std::cout << pretty_print(abi["clauses"].at(index)) << "\n"; + std::cout << pretty_print(section_or_empty(abi, "ricardian_clauses").at(index)) << "\n"; } void find_clauses(const ojson& abi1, const ojson& abi2, char direction) { - for ( int i=0; i < abi1["ricardian_clauses"].size(); i++ ) { + for ( int i=0; i < section_or_empty(abi1, "ricardian_clauses").size(); i++ ) { bool found = false; - for ( int j=0; j < abi2["ricardian_clauses"].size(); j++ ) { - if (abi1["ricardian_clauses"].at(i)["id"] == abi2["ricardian_clauses"].at(j)["id"]) { - if (abi1["ricardian_clauses"].at(i)["body"] != abi2["ricardian_clauses"].at(j)["body"]) - break; - found = true; - } + for ( int j=0; j < section_or_empty(abi2, "ricardian_clauses").size(); j++ ) { + if (section_or_empty(abi1, "ricardian_clauses").at(i)["id"] != section_or_empty(abi2, "ricardian_clauses").at(j)["id"]) + continue; + if (field_or_null(section_or_empty(abi1, "ricardian_clauses").at(i), "body") != field_or_null(section_or_empty(abi2, "ricardian_clauses").at(j), "body")) + break; + found = true; + break; } if (!found) print_clause(abi1, i, direction); @@ -181,20 +436,22 @@ class abidiff { void print_variant(const ojson& abi, int index, char direction) { std::cout << direction << " variant\n"; - std::cout << pretty_print(abi["variants"].at(index)) << "\n"; + std::cout << pretty_print(section_or_empty(abi, "variants").at(index)) << "\n"; } void find_variants(const ojson& abi1, const ojson& abi2, char direction) { - for ( int i=0; i < abi1["variants"].size(); i++ ) { + for ( int i=0; i < section_or_empty(abi1, "variants").size(); i++ ) { bool found = false; - for ( int j=0; j < abi2["variants"].size(); j++ ) { - if (abi1["variants"].at(i)["name"] == abi2["variants"].at(j)["name"]) { - for (int k=0; k < abi1["variants"].at(i)["types"].size(); k++) - if (abi1["variants"].at(i)["types"].at(k) != abi2["variants"].at(j)["types"].at(k)) - break; - found = true; - } - + for ( int j=0; j < section_or_empty(abi2, "variants").size(); j++ ) { + if (section_or_empty(abi1, "variants").at(i)["name"] != section_or_empty(abi2, "variants").at(j)["name"]) + continue; + // The whole type list, length and order. The original broke out of the element + // loop on a mismatch and then set found unconditionally, so a same-named variant + // counted as unchanged however its types differed; with no length check, at(k) + // also threw on a shorter right-hand side. + found = arrays_equal(field_or_null(section_or_empty(abi1, "variants").at(i), "types"), + field_or_null(section_or_empty(abi2, "variants").at(j), "types")); + break; } if (!found) print_variant(abi1, i, direction); @@ -203,18 +460,19 @@ class abidiff { void print_action_results(const ojson& abi, int index, char direction) { std::cout << direction << " action_result\n"; - std::cout << pretty_print(abi["action_results"].at(index)) << "\n"; + std::cout << pretty_print(section_or_empty(abi, "action_results").at(index)) << "\n"; } void find_action_results(const ojson& abi1, const ojson& abi2, char direction) { - for ( int i=0; i < abi1["action_results"].size(); i++ ) { + for ( int i=0; i < section_or_empty(abi1, "action_results").size(); i++ ) { bool found = false; - for ( int j=0; j < abi2["action_results"].size(); j++ ) { - if (abi1["action_results"].at(i)["name"] == abi2["action_results"].at(j)["name"]) { - if (abi1["action_results"].at(i)["result_type"] != abi2["action_results"].at(i)["result_type"]) - break; - found = true; - } + for ( int j=0; j < section_or_empty(abi2, "action_results").size(); j++ ) { + if (section_or_empty(abi1, "action_results").at(i)["name"] != section_or_empty(abi2, "action_results").at(j)["name"]) + continue; + if (field_or_null(section_or_empty(abi1, "action_results").at(i), "result_type") != field_or_null(section_or_empty(abi2, "action_results").at(j), "result_type")) + break; + found = true; + break; } if (!found) @@ -256,6 +514,130 @@ class abidiff { find_action_results(abi_2, abi_1, '>'); } + void print_enum(const ojson& abi, int index, char direction) { + std::cout << direction << " enum\n"; + std::cout << pretty_print(abi["enums"].at(index)) << "\n"; + } + + // enums and protobuf_types were compared by nothing at all, so a contract could change + // an enum's values or an entire protobuf descriptor and cdt-abidiff reported no + // difference. Both are optional sections -- absent in most ABIs -- so both sides are + // read through field_or_null. + void find_enums(const ojson& abi1, const ojson& abi2, char direction) { + const ojson& e1 = field_or_null(abi1, "enums"); + const ojson& e2 = field_or_null(abi2, "enums"); + for ( size_t i=0; i < e1.size(); i++ ) { + bool found = false; + for ( size_t j=0; j < e2.size(); j++ ) { + if (e1.at(i)["name"] != e2.at(j)["name"]) + continue; + found = field_or_null(e1.at(i), "type") == field_or_null(e2.at(j), "type") + && arrays_equal(field_or_null(e1.at(i), "values"), field_or_null(e2.at(j), "values")); + break; + } + if (!found) + print_enum(abi1, static_cast(i), direction); + } + } + + void diff_enums() { + find_enums(abi_1, abi_2, '<'); + find_enums(abi_2, abi_1, '>'); + } + + // protobuf_types is a whole serialized FileDescriptorSet, not a list keyed by name, so + // it is compared as one value rather than element-wise. A difference anywhere in it + // changes the wire encoding of every protobuf action. + // protobuf_types is a may_not_exist holding a FileDescriptorSet as JSON, and the + // chain's custom from_variant accepts either spelling: a JSON object, or a string + // containing that object's JSON. Absent and an empty string are likewise the same empty + // value. Comparing the raw node reported all of those as differences the runtime does + // not see, so each side is reduced to one logical value first. + // + // This is JSON-level canonicalisation, not protobuf-level: two spellings that decode to + // the same FileDescriptorSet but differ as JSON -- an enum written numerically in one + // and symbolically in the other, say -- are still reported as different. Collapsing + // those would mean parsing both through FileDescriptorSet with the chain's options, + // which this tool does not link protobuf to do. + // + // So it over-reports rather than under-reports, with ONE exception it cannot resolve + // here: jsoncons keeps only the last of a duplicate object member, which would hide a + // real difference. Such a document is refused at load (see load_checked) rather than + // compared -- in EITHER spelling, since the string form's own members are invisible to + // a walk of the outer document and are checked separately there. + static ojson canonical_protobuf(const ojson& o) { + if (!o.count(protobuf_types_key)) + return ojson::null(); + const ojson& v = o.at(protobuf_types_key); + if (!v.is_string()) + return v; // already an object (or something else; as-is) + const std::string text = v.as(); + if (text.empty()) + return ojson::null(); // empty string == absent, per to_variant + try { + ojson parsed = parse_strict(text); + // ONLY an object root. JsonStringToMessage requires a message, so a string + // holding "null" or "[1,2]" is content the chain rejects -- treating it as the + // value it decodes to would have equated it with absence, or with a raw array + // that the chain reads quite differently. Anything else stays the string it is, + // and therefore differs from both. + if (parsed.is_object()) + return parsed; + } catch (const std::exception&) { + // Not valid JSON, or not STRICT JSON. A string carrying a comment stays the string + // it is, and so differs from the object spelling -- which is what the chain sees, + // protobuf's JSON parser rejecting the comment and accepting the object. + } + return v; + } + + void diff_protobuf_types() { + const ojson p1 = canonical_protobuf(abi_1); + const ojson p2 = canonical_protobuf(abi_2); + if (p1 != p2) { + std::cout << "< protobuf_types\n" << pretty_print(p1) << "\n"; + std::cout << "> protobuf_types\n" << pretty_print(p2) << "\n"; + } + } + + // error_messages and abi_extensions are the remaining abi_def payload sections. CDT + // does not emit either, but an ABI produced elsewhere can carry them, and a change + // there is a real interface change. Compared whole, like protobuf_types. + // Read through section_or_empty, not field_or_null: an omitted optional section printed + // as `null` against an explicit `[]`, a difference the chain does not see. + void diff_opaque_section(const char* key) { + const ojson& a = section_or_empty(abi_1, key); + const ojson& b = section_or_empty(abi_2, key); + if (a != b) { + std::cout << "< " << key << "\n" << pretty_print(a) << "\n"; + std::cout << "> " << key << "\n" << pretty_print(b) << "\n"; + } + } + + // error_messages is consumed into a map keyed by error_code, so the same codes listed in + // a different order are the same ABI. Matched by key, like every other named section. + void find_error_messages(const ojson& abi1, const ojson& abi2, char direction) { + const ojson& e1 = section_or_empty(abi1, "error_messages"); + const ojson& e2 = section_or_empty(abi2, "error_messages"); + for ( size_t i = 0; i < e1.size(); i++ ) { + bool found = false; + for ( size_t j = 0; j < e2.size(); j++ ) { + if (field_or_null(e1.at(i), "error_code") != field_or_null(e2.at(j), "error_code")) + continue; + found = field_or_null(e1.at(i), "error_msg") == field_or_null(e2.at(j), "error_msg"); + break; + } + if (!found) { + std::cout << direction << " error_message\n" << pretty_print(e1.at(i)) << "\n"; + } + } + } + + void diff_error_messages() { + find_error_messages(abi_1, abi_2, '<'); + find_error_messages(abi_2, abi_1, '>'); + } + void diff() { diff_version(); diff_structs(); @@ -263,10 +645,19 @@ class abidiff { diff_actions(); diff_tables(); diff_clauses(); - if ( get_version(abi_1) >= 11 && get_version(abi_2) >= 11 ) - diff_variants(); - if ( get_version(abi_1) >= 12 && get_version(abi_2) >= 12 ) - diff_action_results(); + diff_enums(); + diff_protobuf_types(); + // Unconditional. These were version-gated, which suppressed real differences: two + // 1.0 documents whose same-named variant changed from ["uint64"] to ["string"] + // reported nothing, and populated action_results below 1.2 likewise. A version + // stamp says which sections a document is REQUIRED to carry, not which ones it may + // contain -- abigen emits `variants` at every version -- so the gate answered the + // wrong question. Both sections are read through section_or_empty, so an input that + // genuinely omits them compares as empty rather than throwing. + diff_variants(); + diff_action_results(); + diff_error_messages(); + diff_opaque_section("abi_extensions"); } }; diff --git a/tools/cc/cdt-cpp.cpp.in b/tools/cc/cdt-cpp.cpp.in index 178066790..59789228e 100644 --- a/tools/cc/cdt-cpp.cpp.in +++ b/tools/cc/cdt-cpp.cpp.in @@ -66,8 +66,9 @@ static bool write_finalize_manifest(const Options& opts, const std::string& outp << fm::k_protobuf_files << "=" << opts.protobuf_files << "\n"; if (!opts.abigen_output.empty() && opts.abigen_output != "''") ss << fm::k_abi_output << "=" << opts.abigen_output << "\n"; - if (opts.abi_version.first > 0) - ss << fm::k_abi_version << "=" << opts.abi_version.first << "." << opts.abi_version.second << "\n"; + // Always recorded: abi_version::parse rejects a zero major and the default is + // non-zero, so there is no "unset" state left for a >0 test to stand in for. + ss << fm::k_abi_version << "=" << opts.abi_version.first << "." << opts.abi_version.second << "\n"; // The descriptor(s) this object produced -- cdt-codegen names each ..desc in the // output dir. One line per source (normally a single source per compile). These ARE per-TU: // cdt-ld accumulates every object's desc_file (sources in different subdirectories are all @@ -150,10 +151,8 @@ int main(int argc, const char **argv) { codegen_args.push_back(opts.abigen_output); } - if (opts.abi_version.first > 0) { - codegen_args.push_back("--abi-version"); - codegen_args.push_back(std::to_string(opts.abi_version.first) + "." + std::to_string(opts.abi_version.second)); - } + codegen_args.push_back("--abi-version"); + codegen_args.push_back(std::to_string(opts.abi_version.first) + "." + std::to_string(opts.abi_version.second)); // Only suppress ABI generation when explicitly not linking (compile-only mode) // and abigen was not requested. When linking, always generate ABI. diff --git a/tools/codegen/cdt-codegen.cpp b/tools/codegen/cdt-codegen.cpp index 22512f68c..1299962ce 100644 --- a/tools/codegen/cdt-codegen.cpp +++ b/tools/codegen/cdt-codegen.cpp @@ -172,9 +172,9 @@ static std::string contract_name; static bool explicit_contract = false; static std::string output_dir = "."; -static std::string abi_version; -static int abi_version_major = 1; -static int abi_version_minor = 3; +static std::string abi_version_arg; +static int abi_version_major = abi_version::default_major; +static int abi_version_minor = abi_version::default_minor; static bool no_abigen = false; static std::string abi_output_path; // Link-time finalize split (see main()): @@ -234,7 +234,8 @@ static void print_usage(const char* prog) { << "\nOptions:\n" << " --contract NAME Contract name\n" << " --output-dir DIR Output directory (default: .)\n" - << " --abi-version VERSION ABI version (e.g. 1.3)\n" + << " --abi-version VERSION ABI version as [.] (default: " + << abi_version::default_spelling() << ")\n" << " --no-abigen Disable ABI generation\n" << " --cxx OPTIONS Additional C++ compiler options\n" << " -I, --include DIR C++ include directory (repeatable)\n" @@ -273,10 +274,13 @@ static void parse_args(int argc, const char** argv) { } else if (arg == "--output-dir" && i + 1 < argc) { output_dir = argv[++i]; } else if (arg == "--abi-version" && i + 1 < argc) { - abi_version = argv[++i]; - float tmp; - abi_version_major = std::stoi(abi_version); - abi_version_minor = (int)(std::modf(std::stof(abi_version), &tmp) * 10); + abi_version_arg = argv[++i]; + if (!abi_version::parse(abi_version_arg, abi_version_major, abi_version_minor)) { + std::cerr << "invalid --abi-version '" << abi_version_arg + << "': expected [.], e.g. " + << abi_version::default_spelling() << "\n"; + exit(1); + } } else if (arg == "--cxx" && i + 1 < argc) { cxx_arg = argv[++i]; } else if ((arg == "-I" || arg == "--include") && i + 1 < argc) { @@ -312,6 +316,19 @@ static void parse_args(int argc, const char** argv) { } } + // A contract with protobuf files ends up stamped at abi_version::protobuf_minor, so + // settle the effective version here -- before gen_actions hands it to the plugin and + // before the finalize pass stamps the merged ABI. The plugin gates its own sections on + // the version it is told, so promoting afterwards produced an ABI claiming 1.3 while + // missing the action_results that 1.2 already required, with the descriptors already + // written and the entries unrecoverable. Both passes run this, because cdt-ld forwards + // --protobuf-files and --abi-version into the finalize invocation. + if (protobuf_files.size() && abi_version_major == abi_version::default_major && + abi_version_minor < abi_version::protobuf_minor) { + abi_version_minor = abi_version::protobuf_minor; + abi_version_arg = abi_version::spelling(abi_version_major, abi_version_minor); + } + // The finalize pass does not compile anything (it only merges existing .desc files), // so it needs no --cxx options. if (!finalize_mode && cxx_arg.empty()) { @@ -400,12 +417,12 @@ static void gen_actions(const std::string& input) { if (abigen_opts.size()) abigen_opts += ","; - if (abi_version.size()) { - abigen_opts += "abi_version=" + abi_version; + if (abi_version_arg.size()) { + abigen_opts += "abi_version=" + abi_version_arg; } else if (no_abigen) { abigen_opts += "no_abigen"; } else { - abigen_opts += "abi_version=1.3"; + abigen_opts += "abi_version=" + abi_version::default_spelling(); } if (suppress_ricardian_warnings) { @@ -697,11 +714,25 @@ int main(int argc, const char** argv) { abi["protobuf_types"] = ojson::parse(protobuf_types_json); - // Bump ABI version to 1.3 when protobuf_types section is present - if (abi_version_major == 1 && abi_version_minor < 3) { - abi_version_minor = 3; - abi["version"] = "sysio::abi/1.3"; + // The promotion itself happened before gen_actions ran (see above), so the + // plugin already gated its sections on this version. All that is left is to + // stamp the merged document. + // + // Take the NEWER of the CLI version and the version the descriptors merged to. + // Stamping the CLI version unconditionally downgraded a document whose + // descriptors declared something newer -- reachable through the fallback scan + // that picks up .desc files from earlier compiles, which may have run with a + // different -abi-version. The previous assert() here was tautological (parse() + // bounds the major to exactly max_supported_major, so its second disjunct was + // unreachable) and compiled away under the default Release TOOLS_BUILD_TYPE. + int merged_major = 0; + int merged_minor = 0; + std::pair stamped{abi_version_major, abi_version_minor}; + if (abi.count("version") && + abi_version::parse_version_string(abi["version"].as(), merged_major, merged_minor)) { + stamped = std::max(stamped, std::pair{merged_major, merged_minor}); } + abi["version"] = abi_version::version_string(stamped.first, stamped.second); } else if (referenced_pb_types.size()) { std::cerr << "protobuf types are used but no protobuf files are specified for contract " << contract_name << ", please use `contract_use_protobuf()` cmake function to specify the protobuf files it depends on\n"; diff --git a/tools/include/compiler_options.hpp.in b/tools/include/compiler_options.hpp.in index ad66fd9af..2b35a7596 100644 --- a/tools/include/compiler_options.hpp.in +++ b/tools/include/compiler_options.hpp.in @@ -1018,13 +1018,16 @@ static Options CreateOptions(bool add_defaults=true) { #endif - int abi_version_major = 1; - int abi_version_minor = 2; + int abi_version_major = abi_version::default_major; + int abi_version_minor = abi_version::default_minor; if (!abi_version_opt.empty()) { - abi_version_major = std::stoi(abi_version_opt); - float tmp = std::stof(abi_version_opt); - abi_version_minor = ((tmp - (int)tmp)*10); + if (!abi_version::parse(abi_version_opt, abi_version_major, abi_version_minor)) { + std::cerr << "invalid -abi-version '" << abi_version_opt + << "': expected [.], e.g. " + << abi_version::default_spelling() << "\n"; + exit(1); + } } #ifndef ONLY_LD diff --git a/tools/include/sysio/abi.hpp b/tools/include/sysio/abi.hpp index 874146324..531893a5f 100644 --- a/tools/include/sysio/abi.hpp +++ b/tools/include/sysio/abi.hpp @@ -4,9 +4,169 @@ #include #include #include +#include #include #include +/** + * The ABI format version this toolchain emits. + * + * Single source of truth for every host tool and for the abigen plugin: cdt-cpp, + * cdt-cc, cdt-ld, cdt-codegen and `sysio_abigen` all take their default from here, + * so a standalone `cdt-codegen` run and an `add_contract()` build cannot stamp + * different versions into a contract's `.abi`. Bumping the format is a one-line + * change here plus a refresh of the `tests/toolchain/abigen-pass/.abi` + * fixtures, which pin the emitted string byte-for-byte. + */ +namespace abi_version { + inline constexpr int default_major = 1; + inline constexpr int default_minor = 2; + + /// "." -- the spelling accepted by the `-abi-version` driver flag + /// and by the abigen plugin's `abi_version=` plugin argument. + inline std::string spelling(int major_v, int minor_v) { + return std::to_string(major_v) + "." + std::to_string(minor_v); + } + + inline std::string default_spelling() { return spelling(default_major, default_minor); } + + /// The highest ABI major this toolchain can emit. `to_json` only knows how to + /// serialize the 1.x shape, so accepting a higher major would stamp a version we + /// cannot honour -- a 2.0 ABI would silently lose every section gated below. + inline constexpr int max_supported_major = 1; + + /// The minor from which the `protobuf_types` ABI section is understood. A + /// contract that emits one is bumped from the baseline to here; a contract that + /// does not stays at the baseline. + inline constexpr int protobuf_minor = 3; + + /// The version at which `variants` entered the format. A FIXED point in the format's + /// history, unrelated to max_supported_major, which is only the highest major this + /// toolchain accepts. Deriving one from the other made raising the accepted maximum move + /// every introduction with it -- supports_variants(1, 10) would become false while parse() + /// still accepted major 1, letting valid 1.x documents omit sections they require. + inline constexpr int variants_major = 1; + inline constexpr int variants_minor = 1; + + /// The version at which `action_results` entered the format. + inline constexpr int action_results_major = 1; + inline constexpr int action_results_minor = 2; + + /// Does a version carry the `variants` section? + /// + /// Provided for symmetry with `supports_action_results`; nothing calls it today, because + /// ABIMerger gates on the introduction pair directly and abigen emits `variants` + /// unconditionally. + inline constexpr bool supports_variants(int major_v, int minor_v) { + return major_v > variants_major || + (major_v == variants_major && minor_v >= variants_minor); + } + + /// Does a version carry the `action_results` section? + /// + /// Used by the abigen plugin to decide whether to emit the section. ABIMerger answers the + /// same question from the introduction constants above rather than through this predicate, + /// because it needs the (major, minor) pair itself to promote a merged document's version; + /// both therefore read the one rule declared here. Two spellings of that rule is how a + /// contract ends up with a version stamp promising a section its ABI does not carry. + /// + /// cdt-abidiff does NOT gate on it -- it diffs every section unconditionally, since a + /// section present in one document and absent from the other is exactly the difference it + /// exists to report, whatever version either side declares. + inline constexpr bool supports_action_results(int major_v, int minor_v) { + return major_v > action_results_major || + (major_v == action_results_major && minor_v >= action_results_minor); + } + + /// The full "sysio::abi/." string stamped into a contract's ABI. + inline std::string version_string(int major_v, int minor_v) { + return "sysio::abi/" + spelling(major_v, minor_v); + } + + /** + * Parse a "" or "." spelling. + * + * Integer parsing throughout: the previous float round-trip + * (`(int)((stof(v) - (int)stof(v)) * 10)`) truncated on any minor whose decimal + * expansion falls short in binary -- "1.3" parsed as minor 2 -- which silently + * desynced the version handed to the plugin from the one handed to ABIMerger. + * + * A zero major is rejected: there is no ABI 0.x. + * + * That rule once carried a second job, worth recording because it is why the driver can + * be as simple as it now is. cdt-cpp USED to read a zero major as "the option was never + * given", so accepting one would have let `cdt-cpp -abi-version 0.1` fall back to the + * default while `cdt-codegen --abi-version 0.1` honoured it -- reintroducing exactly the + * divergence this namespace exists to remove. That sentinel is retired: the driver now + * records and forwards the parsed version unconditionally + * (tools/cc/cdt-cpp.cpp.in), which it can do precisely BECAUSE zero never survives this + * parse. Keeping the rejection is what stops the sentinel being needed again. + * + * A major above max_supported_major is rejected too. to_json only knows the 1.x + * shape and gates action_results on major == 1, so a 2.0 or 10.2 request would + * otherwise be accepted, compared as "newer than 1.2" by the merger, and then + * emitted without the very sections the higher version implies. + * + * @param text the spelling to parse + * @param major_out set to the major component on success; untouched on failure + * @param minor_out set to the minor component on success (0 when omitted); + * untouched on failure + * @return true when @p text is a well-formed version, false otherwise. Callers + * are expected to emit a diagnostic and exit non-zero on false rather + * than proceeding with a partially-parsed version. + */ + inline bool parse(const std::string& text, int& major_out, int& minor_out) { + if (text.empty()) + return false; + + const auto dot = text.find('.'); + const std::string major_text = text.substr(0, dot); + const std::string minor_text = (dot == std::string::npos) ? std::string("0") + : text.substr(dot + 1); + + // Reject anything std::stoi would otherwise accept by prefix ("1x", " 1", "1.2.3"). + auto all_digits = [](const std::string& v) { + return !v.empty() && v.find_first_not_of("0123456789") == std::string::npos; + }; + if (!all_digits(major_text) || !all_digits(minor_text)) + return false; + + int major_v = 0; + int minor_v = 0; + try { + major_v = std::stoi(major_text); + minor_v = std::stoi(minor_text); + } catch (const std::exception&) { + return false; // out of int range + } + if (major_v == 0 || major_v > max_supported_major) + return false; + + major_out = major_v; + minor_out = minor_v; + return true; + } + + /** + * Parse the "::abi/." string stamped into a contract's ABI. + * + * The namespace prefix is not inspected, so a descriptor carrying an inherited + * `eosio::abi/1.2` parses the same as a `sysio::abi/1.2` one. Everything up to + * and including the last '/' is dropped and the remainder handed to parse(); + * a string with no '/' is parsed whole. + * + * @param text the version string to parse + * @param major_out set to the major component on success; untouched on failure + * @param minor_out set to the minor component on success; untouched on failure + * @return true when @p text carries a well-formed version, false otherwise + */ + inline bool parse_version_string(const std::string& text, int& major_out, int& minor_out) { + const auto slash = text.rfind('/'); + return parse(slash == std::string::npos ? text : text.substr(slash + 1), + major_out, minor_out); + } +} // namespace abi_version + struct abi_typedef { std::string new_type_name; std::string type; @@ -119,9 +279,9 @@ struct abi_action_result { /// From sysio libraries/chain/include/sysio/chain/abi_def.hpp struct abi { - int version_major = 1; - int version_minor = 1; - std::string version_string()const { return std::string("sysio::abi/")+std::to_string(version_major)+"."+std::to_string(version_minor); } + int version_major = abi_version::default_major; + int version_minor = abi_version::default_minor; + std::string version_string()const { return abi_version::version_string(version_major, version_minor); } std::set structs; std::set typedefs; std::set actions; diff --git a/tools/include/sysio/abimerge.hpp b/tools/include/sysio/abimerge.hpp index 1990b2a91..d586bd52e 100644 --- a/tools/include/sysio/abimerge.hpp +++ b/tools/include/sysio/abimerge.hpp @@ -6,7 +6,11 @@ #include #include "abi.hpp" +#include +#include +#include #include +#include #include using jsoncons::json; @@ -14,10 +18,12 @@ using jsoncons::ojson; class ABIMerger { public: - ABIMerger(ojson a) : abi(a) {} + /// version_major/version_minor seed an empty accumulator and are not retained: every + /// document that reaches version_of() declares its own version, and one that does not + /// is rejected rather than falling back to the merger's. ABIMerger(ojson a, int version_major, int version_minor) : abi(a) { if (abi.empty()) { - abi["version"] = std::string("sysio::abi/") + std::to_string(version_major) + "." + std::to_string(version_minor); + abi["version"] = abi_version::version_string(version_major, version_minor); abi["types"] = ojson::array(); abi["structs"] = ojson::array(); abi["actions"] = ojson::array(); @@ -41,17 +47,64 @@ class ABIMerger { ret["____comment"] = abi["____comment"]; else if (other.has_key("____comment")) ret["____comment"] = other["____comment"]; + // The emitted version is the newer of the two documents, so the capability + // gate below must consult THAT, not just the left-hand side. Gating on the + // left alone emitted e.g. 1.10 while dropping the action_results the newer + // side carried -- a version stamp promising a section the ABI lacks. + std::pair merged_version = std::max(version_of(abi), version_of(other)); + const std::pair declared_version = merged_version; + // Inserted HERE, before any other section, because ojson preserves insertion order + // and "version" is the first key of every ABI this toolchain has ever emitted. + // The value is corrected in place below once promotion is known; overwriting an + // existing key keeps its position, whereas assigning it late would move it to the + // end of the object and change the bytes of every contract's ABI. ret["version"] = merge_version(other); ret["types"] = merge_types(other); ret["structs"] = merge_structs(other); ret["actions"] = merge_actions(other); ret["tables"] = merge_tables(other); ret["ricardian_clauses"] = merge_clauses(other); - ret["variants"] = merge_variants(other); - std::string vers = abi["version"].as(); - if (std::stod(vers.substr(vers.size()-3))*10 >= 12) { - ret["action_results"] = merge_action_results(other); - } + + // A section belongs to the emitted document if it has content, and the emitted + // VERSION is then raised to one that admits it. + // + // Master emitted `variants` unconditionally, so a contract with a std::variant + // parameter built at -abi-version 1.0 got a document stamped 1.0 that nonetheless + // carried a section the format introduced at 1.1 -- self-inconsistent, though not + // lossy. Simply gating the section on the requested version would have made it + // lossy: the struct field stays typed `variant_uint64_string` while the array + // defining it disappears. Promoting the stamp keeps the document complete AND + // consistent, and is what the protobuf path already does when it moves to 1.3. + // Promote FIRST, from every gated section, then emit -- so the outcome does not + // depend on the order the sections are considered in. (Emitting as we go meant a + // populated action_results could raise the version to 1.2 after an empty variants + // had already been skipped, leaving a 1.2 document missing a section 1.2 requires.) + ojson variants_section = merge_variants(other); + ojson results_section = merge_action_results(other); + if (!variants_section.empty() && variants_since) + merged_version = std::max(merged_version, *variants_since); + if (!results_section.empty() && action_results_since) + merged_version = std::max(merged_version, *action_results_since); + + // A gated section is emitted when it has content, or when the version requires it to + // be present -- an empty array is the correct representation in that second case, + // and section() rejects a document that omits it. Below its version, absent. + const auto emit_section = [&](const char* key, ojson section, + const section_since& since) { + if (!section.empty() || (since && merged_version >= *since)) + ret[key] = std::move(section); + }; + emit_section("variants", std::move(variants_section), variants_since); + emit_section("action_results", std::move(results_section), action_results_since); + + // Rewritten in place (keeping its leading position) only if emit_section raised the + // version above what either input declared. When nothing forced a promotion the + // string merge_version already stamped at the top of merge() stands -- which is + // canonical, not inherited: version ORDERING ignores the namespace prefix so a + // foreign descriptor can be ingested, but the output always carries "sysio::abi/", + // since Wire's abi_serializer accepts nothing else. + if (merged_version != declared_version) + ret["version"] = abi_version::version_string(merged_version.first, merged_version.second); { ojson merged_enums = merge_enums(other); if (!merged_enums.empty()) @@ -60,27 +113,53 @@ class ABIMerger { return ret; } private: + /// The (major, minor) a document declares. + /// + /// Every document reaching here carries a version: the constructor seeds an empty + /// accumulator with one, merge() always stamps ret["version"], and abigen emits it in + /// every descriptor. So a missing key is a malformed external document rather than the + /// accumulator case, and defaulting it silently stamped the emission default onto input + /// the base implementation rejected. An unparsable version is malformed for the same + /// reason -- every capability gate below keys off this value. + std::pair version_of(const ojson& doc) const { + if (!doc.has_key("version")) + throw std::runtime_error("Error, ABI is missing its version"); + + const auto text = doc["version"].as(); + int major_v = 0; + int minor_v = 0; + if (!abi_version::parse_version_string(text, major_v, minor_v)) + throw std::runtime_error("Error, ABI declares an unsupported version : " + text); + return {major_v, minor_v}; + } + + /// The newer of the two versions, canonicalised to this toolchain's namespace. + /// + /// Returning the winning document's raw string emitted whatever prefix it carried: a + /// descriptor declaring "eosio::abi/1.10" produced a merged ABI stamped the same way, + /// which Wire's abi_serializer rejects outright -- it requires "sysio::abi/1.". Version + /// ORDERING ignores the prefix by design, so a foreign descriptor can still be ingested; + /// what it must not do is leave the output undeployable. std::string merge_version(ojson b) { - std::string ver_a = abi["version"].as(); - std::string ver_b = b["version"].as(); - return std::stod(ver_a.substr(ver_a.size()-3))*10 < std::stod(ver_b.substr(ver_b.size()-3))*10 ? - ver_b : ver_a; + const auto winner = std::max(version_of(abi), version_of(b)); + return abi_version::version_string(winner.first, winner.second); } + // Field order is significant: it is the serialization order, so {x,y} and {y,x} are + // different wire layouts. The previous form matched by set membership plus size, so two + // descriptors declaring the same struct with reordered fields merged as identical and + // whichever .desc sorted first silently won -- a determinism hazard keyed on filename. static bool struct_is_same(ojson a, ojson b) { - bool same_fields = a["fields"].size() == b["fields"].size(); - for (auto a_field : a["fields"].array_range()) { - bool found_field = false; - for (auto b_field : b["fields"].array_range()) { - if (a_field["name"] == b_field["name"] && - a_field["type"] == b_field["type"]) - found_field = true; - } - if (!found_field) + if (a["name"] != b["name"] || a["base"] != b["base"]) + return false; + const auto& fa = a["fields"]; + const auto& fb = b["fields"]; + if (fa.size() != fb.size()) + return false; + for (size_t i = 0; i < fa.size(); ++i) + if (fa[i]["name"] != fb[i]["name"] || fa[i]["type"] != fb[i]["type"]) return false; - } - return a["name"] == b["name"] && - a["base"] == b["base"] && same_fields; + return true; } static bool type_is_same(ojson a, ojson b) { @@ -94,37 +173,60 @@ class ABIMerger { a["ricardian_contract"] == b["ricardian_contract"]; } - template - static bool action_is_almost_same(ojson a, ojson b, T& rc) { - if (a["ricardian_contract"].empty()) - rc = b["ricardian_contract"]; - return a["name"] == b["name"] && - a["type"] == b["type"]; - } - + // Length and order, like struct_is_same and like cdt-abidiff's find_variants. The + // previous form asked only whether every type in `a` appeared somewhere in `b`, so + // ["uint64"] and ["uint64","string"] compared equal: merging them kept the accumulator's + // shorter list and dropped the `string` alternative outright, or -- with the descriptors + // in the other order -- failed the build with "v already defined". Which of the two you + // got was decided by sorted .desc filename order. static bool variant_is_same(ojson a, ojson b) { - for (auto tya : a["types"].array_range()) { - bool found_ty = false; - for (auto tyb : b["types"].array_range()) { - if (tyb == tya) - found_ty = true; - } - if (!found_ty) + if (a["name"] != b["name"]) + return false; + const auto& ta = a["types"]; + const auto& tb = b["types"]; + if (ta.size() != tb.size()) + return false; + for (size_t i = 0; i < ta.size(); ++i) + if (ta[i] != tb[i]) return false; - } - return a["name"] == b["name"]; + return true; } static bool table_is_same(ojson a, ojson b) { // key_names/key_types may differ: template-detected tables have them // populated while attribute-only tables have empty arrays. Both are // valid representations of the same table — treat as compatible. + // Optional-key tolerant: an ABI from another toolchain need not carry the Wire + // extensions (table_id, secondary_indexes) at all. + const auto field = [](const ojson& o, const char* k) { + static const ojson absent = ojson::null(); + return o.has_key(k) ? o[k] : absent; + }; + // "Unspecified" is either an ABSENT key or an empty array, and the two are not + // interchangeable in jsoncons: an absent key reads as null, and null.empty() is + // FALSE while array.empty() is true, so testing empty() alone never fired for a + // missing key. abigen writes secondary_indexes only when non-empty, so a + // translation unit that sees a table's [[sysio::table]] but not its indexed + // instantiation omits the key entirely -- and that TU's descriptor then failed to + // merge with the one that has it, breaking multi-file contracts that master builds. + const auto unspecified = [](const ojson& v) { return v.is_null() || v.empty(); }; + const auto compatible = [&](const char* k) { + const ojson x = field(a, k); + const ojson y = field(b, k); + return x == y || unspecified(x) || unspecified(y); + }; return a["name"] == b["name"] && a["type"] == b["type"] && - a["index_type"] == b["index_type"] && - (a["key_names"] == b["key_names"] || - a["key_names"].empty() || b["key_names"].empty()); + field(a, "index_type") == field(b, "index_type") && + // table_id is where the row physically lives and each secondary index carries + // its own, so a difference in either is a different table -- not a merge. + // These were omitted while cdt-abidiff's tables_match compared them, leaving + // the differ and the merger disagreeing on table identity. + field(a, "table_id") == field(b, "table_id") && + compatible("key_names") && + compatible("key_types") && + compatible("secondary_indexes"); } static bool clause_is_same(ojson a, ojson b) { @@ -143,22 +245,64 @@ class ABIMerger { a["values"] == b["values"]; } + /// The version at which a section entered the format, or nullopt for one that is never + /// mandatory. + /// + /// Absence is only legitimate below that version: abigen emits `variants` in every + /// document and `action_results` in every document whose version supports it, so a 1.10 + /// descriptor missing either is truncated, not merely old. Treating them as optional at + /// every version -- as an earlier revision did -- silently dropped contract interface + /// content. `enums` is emitted only when non-empty, so it is genuinely optional + /// everywhere and carries no threshold. + using section_since = std::optional>; + + static constexpr std::pair baseline_section{0, 0}; + static const section_since variants_since; + static const section_since action_results_since; + static const section_since never_mandatory; + + static const ojson& section(const ojson& doc, const std::string& type, + const section_since& since, std::pair doc_version) { + static const ojson empty = ojson::array(); + if (doc.has_key(type)) + return doc[type]; + if (!since || doc_version < *since) + return empty; + throw std::runtime_error("Error, ABI at " + + abi_version::version_string(doc_version.first, doc_version.second) + + " is missing section : " + type); + } + template - void add_object_to_array(ojson& ret, ojson a, ojson b, std::string type, std::string id, F&& is_same_func) { - for (auto obj_a : a[type].array_range()) { + void add_object_to_array(ojson& ret, ojson a, ojson b, std::string type, std::string id, + F&& is_same_func, + const section_since& since = section_since{baseline_section}) { + for (auto obj_a : section(a, type, since, version_of(a)).array_range()) { ret.push_back(obj_a); } - for (auto obj_b : b[type].array_range()) { + for (auto obj_b : section(b, type, since, version_of(b)).array_range()) { bool should_skip = false; for (size_t i = 0; i < ret.size(); ++i) { if (ret[i][id] == obj_b[id]) { if (!is_same_func(ret[i], obj_b)) { throw std::runtime_error(std::string("Error, ABI structs malformed : ")+ret[i][id].as()+" already defined"); } - // Prefer the entry with richer key metadata (non-empty key_names) - if (ret[i].count("key_names") && obj_b.count("key_names") && - ret[i]["key_names"].empty() && !obj_b["key_names"].empty()) { - ret[i] = obj_b; + // Take the richer value for EACH optional list independently, rather than + // replacing the whole entry. Two earlier forms were both order-dependent: + // checking only key_names dropped a secondary_indexes the other side + // carried, and replacing wholesale on any of the three discarded whichever + // list the accumulator was richer in -- so two descriptors each rich in a + // different key produced a different result depending on which .desc + // sorted first. Per-key, the union is the same either way. + // + // is_same_func has already established these describe the same entity, so + // there is no conflict to resolve here: a populated list only ever fills + // in for an absent or empty one. + for (const char* k : {"key_names", "key_types", "secondary_indexes"}) { + const bool have_a = ret[i].count(k) && !ret[i][k].empty(); + const bool have_b = obj_b.count(k) && !obj_b[k].empty(); + if (!have_a && have_b) + ret[i][k] = obj_b[k]; } should_skip = true; } @@ -204,7 +348,7 @@ class ABIMerger { ojson merge_variants(ojson b) { ojson vars = ojson::array(); - add_object_to_array(vars, abi, b, "variants", "name", variant_is_same); + add_object_to_array(vars, abi, b, "variants", "name", variant_is_same, variants_since); return vars; } @@ -228,7 +372,7 @@ class ABIMerger { ojson merge_action_results(ojson b) { ojson res = ojson::array(); - add_object_to_array(res, abi, b, "action_results", "name", action_result_is_same); + add_object_to_array(res, abi, b, "action_results", "name", action_result_is_same, action_results_since); return res; } @@ -237,11 +381,22 @@ class ABIMerger { if (abi.has_key("enums") || b.has_key("enums")) { if (!abi.has_key("enums")) abi["enums"] = ojson::array(); if (!b.has_key("enums")) b["enums"] = ojson::array(); - add_object_to_array(enums, abi, b, "enums", "name", enum_is_same); + add_object_to_array(enums, abi, b, "enums", "name", enum_is_same, never_mandatory); } return enums; } ojson abi; }; + +// The fixed versions at which each section entered the format -- not default_major (what this +// toolchain emits by default) and not max_supported_major (the highest major it accepts). +// Deriving them from either makes a change to that unrelated knob silently move every +// threshold. +inline const ABIMerger::section_since ABIMerger::variants_since{ + std::pair{abi_version::variants_major, abi_version::variants_minor}}; +inline const ABIMerger::section_since ABIMerger::action_results_since{ + std::pair{abi_version::action_results_major, abi_version::action_results_minor}}; +inline const ABIMerger::section_since ABIMerger::never_mandatory{}; + #pragma GCC diagnostic pop