June 2025 - coreboot-gerrit - mail.coreboot.org

[M] Change in coreboot[main]: tree: Replace scan-build by clang-tidy
by Elyes Haouas (Code Review) June 1, 2025

June 1, 2025

Elyes Haouas has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/87908?usp=email ) Change subject: tree: Replace scan-build by clang-tidy ...................................................................... tree: Replace scan-build by clang-tidy Change-Id: I0e59a1667759723bbf8d76232e7e5375837d2e9a Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> --- M Makefile.mk M payloads/libpayload/Makefile M toolchain.mk M util/abuild/abuild M util/cbfstool/lz4/Makefile M util/crossgcc/buildgcc M util/testing/Makefile.mk 7 files changed, 66 insertions(+), 53 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/87908/1 diff --git a/Makefile.mk b/Makefile.mk index 3969bfb..218e388 100644 --- a/Makefile.mk +++ b/Makefile.mk @@ -532,8 +532,8 @@ ifeq ($(CONFIG_COMPILER_GCC),y) CFLAGS_common += -Wold-style-declaration CFLAGS_common += -Wcast-function-type -# Don't add these GCC specific flags when running scan-build -ifeq ($(CCC_ANALYZER_OUTPUT_FORMAT),) +# Don't add these GCC specific flags when running clang-tidy +ifeq ($(CLANG_TIDY),) CFLAGS_common += -Wno-packed-not-aligned CFLAGS_common += -fconserve-stack CFLAGS_common += -Wnull-dereference diff --git a/payloads/libpayload/Makefile b/payloads/libpayload/Makefile index afe38dc..203538b 100644 --- a/payloads/libpayload/Makefile +++ b/payloads/libpayload/Makefile @@ -33,7 +33,7 @@ $(error ERROR: Path to the main directory cannot contain spaces) endif -ifeq ($(INNER_SCANBUILD),y) +ifeq ($(INNER_STATIC_ANALYSIS),y) CC_real:=$(CC) endif @@ -95,7 +95,7 @@ all: real-all -ifeq ($(INNER_SCANBUILD),y) +ifeq ($(INNER_STATIC_ANALYSIS),y) CC:=$(CC_real) HOSTCC:=$(CC_real) --hostcc HOSTCXX:=$(CC_real) --hostcxx @@ -218,7 +218,6 @@ CFLAGS += -std=gnu11 $(CFLAGS_$(ARCH-y)) -ifneq ($(INNER_SCANBUILD),y) ifeq ($(CONFIG_LP_COMPILER_LLVM_CLANG),y) CC:=clang ifneq ($(CONFIG_LP_ARCH_MOCK),y) @@ -226,7 +225,6 @@ endif HOSTCC:=clang endif -endif ifeq ($(CONFIG_LP_CCACHE),y) CCACHE:=$(word 1,$(wildcard $(addsuffix /ccache,$(subst :, ,$(PATH))))) @@ -244,7 +242,7 @@ # The primary target needs to be here before we include the # other files -ifeq ($(INNER_SCANBUILD),y) +ifeq ($(INNER_STATIC_ANALYSIS),y) CONFIG_LP_SCANBUILD_ENABLE:= endif @@ -253,13 +251,8 @@ CONFIG_LP_SCANBUILD_REPORT_LOCATION:=-o $(CONFIG_LP_SCANBUILD_REPORT_LOCATION) endif real-all: - echo '#!/bin/sh' > .ccwrap - echo 'CC="$(CC)"' >> .ccwrap - echo 'if [ "$$1" = "--hostcc" ]; then shift; CC="$(HOSTCC)"; fi' >> .ccwrap - echo 'if [ "$$1" = "--hostcxx" ]; then shift; CC="$(HOSTCXX)"; fi' >> .ccwrap - echo 'eval $$CC $$*' >> .ccwrap - chmod +x .ccwrap - scan-build $(CONFIG_LP_SCANBUILD_REPORT_LOCATION) -analyze-headers --use-cc=$(top)/.ccwrap --use-c++=$(top)/.ccwrap $(MAKE) INNER_SCANBUILD=y + find src -name '*.c' -o -name '*.h' | xargs clang-tidy $(CONFIG_LP_CLANG_TIDY_CHECKS) + $(MAKE) INNER_STATIC_ANALYSIS=y else real-all: real-target endif diff --git a/toolchain.mk b/toolchain.mk index 8d6d449..1d428dc 100644 --- a/toolchain.mk +++ b/toolchain.mk @@ -19,11 +19,10 @@ HOSTCXX:=$(CCACHE) $(HOSTCXX) endif -# scan-build integration -ifneq ($(CCC_ANALYZER_OUTPUT_FORMAT),) - -ifeq ($(CCC_ANALYZER_ANALYSIS),) -export CCC_ANALYZER_ANALYSIS := -analyzer-opt-analyze-headers +# clang-tidy integration +ifneq ($(CLANG_TIDY),) +CLANG_TIDY_CHECKS ?= -checks=* +CLANG_TIDY_ARGS ?= -extra-arg=-Wno-packed-not-aligned endif $(foreach arch,$(ARCH_SUPPORTED), \ diff --git a/util/abuild/abuild b/util/abuild/abuild index faac3eb..8483942 100755 --- a/util/abuild/abuild +++ b/util/abuild/abuild @@ -85,8 +85,8 @@ # quiet mode: only print pass, failure, and 'skipped' messages quiet=false -# clang mode enabled by -sb option. -scanbuild=false +# clang-tidy mode enabled by -sb option. +clangtidy=false # Mark whether abuild was called recursively recursive=false @@ -417,6 +417,35 @@ fi } +run_clang_tidy() +{ + local build_dir="$1" + local log_file="$2" + local clang_tidy_args="" + + if [[ -n "${CLANG_TIDY_CHECKS}" ]]; then + clang_tidy_args+="-checks=${CLANG_TIDY_CHECKS} " + fi + + # Generate compilation database + printf "Generating compilation database for clang-tidy...\n" + ${MAKE} -C "${ROOT}" DOTCONFIG="${build_dir}/config.build" obj="${build_dir}" \ + objutil="${TARGET}/sharedutils" BUILD_TIMELESS=${TIMELESS} \ + compiledb >/dev/null 2>&1 + + if [[ ! -f "${build_dir}/compile_commands.json" ]]; then + printf "Error: Failed to generate compilation database\n" >> "${log_file}" + return 1 + fi + + # Run clang-tidy on all source files + find "${ROOT}/src" -type f $ -name "*.c" -o -name "*.h" $ | while read -r src_file; do + clang-tidy ${clang_tidy_args} "${src_file}" >> "${log_file}" 2>&1 + done + + return 0 +} + compile_target() { local BUILD_NAME=$1 @@ -453,6 +482,17 @@ printf "ok\n" > compile.status printf "%s built successfully. (took %s)\n" "${BUILD_NAME}" "${duration_str}" echo "${BUILD_NAME}" >> "${PASSED_BOARDS}" + + # Run clang-tidy after successful build + if [[ "${clangtidy}" = "true" ]]; then + printf "Running clang-tidy for %s...\n" "${BUILD_NAME}" + run_clang_tidy "${build_dir}" "${build_dir}/clang-tidy.log" + if [[ $? -eq 0 ]]; then + printf "clang-tidy completed for %s\n" "${BUILD_NAME}" + else + printf "clang-tidy failed for %s (see %s/clang-tidy.log)\n" "${BUILD_NAME}" "${build_dir}" + fi + fi else junit "<failure type='BuildFailed'>" junitfile make.log @@ -606,16 +646,10 @@ if [[ ${BUILDENV_CREATED} -eq 0 ]] && [[ ${configureonly} -eq 0 ]]; then BUILDPREFIX= - if [[ "${scanbuild}" = "true" ]]; then - scanbuild_out="${TARGET}/${BUILD_NAME}-scanbuild" - rm -rf "${scanbuild_out}" - BUILDPREFIX="scan-build ${SCANBUILD_ARGS} -o ${scanbuild_out}tmp" + if [[ "${clangtidy}" = "true" ]]; then + printf "Enabling clang-tidy for static analysis\n" fi compile_target "${BUILD_NAME}" - if [[ "${scanbuild}" = "true" ]]; then - mv "${scanbuild_out}tmp"/* "${scanbuild_out}" - rmdir "${scanbuild_out}tmp" - fi fi junit "</testcase>" @@ -720,7 +754,7 @@ [-r|--remove] Remove output dir after build [-R|--root <path>] Absolute path to coreboot sources (defaults to ${ROOT}) - [--scan-build] Use clang's static analyzer + [--static-analysis] Run clang-tidy for static code analysis [--skip_set <value>] Skip building boards with this Kconfig set [--skip_unset <value>] Skip building boards with this Kconfig not set [--timeless] Generate timeless builds @@ -780,7 +814,7 @@ # shellcheck disable=SC2086 if [[ "${getoptbrand:0:6}" == "getopt" ]]; then # Detected GNU getopt that supports long options. - args=$(getopt -l version,verbose,quiet,help,all,target:,board-variant:,payloads:,cpus:,silent,junit,config,loglevel:,remove,prefix:,update,scan-build,ccache,blobs,clang,any-toolchain,clean,clean-somewhat,outdir:,chromeos,xmlfile:,kconfig:,dir:,root:,recursive,checksum:,timeless,exitcode,asserts,name:,skip_set:,skip_unset: -o Vvqhat:b:p:c:sJCl:rP:uyBLAzZo:xX:K:d:R:Ien: -- "$@") || exit 1 + args=$(getopt -l version,verbose,quiet,help,all,target:,board-variant:,payloads:,cpus:,silent,junit,config,loglevel:,remove,prefix:,update,static-analysis,ccache,blobs,clang,any-toolchain,clean,clean-somewhat,outdir:,chromeos,xmlfile:,kconfig:,dir:,root:,recursive,checksum:,timeless,exitcode,asserts,name:,skip_set:,skip_unset: -o Vvqhat:b:p:c:sJCl:rP:uyBLAzZo:xX:K:d:R:Ien: -- "$@") || exit 1 eval set -- ${args} retval=$? else @@ -828,10 +862,9 @@ shift;; # obsolete option -s|--silent) shift;; - --scan-build) shift - scanbuild=true - customizing="${customizing}, scan-build" - SCANBUILD_ARGS=${SCANBUILD_ARGS:-'-k'} + --static-analysis) shift + clangtidy=true + customizing="${customizing}, static-analysis" configoptions="${configoptions}CONFIG_FATAL_ASSERTS=y\n" ;; --skip_set) shift @@ -1008,17 +1041,12 @@ printf "%s" "${configoptions}" > "${TMPCFG}" ${MAKE} -j "${cpus}" DOTCONFIG="${TMPCFG}" obj="${TARGET}/temp" objutil="${TARGET}/sharedutils" olddefconfig 2>/dev/null BUILDPREFIX= - if [[ "${scanbuild}" = "true" ]]; then - scanbuild_out="${TARGET}/sharedutils-scanbuild" - rm -rf "${scanbuild_out}" - BUILDPREFIX="scan-build -o ${scanbuild_out}tmp" - fi mkdir -p "${TARGET}/abuild" ABSPATH="$(cd "${TARGET}/abuild" && pwd)" XMLFILE="${ABSPATH}/__util.xml" rm -f "${XMLFILE}" stime=$(add_timestamp) - ${BUILDPREFIX} "${MAKE}" -j "${cpus}" DOTCONFIG="${TMPCFG}" obj="${TARGET}/temp" objutil="${TARGET}/sharedutils" tools > "${TARGET}/sharedutils/make.log" 2>&1 + ${MAKE} -j "${cpus}" DOTCONFIG="${TMPCFG}" obj="${TARGET}/temp" objutil="${TARGET}/sharedutils" tools > "${TARGET}/sharedutils/make.log" 2>&1 local ret=$? etime=$(add_timestamp) duration=$(ts_delta_seconds "${stime}" "${etime}") @@ -1039,10 +1067,6 @@ return fi - if [[ "${scanbuild}" = "true" ]]; then - mv "${scanbuild_out}"tmp/* "${scanbuild_out}" - rmdir "${scanbuild_out}tmp" - fi rm -rf "${TARGET}/temp" "${TMPCFG}" num_targets=$(wc -w <<<"${targets}") cpus_per_target=$(((${cpus:-1} + num_targets - 1) / num_targets)) @@ -1106,4 +1130,4 @@ fi fi -exit ${failed} +exit ${failed} \ No newline at end of file diff --git a/util/cbfstool/lz4/Makefile b/util/cbfstool/lz4/Makefile index 4506267..d624e84 100644 --- a/util/cbfstool/lz4/Makefile +++ b/util/cbfstool/lz4/Makefile @@ -81,7 +81,7 @@ CFLAGS="-O3 -g -fsanitize=undefined" $(MAKE) test CC=clang FUZZER_TIME="-T1mn" NB_LOOPS=-i1 staticAnalyze: clean - CFLAGS=-g scan-build --status-bugs -v $(MAKE) all + CFLAGS=-g clang-tidy $(CLANG_TIDY_CHECKS) $(CLANG_TIDY_ARGS) $(SRCS) armtest: clean CFLAGS="-O3 -Werror" $(MAKE) -C $(LZ4DIR) all CC=arm-linux-gnueabi-gcc diff --git a/util/crossgcc/buildgcc b/util/crossgcc/buildgcc index 1b0c89a..ca8df15 100755 --- a/util/crossgcc/buildgcc +++ b/util/crossgcc/buildgcc @@ -923,9 +923,6 @@ $MAKE install || touch .failed rm -f ../llvm ../clang ../clang-tools-extra ../compiler-rt ../cmake ../lld ../libunwind - - cp -a ../$CLANG_DIR/tools/scan-build/* "$DESTDIR$TARGETDIR/bin" - cp -a ../$CLANG_DIR/tools/scan-view/* "$DESTDIR$TARGETDIR/bin" } build_CMAKE() { diff --git a/util/testing/Makefile.mk b/util/testing/Makefile.mk index c73502e..58ceca6 100644 --- a/util/testing/Makefile.mk +++ b/util/testing/Makefile.mk @@ -116,8 +116,8 @@ util/lint/lint lint-extended $(JUNIT) test-abuild: -ifneq ($(JENKINS_SKIP_SCANBUILD_TEST),y) - NAME=scanbuild; SCANBUILD_ARGS='-k -plist-html -maxloop 10' util/abuild/abuild -o $(COREBOOT_BUILD_DIR)/$${NAME} $(ABUILD_OPTIONS) --scan-build --target EMULATION_QEMU_X86_Q35 --exitcode --name $${NAME} +ifneq ($(JENKINS_SKIP_STATIC_ANALYSIS_TEST),y) + NAME=clangtidy; util/abuild/abuild -o $(COREBOOT_BUILD_DIR)/$${NAME} $(ABUILD_OPTIONS) --static-analysis --target EMULATION_QEMU_X86_Q35 endif ifneq ($(JENKINS_SKIP_GCC_TESTS),y) ifneq ($(JENKINS_SKIP_ALLTHREAD_TEST),y) @@ -172,7 +172,7 @@ test-cleanup: rm -rf $(COREBOOT_BUILD_DIR)/chromeos $(COREBOOT_BUILD_DIR)/default rm -rf $(COREBOOT_BUILD_DIR)/chromeos-clang $(COREBOOT_BUILD_DIR)/default-clang - rm -rf $(COREBOOT_BUILD_DIR)/scanbuild + rm -rf $(COREBOOT_BUILD_DIR)/clangtidy $(MAKE) clean $(foreach tool, $(TOOLLIST), $(MAKE) -C util/$(tool) clean ; ) $(MAKE) -C src/soc/nvidia/tegra124/lp0 clean -- To view, visit https://review.coreboot.org/c/coreboot/+/87908?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0e59a1667759723bbf8d76232e7e5375837d2e9a Gerrit-Change-Number: 87908 Gerrit-PatchSet: 1 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>

