From 9ae01e40b55e7509adadbfeb40b8309060128609 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 18 Feb 2021 11:08:49 +1100 Subject: [PATCH] ci: Add a test app for not placing embedded file paths into binaries Doubles as a test app that building with assertions off doesn't produce warnings. Closes https://github.com/espressif/esp-idf/issues/6306 --- tools/ci/executable-list.txt | 1 + .../system/no_embedded_paths/CMakeLists.txt | 20 ++++ .../system/no_embedded_paths/README.md | 14 +++ .../no_embedded_paths/check_for_file_paths.py | 94 +++++++++++++++++++ .../no_embedded_paths/main/CMakeLists.txt | 2 + .../main/test_no_embedded_paths_main.c | 4 + .../no_embedded_paths/sdkconfig.ci.noasserts | 8 ++ .../sdkconfig.ci.noasserts.nimble | 13 +++ .../sdkconfig.ci.silentasserts | 7 ++ .../sdkconfig.ci.silentasserts.nimble | 13 +++ 10 files changed, 176 insertions(+) create mode 100644 tools/test_apps/system/no_embedded_paths/CMakeLists.txt create mode 100644 tools/test_apps/system/no_embedded_paths/README.md create mode 100755 tools/test_apps/system/no_embedded_paths/check_for_file_paths.py create mode 100644 tools/test_apps/system/no_embedded_paths/main/CMakeLists.txt create mode 100644 tools/test_apps/system/no_embedded_paths/main/test_no_embedded_paths_main.c create mode 100644 tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts create mode 100644 tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts.nimble create mode 100644 tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts create mode 100644 tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts.nimble diff --git a/tools/ci/executable-list.txt b/tools/ci/executable-list.txt index ba39889e81e..6a6ec198a95 100644 --- a/tools/ci/executable-list.txt +++ b/tools/ci/executable-list.txt @@ -99,6 +99,7 @@ tools/mass_mfg/mfg_gen.py tools/mkdfu.py tools/mkuf2.py tools/set-submodules-to-github.sh +tools/test_apps/system/no_embedded_paths/check_for_file_paths.py tools/test_idf_monitor/run_test_idf_monitor.py tools/test_idf_py/test_idf_py.py tools/test_idf_size/test.sh diff --git a/tools/test_apps/system/no_embedded_paths/CMakeLists.txt b/tools/test_apps/system/no_embedded_paths/CMakeLists.txt new file mode 100644 index 00000000000..e6fdc3cb6be --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/CMakeLists.txt @@ -0,0 +1,20 @@ +# The following lines of boilerplate have to be in your project's +# CMakeLists in this exact order for cmake to work correctly +cmake_minimum_required(VERSION 3.5) + +include($ENV{IDF_PATH}/tools/cmake/project.cmake) +project(no_embedded_paths) + +idf_build_get_property(idf_path IDF_PATH) +idf_build_get_property(python PYTHON) +idf_build_get_property(elf EXECUTABLE) + +# If the configuration is one that doesn't expect any paths to be found then run this build step +# after building the ELF, will fail if it finds any file paths in binary files +if(CONFIG_OPTIMIZATION_ASSERTIONS_SILENT OR CONFIG_OPTIMIZATION_ASSERTIONS_DISABLED) + add_custom_command( + TARGET ${elf} + POST_BUILD + COMMAND ${python} "${CMAKE_CURRENT_LIST_DIR}/check_for_file_paths.py" "${idf_path}" "${CMAKE_BINARY_DIR}" + ) +endif() diff --git a/tools/test_apps/system/no_embedded_paths/README.md b/tools/test_apps/system/no_embedded_paths/README.md new file mode 100644 index 00000000000..32a9312a32d --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/README.md @@ -0,0 +1,14 @@ +# No Embedded Paths + +This test app exists to verify that paths (like __FILE__) are not compiled into +any object files in configurations where this should be avoided. + +It doubles up as a build-time check that disabling assertions doesn't lead to +any warnings. + +(These configurations include: assertions disabled, 'silent' asserts, any reproducible +builds configuration.) + +Not embedding paths reduces the binary size, avoids leaking information about +the compilation environment, and is a necessary step to supporet reproducible +builds across projects built in different directories. diff --git a/tools/test_apps/system/no_embedded_paths/check_for_file_paths.py b/tools/test_apps/system/no_embedded_paths/check_for_file_paths.py new file mode 100755 index 00000000000..ca0bd21b9c3 --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/check_for_file_paths.py @@ -0,0 +1,94 @@ +#!/usr/bin/env python +# +# 'check_for_file_paths.py' is a CI tool that checks all the unlinked object files +# in a CMake build directory for embedded copies of IDF_PATH. +# +# Designed to be run in CI as a check that __FILE__ macros haven't snuck into any source code. +# +# Checking the unlinked object files means we don't rely on anything being actually linked into the binary, +# just anything which could potentially be linked. +# +# Usage: +# ./check_for_file_paths.py +# +# +# +# Copyright 2019 Espressif Systems (Shanghai) PTE LTD +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +import os +import re +import sys + +from elftools.elf.elffile import ELFFile + +# If an ESP-IDF source file has no option but to include __FILE__ macros, name it here (as re expression). +# +# IMPORTANT: This should only be used for upstream code where there is no other +# option. ESP-IDF code should avoid embedding file names as much as possible to +# limit the binary size and support reproducible builds +# +# Note: once ESP-IDF moves to Python >=3.6 then this can be simplified to use 'glob' and '**' +EXCEPTIONS = [ + r'openssl/.+/ssl_pm.c.obj$', # openssl API requires __FILE__ in error reporting functions, as per upstream API + r'openssl/.+/ssl_bio.c.obj$', + r'unity/.+/unity_runner.c.obj$', # unity is not for production use, has __FILE__ for test information +] + + +def main(): # type: () -> None + idf_path = sys.argv[1] + build_dir = sys.argv[2] + + assert os.path.exists(idf_path) + assert os.path.exists(build_dir) + + print('Checking object files in {} for mentions of {}...'.format(build_dir, idf_path)) + + # note: once ESP-IDF moves to Python >=3.6 then this can be simplified to use 'glob' and f'{build_dir}**/*.obj' + files = [] + for (dirpath, _, filepaths) in os.walk(build_dir): + files += [os.path.join(dirpath, filepath) for filepath in filepaths if filepath.endswith('.obj')] + + print('Found {} object files...'.format(len(files))) + + idf_path_binary = idf_path.encode() # we're going to be checking binary streams (note: probably non-ascii IDF_PATH will not match OK) + + failures = 0 + for obj_file in files: + if not any(re.search(exception, obj_file) for exception in EXCEPTIONS): + failures += check_file(obj_file, idf_path_binary) + if failures > 0: + raise SystemExit('{} source files are embedding file paths, see list above.'.format(failures)) + print('No embedded file paths found') + + +def check_file(obj_file, idf_path): # type: (str, bytes) -> int + failures = 0 + with open(obj_file, 'rb') as f: + elf = ELFFile(f) + for sec in elf.iter_sections(): + # can't find a better way to filter out only sections likely to contain strings, + # and exclude debug sections. .dram matches DRAM_STR, which links to .dram1 + if '.rodata' in sec.name or '.dram' in sec.name: + contents = sec.data() + if idf_path in contents: + print('error: {} contains an unwanted __FILE__ macro'.format(obj_file)) + failures += 1 + break + return failures + + +if __name__ == '__main__': + main() diff --git a/tools/test_apps/system/no_embedded_paths/main/CMakeLists.txt b/tools/test_apps/system/no_embedded_paths/main/CMakeLists.txt new file mode 100644 index 00000000000..5121b33045c --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/main/CMakeLists.txt @@ -0,0 +1,2 @@ +idf_component_register(SRCS "test_no_embedded_paths_main.c" + INCLUDE_DIRS ".") diff --git a/tools/test_apps/system/no_embedded_paths/main/test_no_embedded_paths_main.c b/tools/test_apps/system/no_embedded_paths/main/test_no_embedded_paths_main.c new file mode 100644 index 00000000000..c6d63408832 --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/main/test_no_embedded_paths_main.c @@ -0,0 +1,4 @@ +/* This test app only exists for the build stage, so doesn't need to do anything at runtime */ +void app_main(void) +{ +} diff --git a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts new file mode 100644 index 00000000000..446a04da0b0 --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts @@ -0,0 +1,8 @@ +CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE=y +CONFIG_FREERTOS_ASSERT_DISABLE=y + +# compiling as many files as possible here (we don't have 100% coverage of course, due to config options, but +# try to maximize what we can check +CONFIG_BT_ENABLED=y +CONFIG_BT_BLUEDROID_ENABLED=y +CONFIG_BLE_MESH=y diff --git a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts.nimble b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts.nimble new file mode 100644 index 00000000000..b91ad2022de --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.noasserts.nimble @@ -0,0 +1,13 @@ +CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE=y +CONFIG_FREERTOS_ASSERT_DISABLE=y + +# the other sdkconfig builds Bluedroid, build NimBLE here +# +# (Note: ESP32-S2 will build both these configs as well, but they're identical. This is simpler than +# needing to specify per-target configs for both Bluedroid and Nimble on ESP32, ESP32-C3.) +CONFIG_BT_ENABLED=y +CONFIG_BT_NIMBLE_ENABLED=y +CONFIG_BT_NIMBLE_CRYPTO_STACK_MBEDTLS=n +CONFIG_BT_NIMBLE_MESH=y +CONFIG_BLE_MESH=y +CONFIG_BT_NIMBLE_MAX_CONNECTIONS=1 diff --git a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts new file mode 100644 index 00000000000..9b54479ee97 --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts @@ -0,0 +1,7 @@ +CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT=y + +# compiling as many files as possible here (we don't have 100% coverage of course, due to config options, but +# try to maximize what we can check +CONFIG_BT_ENABLED=y +CONFIG_BT_BLUEDROID_ENABLED=y +CONFIG_BLE_MESH=y diff --git a/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts.nimble b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts.nimble new file mode 100644 index 00000000000..ab0cd5aa6d6 --- /dev/null +++ b/tools/test_apps/system/no_embedded_paths/sdkconfig.ci.silentasserts.nimble @@ -0,0 +1,13 @@ +CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT=y +CONFIG_FREERTOS_ASSERT_DISABLE=y + +# the other sdkconfig builds Bluedroid, build NimBLE here +# +# (Note: ESP32-S2 will build both these configs as well, but they're identical. This is simpler than +# needing to specify per-target configs for both Bluedroid and Nimble on ESP32, ESP32-C3.) +CONFIG_BT_ENABLED=y +CONFIG_BT_NIMBLE_ENABLED=y +CONFIG_BT_NIMBLE_CRYPTO_STACK_MBEDTLS=n +CONFIG_BT_NIMBLE_MESH=y +CONFIG_BLE_MESH=y +CONFIG_BT_NIMBLE_MAX_CONNECTIONS=1