From 1b6295b2bf38c0218ebe3c1168db18db63d90f70 Mon Sep 17 00:00:00 2001 From: Sergey Gavrilov Date: Mon, 23 Oct 2023 12:49:16 +0300 Subject: [PATCH 1/2] [FL-3632] FastFAP: human readable error log (#3156) --- lib/flipper_application/elf/elf_file.c | 55 +++++++++++++++++++++----- 1 file changed, 45 insertions(+), 10 deletions(-) diff --git a/lib/flipper_application/elf/elf_file.c b/lib/flipper_application/elf/elf_file.c index 9b8b4c8f5..b2c9445ff 100644 --- a/lib/flipper_application/elf/elf_file.c +++ b/lib/flipper_application/elf/elf_file.c @@ -613,10 +613,31 @@ static Elf32_Addr elf_address_of_by_hash(ELFFile* elf, uint32_t hash) { return ELF_INVALID_ADDRESS; } +static bool elf_file_find_string_by_hash(ELFFile* elf, uint32_t hash, FuriString* out) { + bool result = false; + + FuriString* symbol_name = furi_string_alloc(); + Elf32_Sym sym; + for(size_t i = 0; i < elf->symbol_count; i++) { + furi_string_reset(symbol_name); + if(elf_read_symbol(elf, i, &sym, symbol_name)) { + if(elf_symbolname_hash(furi_string_get_cstr(symbol_name)) == hash) { + furi_string_set(out, symbol_name); + result = true; + break; + } + } + } + furi_string_free(symbol_name); + + return result; +} + static bool elf_relocate_fast(ELFFile* elf, ELFSection* s) { UNUSED(elf); const uint8_t* start = s->fast_rel->data; const uint8_t version = *start; + bool no_errors = true; if(version != FAST_RELOCATION_VERSION) { FURI_LOG_E(TAG, "Unsupported fast relocation version %d", version); @@ -664,16 +685,30 @@ static bool elf_relocate_fast(ELFFile* elf, ELFSection* s) { } if(address == ELF_INVALID_ADDRESS) { - FURI_LOG_E(TAG, "Failed to resolve address for hash %lX", hash_or_section_index); - return false; - } + FuriString* symbol_name = furi_string_alloc(); + if(elf_file_find_string_by_hash(elf, hash_or_section_index, symbol_name)) { + FURI_LOG_E( + TAG, + "Failed to resolve address for symbol %s (hash %lX)", + furi_string_get_cstr(symbol_name), + hash_or_section_index); + } else { + FURI_LOG_E( + TAG, + "Failed to resolve address for hash %lX (string not found)", + hash_or_section_index); + } + furi_string_free(symbol_name); - for(uint32_t j = 0; j < offsets_count; j++) { - uint32_t offset = *((uint32_t*)start) & 0x00FFFFFF; - start += 3; - // FURI_LOG_I(TAG, " Fast relocation offset %ld: %ld", j, offset); - Elf32_Addr relAddr = ((Elf32_Addr)s->data) + offset; - elf_relocate_symbol(elf, relAddr, type, address); + no_errors = false; + start += 3 * offsets_count; + } else { + for(uint32_t j = 0; j < offsets_count; j++) { + uint32_t offset = *((uint32_t*)start) & 0x00FFFFFF; + start += 3; + Elf32_Addr relAddr = ((Elf32_Addr)s->data) + offset; + elf_relocate_symbol(elf, relAddr, type, address); + } } } @@ -681,7 +716,7 @@ static bool elf_relocate_fast(ELFFile* elf, ELFSection* s) { free(s->fast_rel); s->fast_rel = NULL; - return true; + return no_errors; } static bool elf_relocate_section(ELFFile* elf, ELFSection* section) { From 35c903494c511f41a518dd045be5ab8991fa3991 Mon Sep 17 00:00:00 2001 From: hedger Date: Mon, 23 Oct 2023 13:55:36 +0400 Subject: [PATCH 2/2] [FL-3627, FL-3628, FL-3631] fbt: glob & git improvements (#3151) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fbt: optional shallow submodule checkout * fbt: more git threads by default * fbt: git condition fix * fbt: renamed FBT_SHALLOW to FBT_GIT_SUBMODULE_SHALLOW * github: enabled FBT_GIT_SUBMODULE_SHALLOW in flows * fbt: always compile icons' .c, even if user does not specify a proper source glob; changed glob to require files at user-specified paths to exist * fbt: fail build for missing imports in .faps * fbt: moved STRICT_FAP_IMPORT_CHECK to commandline options; enabled by default * ufbt: enabled STRICT_FAP_IMPORT_CHECK Co-authored-by: あく --- .github/workflows/build.yml | 1 + .github/workflows/build_compact.yml | 1 + .github/workflows/pvs_studio.yml | 1 + .github/workflows/unit_tests.yml | 1 + .github/workflows/updater_test.yml | 1 + fbt | 11 +++++++++-- fbt.cmd | 12 ++++++++++-- scripts/fbt/appmanifest.py | 4 ++++ scripts/fbt_tools/fbt_extapps.py | 15 +++++++++++++-- scripts/fbt_tools/sconsrecursiveglob.py | 7 +++---- scripts/ufbt/commandline.scons | 5 +++++ site_scons/commandline.scons | 5 +++++ 12 files changed, 54 insertions(+), 10 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7a7188837..4c5535066 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -11,6 +11,7 @@ on: env: DEFAULT_TARGET: f7 FBT_TOOLCHAIN_PATH: /runner/_work + FBT_GIT_SUBMODULE_SHALLOW: 1 jobs: main: diff --git a/.github/workflows/build_compact.yml b/.github/workflows/build_compact.yml index 7556c15ac..c63be24a4 100644 --- a/.github/workflows/build_compact.yml +++ b/.github/workflows/build_compact.yml @@ -5,6 +5,7 @@ on: env: FBT_TOOLCHAIN_PATH: /runner/_work + FBT_GIT_SUBMODULE_SHALLOW: 1 jobs: compact: diff --git a/.github/workflows/pvs_studio.yml b/.github/workflows/pvs_studio.yml index 24964a309..4f650f1b7 100644 --- a/.github/workflows/pvs_studio.yml +++ b/.github/workflows/pvs_studio.yml @@ -10,6 +10,7 @@ env: TARGETS: f7 DEFAULT_TARGET: f7 FBT_TOOLCHAIN_PATH: /runner/_work + FBT_GIT_SUBMODULE_SHALLOW: 1 jobs: analyse_c_cpp: diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 794dce37f..35085ba57 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -7,6 +7,7 @@ env: TARGETS: f7 DEFAULT_TARGET: f7 FBT_TOOLCHAIN_PATH: /opt + FBT_GIT_SUBMODULE_SHALLOW: 1 jobs: run_units_on_bench: diff --git a/.github/workflows/updater_test.yml b/.github/workflows/updater_test.yml index f4064e1cc..9987fdd32 100644 --- a/.github/workflows/updater_test.yml +++ b/.github/workflows/updater_test.yml @@ -7,6 +7,7 @@ env: TARGETS: f7 DEFAULT_TARGET: f7 FBT_TOOLCHAIN_PATH: /opt + FBT_GIT_SUBMODULE_SHALLOW: 1 jobs: test_updater_on_bench: diff --git a/fbt b/fbt index 26f325d45..e6133d07b 100755 --- a/fbt +++ b/fbt @@ -5,7 +5,8 @@ set -eu; # private variables -N_GIT_THREADS="$(getconf _NPROCESSORS_ONLN)"; +N_CORES="$(getconf _NPROCESSORS_ONLN)"; +N_GIT_THREADS="$(($N_CORES * 2))"; SCRIPT_PATH="$(cd "$(dirname "$0")" && pwd -P)"; SCONS_DEFAULT_FLAGS="--warn=target-not-built"; SCONS_EP="python3 -m SCons"; @@ -15,6 +16,7 @@ FBT_NOENV="${FBT_NOENV:-""}"; FBT_NO_SYNC="${FBT_NO_SYNC:-""}"; FBT_TOOLCHAIN_PATH="${FBT_TOOLCHAIN_PATH:-$SCRIPT_PATH}"; FBT_VERBOSE="${FBT_VERBOSE:-""}"; +FBT_GIT_SUBMODULE_SHALLOW="${FBT_GIT_SUBMODULE_SHALLOW:-""}"; if [ -z "$FBT_NOENV" ]; then FBT_VERBOSE="$FBT_VERBOSE" . "$SCRIPT_PATH/scripts/toolchain/fbtenv.sh"; @@ -29,7 +31,12 @@ if [ -z "$FBT_NO_SYNC" ]; then echo "\".git\" directory not found, please clone repo via \"git clone\""; exit 1; fi - git submodule update --init --jobs "$N_GIT_THREADS"; + _FBT_CLONE_FLAGS="--jobs $N_GIT_THREADS"; + if [ ! -z "$FBT_GIT_SUBMODULE_SHALLOW" ]; then + _FBT_CLONE_FLAGS="$_FBT_CLONE_FLAGS --depth 1"; + fi + + git submodule update --init --recursive $_FBT_CLONE_FLAGS; fi $SCONS_EP $SCONS_DEFAULT_FLAGS "$@" diff --git a/fbt.cmd b/fbt.cmd index 03e4ec3d0..20432be1c 100644 --- a/fbt.cmd +++ b/fbt.cmd @@ -4,10 +4,18 @@ call "%~dp0scripts\toolchain\fbtenv.cmd" env set SCONS_EP=python -m SCons if [%FBT_NO_SYNC%] == [] ( + set _FBT_CLONE_FLAGS=--jobs %NUMBER_OF_PROCESSORS% + if not [%FBT_GIT_SUBMODULE_SHALLOW%] == [] ( + set _FBT_CLONE_FLAGS=%_FBT_CLONE_FLAGS% --depth 1 + ) if exist ".git" ( - git submodule update --init --depth 1 --jobs %NUMBER_OF_PROCESSORS% + git submodule update --init --recursive %_FBT_CLONE_FLAGS% + if %ERRORLEVEL% neq 0 ( + echo Failed to update submodules, set FBT_NO_SYNC to skip + exit /b 1 + ) ) else ( - echo Not in a git repo, please clone with "git clone" + echo .git not found, please clone repo with "git clone" exit /b 1 ) ) diff --git a/scripts/fbt/appmanifest.py b/scripts/fbt/appmanifest.py index ff49707b1..1a6cae9b1 100644 --- a/scripts/fbt/appmanifest.py +++ b/scripts/fbt/appmanifest.py @@ -100,6 +100,10 @@ class FlipperApplication: def is_default_deployable(self): return self.apptype != FlipperAppType.DEBUG and self.fap_category != "Examples" + @property + def do_strict_import_checks(self): + return self.apptype != FlipperAppType.PLUGIN + def __post_init__(self): if self.apptype == FlipperAppType.PLUGIN: self.stack_size = 0 diff --git a/scripts/fbt_tools/fbt_extapps.py b/scripts/fbt_tools/fbt_extapps.py index bc8f9d5df..963429f24 100644 --- a/scripts/fbt_tools/fbt_extapps.py +++ b/scripts/fbt_tools/fbt_extapps.py @@ -42,6 +42,7 @@ class AppBuilder: self.ext_apps_work_dir = env["EXT_APPS_WORK_DIR"] self.app_work_dir = self.get_app_work_dir(env, app) self.app_alias = f"fap_{self.app.appid}" + self.icons_src = None self.externally_built_files = [] self.private_libs = [] @@ -93,6 +94,7 @@ class AppBuilder: ) self.app_env.Alias("_fap_icons", fap_icons) self.fw_env.Append(_APP_ICONS=[fap_icons]) + self.icons_src = next(filter(lambda n: n.path.endswith(".c"), fap_icons)) def _build_private_libs(self): for lib_def in self.app.fap_private_libs: @@ -160,6 +162,10 @@ class AppBuilder: if not app_sources: raise UserError(f"No source files found for {self.app.appid}") + # Ensure that icons are included in the build, regardless of user-configured sources + if self.icons_src and not self.icons_src in app_sources: + app_sources.append(self.icons_src) + ## Uncomment for debug # print(f"App sources for {self.app.appid}: {list(f.path for f in app_sources)}") @@ -180,7 +186,9 @@ class AppBuilder: self.app._assets_dirs.append(self.app_work_dir.Dir("assets")) app_artifacts.validator = self.app_env.ValidateAppImports( - app_artifacts.compact + app_artifacts.compact, + _CHECK_APP=self.app.do_strict_import_checks + and self.app_env.get("STRICT_FAP_IMPORT_CHECK"), )[0] if self.app.apptype == FlipperAppType.PLUGIN: @@ -300,7 +308,10 @@ def validate_app_imports(target, source, env): + fg.brightmagenta(f"{disabled_api_syms}") + fg.brightyellow(")") ) - SCons.Warnings.warn(SCons.Warnings.LinkWarning, warning_msg), + if env.get("_CHECK_APP"): + raise UserError(warning_msg) + else: + SCons.Warnings.warn(SCons.Warnings.LinkWarning, warning_msg), def GetExtAppByIdOrPath(env, app_dir): diff --git a/scripts/fbt_tools/sconsrecursiveglob.py b/scripts/fbt_tools/sconsrecursiveglob.py index 38aa9a839..e7eb8fb72 100644 --- a/scripts/fbt_tools/sconsrecursiveglob.py +++ b/scripts/fbt_tools/sconsrecursiveglob.py @@ -20,10 +20,9 @@ def GlobRecursive(env, pattern, node=".", exclude=[]): source=True, exclude=exclude, ) - # Otherwise, just check if that's an existing file path - # NB: still creates "virtual" nodes as part of existence check - elif (file_node := node.File(pattern)).exists() or file_node.rexists(): - results.append(file_node) + # Otherwise, just assume that file at path exists + else: + results.append(node.File(pattern)) # print(f"Glob result for {pattern} from {node}: {results}") return results diff --git a/scripts/ufbt/commandline.scons b/scripts/ufbt/commandline.scons index 99c34c35d..a3ba7e08d 100644 --- a/scripts/ufbt/commandline.scons +++ b/scripts/ufbt/commandline.scons @@ -88,6 +88,11 @@ vars.AddVariables( "CDC Port of Flipper to use, if multiple are connected", "auto", ), + BoolVariable( + "STRICT_FAP_IMPORT_CHECK", + help="Enable strict import check for .faps", + default=True, + ), ) Return("vars") diff --git a/site_scons/commandline.scons b/site_scons/commandline.scons index f75d5e0e4..2d2abd5c6 100644 --- a/site_scons/commandline.scons +++ b/site_scons/commandline.scons @@ -269,6 +269,11 @@ vars.AddVariables( "clangd", ], ), + BoolVariable( + "STRICT_FAP_IMPORT_CHECK", + help="Enable strict import check for .faps", + default=True, + ), ) Return("vars")