From 0d0d28e5e5bf89026b6659db02a3fcd1cb58df74 Mon Sep 17 00:00:00 2001 From: Mike Date: Thu, 27 Jun 2024 08:39:31 +0100 Subject: [PATCH] Add sanitizer build options for host (#2842) This PR adds a couple of build variables so host builds can be compiled with the available address sanitisers. I spent way too much time today trying to track down some odd behaviour with the graphics library virtual screen in MacOS. Since valgrind isn't available, I tried enabling some address sanitisers which I've not used before. Actually seems more effective than valgrind and is supported on both GCC and CLANG (and MacOS). Found the issue immediately. There are a lot of possible sanitizer options so I've picked some which look most helpful. Also added stack checking. All of this obviously affects build size and runtime speed. To try this out: ``` export ENABLE_SANITIZERS=1 make clean components-clean make flash run ``` That's it. `make list-config | grep SANITIZER` will show applicable options. NB. GCC requires the `libasan` and `libubsan` packages. **Bug in MacOS getHostAppDir** Bad len value causes memory corruption, very difficult to track down without valgrind/santizers. **Fix graphics virtual display handling with 64-bit host** Small fix found whilst hunting the above bug, affects all 64-bit builds not just MacOS. --- Sming/Arch/Host/Components/hostlib/hostlib.c | 2 +- Sming/Arch/Host/README.rst | 37 +++++++++++++++++--- Sming/Arch/Host/app.mk | 4 +++ Sming/Arch/Host/build.mk | 16 +++++++++ Sming/Libraries/Graphics | 2 +- Sming/Wiring/FakePgmSpace.cpp | 6 +++- 6 files changed, 60 insertions(+), 7 deletions(-) diff --git a/Sming/Arch/Host/Components/hostlib/hostlib.c b/Sming/Arch/Host/Components/hostlib/hostlib.c index 0671b8bb79..5dca640e45 100644 --- a/Sming/Arch/Host/Components/hostlib/hostlib.c +++ b/Sming/Arch/Host/Components/hostlib/hostlib.c @@ -44,7 +44,7 @@ size_t getHostAppDir(char* path, size_t bufSize) char sep = '\\'; #elif defined(__APPLE__) uint32_t size = bufSize; - size_t len = _NSGetExecutablePath(path, &size) ? 0 : size; + size_t len = _NSGetExecutablePath(path, &size) ? 0 : strlen(path); char sep = '/'; #else size_t len = readlink("/proc/self/exe", path, bufSize - 1); diff --git a/Sming/Arch/Host/README.rst b/Sming/Arch/Host/README.rst index e3a90e4fa1..b145c61763 100644 --- a/Sming/Arch/Host/README.rst +++ b/Sming/Arch/Host/README.rst @@ -52,6 +52,12 @@ To find out what options are in force, use ``make list-config``. Configuration ------------- +.. note:: + + The following settings are for debugging purposes and are not 'sticky'. + Where used, they should generally be defined globally using ``export``. + + .. envvar:: CLI_TARGET_OPTIONS Use this to add any custom options to the emulator command line. e.g.: @@ -59,8 +65,6 @@ Configuration make run CLI_TARGET_OPTIONS=--help make run CLI_TARGET_OPTIONS="--debug=0 --cpulimit=2" - Note: These settings are not 'sticky' - .. envvar:: CLANG_BUILD @@ -68,8 +72,6 @@ Configuration 1: Use standard ``clang`` N: Use specific installed version, ``clang-N`` - Note: This setting is not 'sticky' - .. envvar:: BUILD64 @@ -79,6 +81,33 @@ Configuration On MacOS builds are 64-bit only. Default for other systems is 32-bit. +.. envvar:: ENABLE_SANITIZERS + + default: 0 (off) + + Enable this option to build with lots of runtime checking. + + This provides some of the capabilities of valgrind but by instrumenting + the code when it is compiled, rather than patching at runtime. + + It also links in some additional runtime support libraries. + + Run a full rebuild after changing this setting (or :envvar:`SANITIZERS`):: + + make clean components-clean + make + + .. note:: + + If using :envvar:`CLANG_BUILD` then all runtime libraries should already be available. + For GCC you will also need to install ``libasan`` and ``libubsan``. + + +.. envvar:: SANITIZERS + + Selects which sanitizers are used. See :envvar:`ENABLE_SANITIZERS`. + + Components ---------- diff --git a/Sming/Arch/Host/app.mk b/Sming/Arch/Host/app.mk index cf1d452e3d..3d0540d206 100644 --- a/Sming/Arch/Host/app.mk +++ b/Sming/Arch/Host/app.mk @@ -9,6 +9,10 @@ ifneq ($(BUILD64),1) LDFLAGS += -m32 endif +ifeq ($(ENABLE_SANITIZERS),1) +LDFLAGS += $(foreach s,$(SANITIZERS),-fsanitize=$s) +endif + # Executable TARGET_OUT_0 := $(FW_BASE)/$(APP_NAME)$(TOOL_EXT) diff --git a/Sming/Arch/Host/build.mk b/Sming/Arch/Host/build.mk index 885e6f22b2..3758752fc8 100644 --- a/Sming/Arch/Host/build.mk +++ b/Sming/Arch/Host/build.mk @@ -54,6 +54,22 @@ CPPFLAGS += \ -D_FILE_OFFSET_BITS=64 \ -D_TIME_BITS=64 +# Sanitizers +DEBUG_VARS += ENABLE_SANITIZERS SANITIZERS +ENABLE_SANITIZERS ?= 0 +SANITIZERS ?= \ + address \ + pointer-compare \ + pointer-subtract \ + leak \ + undefined +ifeq ($(ENABLE_SANITIZERS),1) +CPPFLAGS += \ + -fstack-protector-all \ + -fsanitize-address-use-after-scope \ + $(foreach s,$(SANITIZERS),-fsanitize=$s) +endif + # => Tools MEMANALYZER = size diff --git a/Sming/Libraries/Graphics b/Sming/Libraries/Graphics index 583f2fef95..9c3031f3ef 160000 --- a/Sming/Libraries/Graphics +++ b/Sming/Libraries/Graphics @@ -1 +1 @@ -Subproject commit 583f2fef9596c0a5f1a783da27eb94cc92839835 +Subproject commit 9c3031f3efd8aed50a8abe23749c5e99d18638fc diff --git a/Sming/Wiring/FakePgmSpace.cpp b/Sming/Wiring/FakePgmSpace.cpp index 156a31df6c..0a032c5453 100644 --- a/Sming/Wiring/FakePgmSpace.cpp +++ b/Sming/Wiring/FakePgmSpace.cpp @@ -19,7 +19,11 @@ void* memcpy_aligned(void* dst, const void* src, unsigned len) { assert(IS_ALIGNED(dst) && IS_ALIGNED(src)); - memcpy(dst, src, ALIGNUP4(len)); +#ifndef ARCH_HOST + // Address sanitisers get tripped if we do this in Host builds + len = ALIGNUP4(len); +#endif + memcpy(dst, src, len); return dst; }