1 0

[S] Change in coreboot[main]: device/device_util.c: Replace some TODOs in comments with descriptions
by NyeonWoo Kim (Code Review) June 1, 2025

June 1, 2025

NyeonWoo Kim has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/87907?usp=email ) Change subject: device/device_util.c: Replace some TODOs in comments with descriptions ...................................................................... device/device_util.c: Replace some TODOs in comments with descriptions I've found some TODOs in comment in device_util.c. So I replaced them with descriptions for readability. Change-Id: I429ce0b2a1b56a60fdb4127591ec219768c2c044 Signed-off-by: NyeonWoo Kim <knw0507(a)naver.com> --- M src/device/device_util.c 1 file changed, 8 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/87907/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/87907?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I429ce0b2a1b56a60fdb4127591ec219768c2c044 Gerrit-Change-Number: 87907 Gerrit-PatchSet: 2 Gerrit-Owner: NyeonWoo Kim Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[S] Change in coreboot[main]: device/device_util.c: Replace some TODOs in comments with descriptions.
by NyeonWoo Kim (Code Review) June 1, 2025

June 1, 2025

NyeonWoo Kim has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/87907?usp=email ) Change subject: device/device_util.c: Replace some TODOs in comments with descriptions. ...................................................................... device/device_util.c: Replace some TODOs in comments with descriptions. I've found some TODOs in comment in device_util.c. So I replaced them with descriptions for readability. Change-Id: I429ce0b2a1b56a60fdb4127591ec219768c2c044 Signed-off-by: NyeonWoo Kim <knw0507(a)naver.com> --- M src/device/device_util.c 1 file changed, 8 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/87907/1 diff --git a/src/device/device_util.c b/src/device/device_util.c index fc585a0..864e64c 100644 --- a/src/device/device_util.c +++ b/src/device/device_util.c @@ -293,7 +293,7 @@ /** * Allocate 64 more resources to the free list. * - * @return TODO. + * @return Return 0 for allocation failure, Return 1 for allocation success. */ static int allocate_more_resources(void) { @@ -317,10 +317,9 @@ /** * Remove resource res from the device's list and add it to the free list. * - * @param dev TODO - * @param res TODO - * @param prev TODO - * @return TODO. + * @param dev The device which contains a resource to be freed(detached). + * @param res The resource to be freed(detached) which is part of 'dev'. + * @param prev The resource for 'dev' to be linked to next resource after 'res' is removed. */ static void free_resource(struct device *dev, struct resource *res, struct resource *prev) @@ -383,7 +382,7 @@ * * @param dev The device to find the resource on. * @param index The index of the resource on the device. - * @return TODO. + * @return The resource. if it exists, then return existing one, else return new one. */ struct resource *new_resource(struct device *dev, unsigned int index) { @@ -431,7 +430,7 @@ * * @param dev The device to find the resource on. * @param index The index of the resource on the device. - * return TODO. + * @return The resource found in 'dev' for 'index'. */ struct resource *find_resource(const struct device *dev, unsigned int index) { @@ -520,7 +519,7 @@ * Return the resource type of a resource. * * @param resource The resource type to decode. - * @return TODO. + * @return The string representation of type. */ const char *resource_type(const struct resource *resource) { @@ -542,7 +541,7 @@ * * @param dev The device the stored resource lives on. * @param resource The resource that was just stored. - * @param comment TODO + * @param comment The comment just for additional information. */ void report_resource_stored(struct device *dev, const struct resource *resource, const char *comment) -- To view, visit https://review.coreboot.org/c/coreboot/+/87907?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I429ce0b2a1b56a60fdb4127591ec219768c2c044 Gerrit-Change-Number: 87907 Gerrit-PatchSet: 1 Gerrit-Owner: NyeonWoo Kim

1 0