* [LTP] [RFC PATCH v2 0/2] Libclang based analyzer @ 2021-06-04 11:14 Richard Palethorpe 2021-06-04 11:14 ` [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system Richard Palethorpe ` (3 more replies) 0 siblings, 4 replies; 20+ messages in thread From: Richard Palethorpe @ 2021-06-04 11:14 UTC (permalink / raw) To: ltp Hello, This implements a TEST() check and integrates the check into the build system. Compared to the Coccinelle version it's very ugly. However I think this will allow us to get all the low hanging fruit without creating major problems for test developers. I guess it could be run during CI if we either fix all the existing TEST() usages in the library or add an ignore list. I already have a Coccinelle script to help with the former. V2: * Consistently use singular form of 'check' * Include missing clang-check.mk * Add some more comments in main.c Richard Palethorpe (2): Add 'make check' and clang-check to build system Start libclang based analyzer and TEST() check configure.ac | 2 + include/mk/clang-check.mk | 9 ++ include/mk/config.mk.in | 5 + include/mk/env_post.mk | 8 + include/mk/generic_leaf_target.inc | 5 +- include/mk/lib.mk | 3 + include/mk/rules.mk | 9 ++ include/mk/testcases.mk | 1 + tools/clang-check/.gitignore | 1 + tools/clang-check/Makefile | 14 ++ tools/clang-check/main.c | 239 +++++++++++++++++++++++++++++ 11 files changed, 295 insertions(+), 1 deletion(-) create mode 100644 include/mk/clang-check.mk create mode 100644 tools/clang-check/.gitignore create mode 100644 tools/clang-check/Makefile create mode 100644 tools/clang-check/main.c -- 2.31.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system 2021-06-04 11:14 [LTP] [RFC PATCH v2 0/2] Libclang based analyzer Richard Palethorpe @ 2021-06-04 11:14 ` Richard Palethorpe 2021-06-04 14:10 ` Petr Vorel ` (2 more replies) 2021-06-04 11:14 ` [LTP] [RFC PATCH v2 2/2] Start libclang based analyzer and TEST() check Richard Palethorpe ` (2 subsequent siblings) 3 siblings, 3 replies; 20+ messages in thread From: Richard Palethorpe @ 2021-06-04 11:14 UTC (permalink / raw) To: ltp Allows the user to run 'make check' to check all source files or 'make check-<target>' to check one source file corresponding to a target. Adds makefile pieces for tools/clang-check/main which will be a libclang based tool. By default this is ran by 'make check'. In theory allows other tools to be specified with 'make CHECK=tool CHECK_FLAGS=<args> check...'. e.g. 'make CHECK=sparse CHECK_FLAGS= check-tst_cgroup' Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> --- configure.ac | 2 ++ include/mk/clang-check.mk | 9 +++++++++ include/mk/config.mk.in | 5 +++++ include/mk/env_post.mk | 8 ++++++++ include/mk/generic_leaf_target.inc | 5 ++++- include/mk/lib.mk | 3 +++ include/mk/rules.mk | 9 +++++++++ include/mk/testcases.mk | 1 + tools/clang-check/.gitignore | 1 + tools/clang-check/Makefile | 14 ++++++++++++++ 10 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 include/mk/clang-check.mk create mode 100644 tools/clang-check/.gitignore create mode 100644 tools/clang-check/Makefile diff --git a/configure.ac b/configure.ac index 136d82d09..b37c13c3c 100644 --- a/configure.ac +++ b/configure.ac @@ -14,6 +14,7 @@ AC_CONFIG_FILES([ \ ]) AC_ARG_VAR(HOSTCC, [The C compiler on the host]) +AC_ARG_VAR(CLANG, [The LLVM Clang C compiler on the host]) AM_MAINTAINER_MODE([enable]) @@ -42,6 +43,7 @@ AC_CHECK_DECLS([SEM_STAT_ANY],,,[#include <sys/sem.h>]) AC_CHECK_HEADERS_ONCE([ \ asm/ldt.h \ + clang-c/Index.h \ ifaddrs.h \ keyutils.h \ linux/can.h \ diff --git a/include/mk/clang-check.mk b/include/mk/clang-check.mk new file mode 100644 index 000000000..2ab7b67a1 --- /dev/null +++ b/include/mk/clang-check.mk @@ -0,0 +1,9 @@ +# Rules to make clang-check tool(s) for inclusion in lib and testcases Makefiles + +CLANG_CHECK_DIR:= $(abs_top_builddir)/tools/clang-check + +$(CLANG_CHECK_DIR)/main: $(CLANG_CHECK_DIR) + $(MAKE) -C "$^" -f "$(CLANG_CHECK_DIR)/Makefile" all + +$(CLANG_CHECK_DIR): %: + mkdir -p "$@" diff --git a/include/mk/config.mk.in b/include/mk/config.mk.in index 218447ef3..361b6a746 100644 --- a/include/mk/config.mk.in +++ b/include/mk/config.mk.in @@ -44,6 +44,11 @@ HOSTCC := cc endif endif +CLANG := @CLANG@ +ifeq ($(strip $(CLANG)),) +CLANG := clang +endif + AIO_LIBS := @AIO_LIBS@ CAP_LIBS := @CAP_LIBS@ ACL_LIBS := @ACL_LIBS@ diff --git a/include/mk/env_post.mk b/include/mk/env_post.mk index 1d22f9c53..8903a934d 100644 --- a/include/mk/env_post.mk +++ b/include/mk/env_post.mk @@ -89,6 +89,14 @@ $(error You must define $$(prefix) before executing install) endif # END $(filter-out install,$(MAKECMDGOALS)),$(MAKECMDGOALS) endif +CHECK_TARGETS ?= $(addprefix check-,$(MAKE_TARGETS)) +CHECK ?= $(abs_top_srcdir)/tools/clang-check/main +CHECK_FLAGS ?= -resource-dir $(shell $(CLANG) -print-resource-dir) + +ifeq ($(dir $(CHECK)),$(abs_top_srcdir)/tools/clang-check/) +CHECK_DEPS += $(CHECK) +endif + include $(top_srcdir)/include/mk/rules.mk endif diff --git a/include/mk/generic_leaf_target.inc b/include/mk/generic_leaf_target.inc index 64953f89a..aa092a5a3 100644 --- a/include/mk/generic_leaf_target.inc +++ b/include/mk/generic_leaf_target.inc @@ -92,7 +92,7 @@ # INSTALL_DIR := $(libdir) # -.PHONY: all clean install +.PHONY: all clean install check ifneq ($(strip $(MAKE_TARGETS)),) $(MAKE_TARGETS) += $(HOST_MAKE_TARGETS) @@ -109,4 +109,7 @@ $(INSTALL_FILES): | $(INSTALL_DEPS) install: $(INSTALL_FILES) +$(CHECK_TARGETS): | $(CHECK_DEPS) +check: $(CHECK_TARGETS) + # vim: syntax=make diff --git a/include/mk/lib.mk b/include/mk/lib.mk index f9b6c0aff..a3961bce5 100644 --- a/include/mk/lib.mk +++ b/include/mk/lib.mk @@ -26,6 +26,7 @@ # Makefile to include for libraries. include $(top_srcdir)/include/mk/env_pre.mk +include $(top_srcdir)/include/mk/clang-check.mk INSTALL_DIR := $(libdir) @@ -57,6 +58,8 @@ LIBSRCS := $(filter-out $(FILTER_OUT_LIBSRCS),$(LIBSRCS)) LIBOBJS := $(LIBSRCS:.c=.o) +CHECK_TARGETS := $(addprefix check-,$(notdir $(LIBSRCS:.c=))) + $(LIB): $(notdir $(LIBOBJS)) @if [ -z "$(strip $^)" ] ; then \ echo "Cowardly refusing to create empty archive"; \ diff --git a/include/mk/rules.mk b/include/mk/rules.mk index c8f4bbbbe..2a04b2b67 100644 --- a/include/mk/rules.mk +++ b/include/mk/rules.mk @@ -37,3 +37,12 @@ else @$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) $^ $(LTPLDLIBS) $(LDLIBS) -o $@ @echo CC $(target_rel_dir)$@ endif + +.PHONY: $(CHECK_TARGETS) +$(CHECK_TARGETS): check-%: %.c +ifdef VERBOSE + $(CHECK) $(CHECK_FLAGS) $(CPPFLAGS) $(CFLAGS) $< +else + @$(CHECK) $(CHECK_FLAGS) $(CPPFLAGS) $(CFLAGS) $< + @echo CHECK $(target_rel_dir)$< +endif diff --git a/include/mk/testcases.mk b/include/mk/testcases.mk index 1c81773d0..e59899898 100644 --- a/include/mk/testcases.mk +++ b/include/mk/testcases.mk @@ -22,6 +22,7 @@ include $(top_srcdir)/include/mk/env_pre.mk include $(top_srcdir)/include/mk/functions.mk +include $(top_srcdir)/include/mk/clang-check.mk APICMDS_DIR := $(abs_top_builddir)/tools/apicmds diff --git a/tools/clang-check/.gitignore b/tools/clang-check/.gitignore new file mode 100644 index 000000000..ba2906d06 --- /dev/null +++ b/tools/clang-check/.gitignore @@ -0,0 +1 @@ +main diff --git a/tools/clang-check/Makefile b/tools/clang-check/Makefile new file mode 100644 index 000000000..4650d6057 --- /dev/null +++ b/tools/clang-check/Makefile @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (c) 2021 SUSE LLC <rpalethorpe@suse.com> +# Copyright (c) 2019 Cyril Hrubis <chrubis@suse.cz> +# Copyright (c) 2020 Petr Vorel <pvorel@suse.cz> + +top_srcdir ?= ../.. + +include $(top_srcdir)/include/mk/env_pre.mk +include $(top_srcdir)/include/mk/functions.mk + +HOST_MAKE_TARGETS := main +HOST_LDFLAGS += -lclang + +include $(top_srcdir)/include/mk/generic_leaf_target.mk -- 2.31.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system 2021-06-04 11:14 ` [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system Richard Palethorpe @ 2021-06-04 14:10 ` Petr Vorel 2021-06-04 14:11 ` Cyril Hrubis 2021-06-04 14:28 ` Richard Palethorpe 2021-06-07 9:18 ` Joerg Vehlow 2021-06-11 13:49 ` Petr Vorel 2 siblings, 2 replies; 20+ messages in thread From: Petr Vorel @ 2021-06-04 14:10 UTC (permalink / raw) To: ltp Hi Richie, > Allows the user to run 'make check' to check all source files or > 'make check-<target>' to check one source file corresponding to a > target. > Adds makefile pieces for tools/clang-check/main which will be a > libclang based tool. By default this is ran by 'make check'. I haven't looked at Coccinelle, but this LGTM. But, it still fails to compile: $ make autotools && ./configure && cd testcases/kernel/syscalls/chown/ $ make clean; make make -C "/home/pvorel/install/src/ltp.git/lib" -f "/src/ltp/lib/Makefile" all make[1]: Entering directory '/src/ltp/lib' make[2]: Nothing to be done for 'all'. make[2]: Nothing to be done for 'all'. make[1]: Leaving directory '/src/ltp/lib' CC testcases/kernel/syscalls/chown/chown01.o LD testcases/kernel/syscalls/chown/chown01 CC testcases/kernel/syscalls/chown/chown02.o LD testcases/kernel/syscalls/chown/chown02 CC testcases/kernel/syscalls/chown/chown03.o LD testcases/kernel/syscalls/chown/chown03 CC testcases/kernel/syscalls/chown/chown04.o LD testcases/kernel/syscalls/chown/chown04 CC testcases/kernel/syscalls/chown/chown05.o LD testcases/kernel/syscalls/chown/chown05 make: *** No rule to make target 'chown01_16.c', needed by 'chown01_16'. Stop. rm chown01.o chown03.o chown02.o chown05.o chown04.o This is a newly introduced failure caused by some change in include/mk/ in this commit. Could we have also make check in the top level Makefile? $ make check make: *** No rule to make target 'check'. Stop. $ cd lib && make check CHECK lib/cloner.c CHECK lib/get_path.c CHECK lib/parse_opts.c CHECK lib/random_range.c CHECK lib/safe_file_ops.c CHECK lib/safe_macros.c CHECK lib/safe_net.c CHECK lib/safe_pthread.c CHECK lib/safe_stdio.c CHECK lib/self_exec.c CHECK lib/tlibio.c tst_af_alg.c:16:2: CHECK ERROR: TEST() macro should not be used in library tst_af_alg.c:27:2: CHECK ERROR: TEST() macro should not be used in library tst_af_alg.c:74:2: CHECK ERROR: TEST() macro should not be used in library tst_af_alg.c:109:2: CHECK ERROR: TEST() macro should not be used in library tst_af_alg.c:119:2: CHECK ERROR: TEST() macro should not be used in library make: *** [../include/mk/rules.mk:46: check-tst_af_alg] Error 1 Similarly what I added to my patchset which also adds new make target: https://patchwork.ozlabs.org/project/ltp/patch/20210603183827.24339-2-pvorel@suse.cz/ Although my code has duplicate issue: ../include/mk/generic_trunk_target.inc:105: warning: overriding recipe for target 'check-c' ../include/mk/generic_leaf_target.inc:110: warning: ignoring old recipe for target 'check-c' ../include/mk/generic_trunk_target.inc:105: warning: overriding recipe for target 'check-shell' ../include/mk/generic_leaf_target.inc:118: warning: ignoring old recipe for target 'check-shell' Also make check on regular test expect it's a library. IMHO these two must be probably separated: $ cd testcases/kernel/syscalls/fchown/ && make check CHECK testcases/kernel/syscalls/fchown/fchown01.c CHECK testcases/kernel/syscalls/fchown/fchown02.c CHECK testcases/kernel/syscalls/fchown/fchown03.c CHECK testcases/kernel/syscalls/fchown/fchown04.c fchown05.c:80:4: CHECK ERROR: TEST() macro should not be used in library make: *** [../../../../include/mk/rules.mk:46: check-fchown05] Error 1 Kind regards, Petr ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system 2021-06-04 14:10 ` Petr Vorel @ 2021-06-04 14:11 ` Cyril Hrubis 2021-06-04 14:28 ` Richard Palethorpe 1 sibling, 0 replies; 20+ messages in thread From: Cyril Hrubis @ 2021-06-04 14:11 UTC (permalink / raw) To: ltp Hi! > make -C "/home/pvorel/install/src/ltp.git/lib" -f "/src/ltp/lib/Makefile" all > make[1]: Entering directory '/src/ltp/lib' > make[2]: Nothing to be done for 'all'. > make[2]: Nothing to be done for 'all'. > make[1]: Leaving directory '/src/ltp/lib' > CC testcases/kernel/syscalls/chown/chown01.o > LD testcases/kernel/syscalls/chown/chown01 > CC testcases/kernel/syscalls/chown/chown02.o > LD testcases/kernel/syscalls/chown/chown02 > CC testcases/kernel/syscalls/chown/chown03.o > LD testcases/kernel/syscalls/chown/chown03 > CC testcases/kernel/syscalls/chown/chown04.o > LD testcases/kernel/syscalls/chown/chown04 > CC testcases/kernel/syscalls/chown/chown05.o > LD testcases/kernel/syscalls/chown/chown05 > make: *** No rule to make target 'chown01_16.c', needed by 'chown01_16'. Stop. > rm chown01.o chown03.o chown02.o chown05.o chown04.o > > This is a newly introduced failure caused by some change in include/mk/ in this > commit. This is caused by the definition of CHECK_TARGETS in the env_post.mk, the foo_16 binaries are generated from foo.c sources and there is no foo_16.c which in turn confuses the check code. I.e. what this means is that MAKE_TARGETS != list of sources without suffix and we have to do: diff --git a/include/mk/env_post.mk b/include/mk/env_post.mk index 8903a934d..c6367b0a5 100644 --- a/include/mk/env_post.mk +++ b/include/mk/env_post.mk @@ -89,7 +89,7 @@ $(error You must define $$(prefix) before executing install) endif # END $(filter-out install,$(MAKECMDGOALS)),$(MAKECMDGOALS) endif -CHECK_TARGETS ?= $(addprefix check-,$(MAKE_TARGETS)) +CHECK_TARGETS ?= $(addprefix check-, $(patsubst %.c,%,$(sort $(wildcard $(abs_srcdir)/*.c)))) CHECK ?= $(abs_top_srcdir)/tools/clang-check/main CHECK_FLAGS ?= -resource-dir $(shell $(CLANG) -print-resource-dir) > Also make check on regular test expect it's a library. IMHO these two must be > probably separated: > > $ cd testcases/kernel/syscalls/fchown/ && make check > CHECK testcases/kernel/syscalls/fchown/fchown01.c > CHECK testcases/kernel/syscalls/fchown/fchown02.c > CHECK testcases/kernel/syscalls/fchown/fchown03.c > CHECK testcases/kernel/syscalls/fchown/fchown04.c > fchown05.c:80:4: CHECK ERROR: TEST() macro should not be used in library > make: *** [../../../../include/mk/rules.mk:46: check-fchown05] Error 1 I guess that we can pass different flags to the binary so that it gets the context right. -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system 2021-06-04 14:10 ` Petr Vorel 2021-06-04 14:11 ` Cyril Hrubis @ 2021-06-04 14:28 ` Richard Palethorpe 2021-06-04 14:33 ` Cyril Hrubis 1 sibling, 1 reply; 20+ messages in thread From: Richard Palethorpe @ 2021-06-04 14:28 UTC (permalink / raw) To: ltp Hell Petr, Petr Vorel <pvorel@suse.cz> writes: > Hi Richie, > >> Allows the user to run 'make check' to check all source files or >> 'make check-<target>' to check one source file corresponding to a >> target. > >> Adds makefile pieces for tools/clang-check/main which will be a >> libclang based tool. By default this is ran by 'make check'. > > I haven't looked at Coccinelle, but this LGTM. > > But, it still fails to compile: > > $ make autotools && ./configure && cd testcases/kernel/syscalls/chown/ > $ make clean; make > > make -C "/home/pvorel/install/src/ltp.git/lib" -f "/src/ltp/lib/Makefile" all > make[1]: Entering directory '/src/ltp/lib' > make[2]: Nothing to be done for 'all'. > make[2]: Nothing to be done for 'all'. > make[1]: Leaving directory '/src/ltp/lib' > CC testcases/kernel/syscalls/chown/chown01.o > LD testcases/kernel/syscalls/chown/chown01 > CC testcases/kernel/syscalls/chown/chown02.o > LD testcases/kernel/syscalls/chown/chown02 > CC testcases/kernel/syscalls/chown/chown03.o > LD testcases/kernel/syscalls/chown/chown03 > CC testcases/kernel/syscalls/chown/chown04.o > LD testcases/kernel/syscalls/chown/chown04 > CC testcases/kernel/syscalls/chown/chown05.o > LD testcases/kernel/syscalls/chown/chown05 > make: *** No rule to make target 'chown01_16.c', needed by 'chown01_16'. Stop. > rm chown01.o chown03.o chown02.o chown05.o chown04.o > > This is a newly introduced failure caused by some change in include/mk/ in this > commit. I guess if Cyril likes the overall approach and doesn't spot what is causing this, then I will try fixing it. > > Could we have also make check in the top level Makefile? Yes, I didn't try it for the RFC though. > > $ make check > make: *** No rule to make target 'check'. Stop. > > $ cd lib && make check > CHECK lib/cloner.c > CHECK lib/get_path.c > CHECK lib/parse_opts.c > CHECK lib/random_range.c > CHECK lib/safe_file_ops.c > CHECK lib/safe_macros.c > CHECK lib/safe_net.c > CHECK lib/safe_pthread.c > CHECK lib/safe_stdio.c > CHECK lib/self_exec.c > CHECK lib/tlibio.c > tst_af_alg.c:16:2: CHECK ERROR: TEST() macro should not be used in library > tst_af_alg.c:27:2: CHECK ERROR: TEST() macro should not be used in library > tst_af_alg.c:74:2: CHECK ERROR: TEST() macro should not be used in library > tst_af_alg.c:109:2: CHECK ERROR: TEST() macro should not be used in library > tst_af_alg.c:119:2: CHECK ERROR: TEST() macro should not be used in library > make: *** [../include/mk/rules.mk:46: check-tst_af_alg] Error 1 > > Similarly what I added to my patchset which also adds new make target: > https://patchwork.ozlabs.org/project/ltp/patch/20210603183827.24339-2-pvorel@suse.cz/ > Although my code has duplicate issue: > ../include/mk/generic_trunk_target.inc:105: warning: overriding recipe for target 'check-c' > ../include/mk/generic_leaf_target.inc:110: warning: ignoring old recipe for target 'check-c' > ../include/mk/generic_trunk_target.inc:105: warning: overriding recipe for target 'check-shell' > ../include/mk/generic_leaf_target.inc:118: warning: ignoring old recipe for target 'check-shell' > > Also make check on regular test expect it's a library. IMHO these two must be > probably separated: > > $ cd testcases/kernel/syscalls/fchown/ && make check > CHECK testcases/kernel/syscalls/fchown/fchown01.c > CHECK testcases/kernel/syscalls/fchown/fchown02.c > CHECK testcases/kernel/syscalls/fchown/fchown03.c > CHECK testcases/kernel/syscalls/fchown/fchown04.c > fchown05.c:80:4: CHECK ERROR: TEST() macro should not be used in library > make: *** [../../../../include/mk/rules.mk:46: check-fchown05] Error 1 It fails because it is using the old test API, so there is no struct tst_test test var. I don't think we need to separate the checker. It can detect what type of file (translation unit) it is processing. In the case of old API tests, I would simply disable any checks. All the old tests and test libraries will have something in common. For the tests, they all import "test.h" (even if through another header), so we can identify them by that. > > Kind regards, > Petr -- Thank you, Richard. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system 2021-06-04 14:28 ` Richard Palethorpe @ 2021-06-04 14:33 ` Cyril Hrubis 0 siblings, 0 replies; 20+ messages in thread From: Cyril Hrubis @ 2021-06-04 14:33 UTC (permalink / raw) To: ltp Hi! > > $ make check > > make: *** No rule to make target 'check'. Stop. > > > > $ cd lib && make check > > CHECK lib/cloner.c > > CHECK lib/get_path.c > > CHECK lib/parse_opts.c > > CHECK lib/random_range.c > > CHECK lib/safe_file_ops.c > > CHECK lib/safe_macros.c > > CHECK lib/safe_net.c > > CHECK lib/safe_pthread.c > > CHECK lib/safe_stdio.c > > CHECK lib/self_exec.c > > CHECK lib/tlibio.c > > tst_af_alg.c:16:2: CHECK ERROR: TEST() macro should not be used in library > > tst_af_alg.c:27:2: CHECK ERROR: TEST() macro should not be used in library > > tst_af_alg.c:74:2: CHECK ERROR: TEST() macro should not be used in library > > tst_af_alg.c:109:2: CHECK ERROR: TEST() macro should not be used in library > > tst_af_alg.c:119:2: CHECK ERROR: TEST() macro should not be used in library > > make: *** [../include/mk/rules.mk:46: check-tst_af_alg] Error 1 > > > > Similarly what I added to my patchset which also adds new make target: > > https://patchwork.ozlabs.org/project/ltp/patch/20210603183827.24339-2-pvorel@suse.cz/ > > Although my code has duplicate issue: > > ../include/mk/generic_trunk_target.inc:105: warning: overriding recipe for target 'check-c' > > ../include/mk/generic_leaf_target.inc:110: warning: ignoring old recipe for target 'check-c' > > ../include/mk/generic_trunk_target.inc:105: warning: overriding recipe for target 'check-shell' > > ../include/mk/generic_leaf_target.inc:118: warning: ignoring old recipe for target 'check-shell' > > > > Also make check on regular test expect it's a library. IMHO these two must be > > probably separated: > > > > $ cd testcases/kernel/syscalls/fchown/ && make check > > CHECK testcases/kernel/syscalls/fchown/fchown01.c > > CHECK testcases/kernel/syscalls/fchown/fchown02.c > > CHECK testcases/kernel/syscalls/fchown/fchown03.c > > CHECK testcases/kernel/syscalls/fchown/fchown04.c > > fchown05.c:80:4: CHECK ERROR: TEST() macro should not be used in library > > make: *** [../../../../include/mk/rules.mk:46: check-fchown05] Error 1 > > It fails because it is using the old test API, so there is no struct > tst_test test var. I don't think we need to separate the checker. It can > detect what type of file (translation unit) it is processing. In the > case of old API tests, I would simply disable any checks. > > All the old tests and test libraries will have something in common. For > the tests, they all import "test.h" (even if through another header), so > we can identify them by that. For the top level library we pass -DLTPLIB in CFLAGS, not sure if that could be used to identify library code... -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system 2021-06-04 11:14 ` [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system Richard Palethorpe 2021-06-04 14:10 ` Petr Vorel @ 2021-06-07 9:18 ` Joerg Vehlow 2021-06-07 9:12 ` Cyril Hrubis 2021-06-11 13:49 ` Petr Vorel 2 siblings, 1 reply; 20+ messages in thread From: Joerg Vehlow @ 2021-06-07 9:18 UTC (permalink / raw) To: ltp Hi, On 6/4/2021 1:14 PM, Richard Palethorpe via ltp wrote: > diff --git a/tools/clang-check/Makefile b/tools/clang-check/Makefile > new file mode 100644 > index 000000000..4650d6057 > --- /dev/null > +++ b/tools/clang-check/Makefile > @@ -0,0 +1,14 @@ > +# SPDX-License-Identifier: GPL-2.0-or-later > +# Copyright (c) 2021 SUSE LLC <rpalethorpe@suse.com> > +# Copyright (c) 2019 Cyril Hrubis <chrubis@suse.cz> > +# Copyright (c) 2020 Petr Vorel <pvorel@suse.cz> > + > +top_srcdir ?= ../.. > + > +include $(top_srcdir)/include/mk/env_pre.mk > +include $(top_srcdir)/include/mk/functions.mk > + > +HOST_MAKE_TARGETS := main > +HOST_LDFLAGS += -lclang If anyone else trying this has problems linking the tool, at least for me, this must be HOST_LDLIBS ??? ??? += -lclang Otherwise the link order is wrong (main.c after -lclang) and all symbols from libclang are undefined. J?rg ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system 2021-06-07 9:18 ` Joerg Vehlow @ 2021-06-07 9:12 ` Cyril Hrubis 0 siblings, 0 replies; 20+ messages in thread From: Cyril Hrubis @ 2021-06-07 9:12 UTC (permalink / raw) To: ltp Hi! > > +include $(top_srcdir)/include/mk/env_pre.mk > > +include $(top_srcdir)/include/mk/functions.mk > > + > > +HOST_MAKE_TARGETS := main > > +HOST_LDFLAGS += -lclang > If anyone else trying this has problems linking the tool, at least for > me, this must be > HOST_LDLIBS ?????? ?????? += -lclang > > Otherwise the link order is wrong (main.c after -lclang) and all symbols > from libclang are undefined. Thx for report, indeed that has to be fixed. -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system 2021-06-04 11:14 ` [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system Richard Palethorpe 2021-06-04 14:10 ` Petr Vorel 2021-06-07 9:18 ` Joerg Vehlow @ 2021-06-11 13:49 ` Petr Vorel 2021-06-11 14:17 ` Petr Vorel 2 siblings, 1 reply; 20+ messages in thread From: Petr Vorel @ 2021-06-11 13:49 UTC (permalink / raw) To: ltp Hi Richie, > Allows the user to run 'make check' to check all source files or > 'make check-<target>' to check one source file corresponding to a > target. > Adds makefile pieces for tools/clang-check/main which will be a > libclang based tool. By default this is ran by 'make check'. > In theory allows other tools to be specified with > 'make CHECK=tool CHECK_FLAGS=<args> check...'. e.g. 'make CHECK=sparse > CHECK_FLAGS= check-tst_cgroup' one more proposal (addition to Metan's fix [1]): how about to add top level make check target: diff --git Makefile Makefile index 56812d77b..b65315618 100644 --- Makefile +++ Makefile @@ -79,6 +79,7 @@ BOOTSTRAP_TARGETS := $(sort $(COMMON_TARGETS) $(CLEAN_TARGETS) $(INSTALL_TARGETS CLEAN_TARGETS := $(addsuffix -clean,$(CLEAN_TARGETS)) INSTALL_TARGETS := $(addsuffix -install,$(INSTALL_TARGETS)) MAKE_TARGETS := $(addsuffix -all,$(filter-out lib,$(COMMON_TARGETS))) +CHECK_TARGETS := $(addsuffix -check,testcases lib) # There's no reason why we should run `all' twice. Otherwise we're just wasting # 3+ mins of useful CPU cycles on a modern machine, and even more time on an @@ -99,6 +100,11 @@ INSTALL_DIR := $(abspath $(INSTALL_DIR)) $(sort $(addprefix $(abs_top_builddir)/,$(BOOTSTRAP_TARGETS)) $(INSTALL_DIR) $(DESTDIR)/$(bindir)): mkdir -m 00755 -p "$@" +$(CHECK_TARGETS): + echo "CHECK_TARGETS: $(CHECK_TARGETS)"; \ + $(MAKE) -C "$(subst -check,,$@)" \ + -f "$(abs_top_srcdir)/$(subst -check,,$@)/Makefile" all + ## Pattern based subtarget rules. lib-install: lib-all @@ -189,6 +195,9 @@ INSTALL_TARGETS += $(addprefix $(DESTDIR)/$(bindir)/,$(BINDIR_INSTALL_SCRIPTS)) $(INSTALL_TARGETS): $(INSTALL_DIR) $(DESTDIR)/$(bindir) +## Check +check: $(CHECK_TARGETS) + ## Install install: $(INSTALL_TARGETS) --- Kind regards, Petr [1] https://lists.linux.it/pipermail/ltp/2021-June/023017.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system 2021-06-11 13:49 ` Petr Vorel @ 2021-06-11 14:17 ` Petr Vorel 2021-06-14 11:09 ` Richard Palethorpe 0 siblings, 1 reply; 20+ messages in thread From: Petr Vorel @ 2021-06-11 14:17 UTC (permalink / raw) To: ltp Hi Richie, > Hi Richie, > one more proposal (addition to Metan's fix [1]): > how about to add top level make check target: > diff --git Makefile Makefile > index 56812d77b..b65315618 100644 > --- Makefile > +++ Makefile > @@ -79,6 +79,7 @@ BOOTSTRAP_TARGETS := $(sort $(COMMON_TARGETS) $(CLEAN_TARGETS) $(INSTALL_TARGETS > CLEAN_TARGETS := $(addsuffix -clean,$(CLEAN_TARGETS)) > INSTALL_TARGETS := $(addsuffix -install,$(INSTALL_TARGETS)) > MAKE_TARGETS := $(addsuffix -all,$(filter-out lib,$(COMMON_TARGETS))) > +CHECK_TARGETS := $(addsuffix -check,testcases lib) > # There's no reason why we should run `all' twice. Otherwise we're just wasting > # 3+ mins of useful CPU cycles on a modern machine, and even more time on an > @@ -99,6 +100,11 @@ INSTALL_DIR := $(abspath $(INSTALL_DIR)) > $(sort $(addprefix $(abs_top_builddir)/,$(BOOTSTRAP_TARGETS)) $(INSTALL_DIR) $(DESTDIR)/$(bindir)): > mkdir -m 00755 -p "$@" > +$(CHECK_TARGETS): > + echo "CHECK_TARGETS: $(CHECK_TARGETS)"; \ This should be obviously left out (my debug message). > + $(MAKE) -C "$(subst -check,,$@)" \ > + -f "$(abs_top_srcdir)/$(subst -check,,$@)/Makefile" all This should be check target -f "$(abs_top_srcdir)/$(subst -check,,$@)/Makefile" check > + > ## Pattern based subtarget rules. > lib-install: lib-all > @@ -189,6 +195,9 @@ INSTALL_TARGETS += $(addprefix $(DESTDIR)/$(bindir)/,$(BINDIR_INSTALL_SCRIPTS)) > $(INSTALL_TARGETS): $(INSTALL_DIR) $(DESTDIR)/$(bindir) > +## Check > +check: $(CHECK_TARGETS) > + > ## Install > install: $(INSTALL_TARGETS) > --- + there needs to be check added to RECURSIVE_TARGETS +++ include/mk/generic_trunk_target.inc @@ -48,7 +48,7 @@ include $(top_srcdir)/include/mk/functions.mk -RECURSIVE_TARGETS ?= all install +RECURSIVE_TARGETS ?= all install check $(eval $(get_make_dirs)) --- Kind regards, Petr ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system 2021-06-11 14:17 ` Petr Vorel @ 2021-06-14 11:09 ` Richard Palethorpe 0 siblings, 0 replies; 20+ messages in thread From: Richard Palethorpe @ 2021-06-14 11:09 UTC (permalink / raw) To: ltp Hello Petr, Petr Vorel <pvorel@suse.cz> writes: > Hi Richie, >> Hi Richie, > >> one more proposal (addition to Metan's fix [1]): >> how about to add top level make check target: > >> diff --git Makefile Makefile >> index 56812d77b..b65315618 100644 >> --- Makefile >> +++ Makefile >> @@ -79,6 +79,7 @@ BOOTSTRAP_TARGETS := $(sort $(COMMON_TARGETS) $(CLEAN_TARGETS) $(INSTALL_TARGETS >> CLEAN_TARGETS := $(addsuffix -clean,$(CLEAN_TARGETS)) >> INSTALL_TARGETS := $(addsuffix -install,$(INSTALL_TARGETS)) >> MAKE_TARGETS := $(addsuffix -all,$(filter-out lib,$(COMMON_TARGETS))) >> +CHECK_TARGETS := $(addsuffix -check,testcases lib) > >> # There's no reason why we should run `all' twice. Otherwise we're just wasting >> # 3+ mins of useful CPU cycles on a modern machine, and even more time on an >> @@ -99,6 +100,11 @@ INSTALL_DIR := $(abspath $(INSTALL_DIR)) >> $(sort $(addprefix $(abs_top_builddir)/,$(BOOTSTRAP_TARGETS)) $(INSTALL_DIR) $(DESTDIR)/$(bindir)): >> mkdir -m 00755 -p "$@" > >> +$(CHECK_TARGETS): >> + echo "CHECK_TARGETS: $(CHECK_TARGETS)"; \ > This should be obviously left out (my debug message). >> + $(MAKE) -C "$(subst -check,,$@)" \ >> + -f "$(abs_top_srcdir)/$(subst -check,,$@)/Makefile" all > This should be check target > -f "$(abs_top_srcdir)/$(subst -check,,$@)/Makefile" check >> + >> ## Pattern based subtarget rules. >> lib-install: lib-all > >> @@ -189,6 +195,9 @@ INSTALL_TARGETS += $(addprefix $(DESTDIR)/$(bindir)/,$(BINDIR_INSTALL_SCRIPTS)) > >> $(INSTALL_TARGETS): $(INSTALL_DIR) $(DESTDIR)/$(bindir) > >> +## Check >> +check: $(CHECK_TARGETS) >> + >> ## Install >> install: $(INSTALL_TARGETS) > >> --- > > + there needs to be check added to RECURSIVE_TARGETS > +++ include/mk/generic_trunk_target.inc > @@ -48,7 +48,7 @@ > > include $(top_srcdir)/include/mk/functions.mk > > -RECURSIVE_TARGETS ?= all install > +RECURSIVE_TARGETS ?= all install check > > $(eval $(get_make_dirs)) > > --- > > Kind regards, > Petr Thanks. It appears defining trunk-check (similar to check-all) and "check: trunk-check" in addition to adding check to the recursive targets seems to work. -- Thank you, Richard. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 2/2] Start libclang based analyzer and TEST() check 2021-06-04 11:14 [LTP] [RFC PATCH v2 0/2] Libclang based analyzer Richard Palethorpe 2021-06-04 11:14 ` [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system Richard Palethorpe @ 2021-06-04 11:14 ` Richard Palethorpe 2021-06-04 12:52 ` [LTP] [RFC PATCH v2 0/2] Libclang based analyzer Cyril Hrubis 2021-06-07 8:37 ` Joerg Vehlow 3 siblings, 0 replies; 20+ messages in thread From: Richard Palethorpe @ 2021-06-04 11:14 UTC (permalink / raw) To: ltp This uses the stable Clang C API to find usages of the TEST() macro. It can also determine if a translation unit is a test executable by finding the struct tst_test test instantiation. This Clang API only exposes the AST along with some other utilities for evaluating constants, indexing, auto completion and source rewriting. This is somewhat less than what Smatch, Coccinelle and the unstable Clang C++ APIs expose. However it is a simple, stable and well supported C API. Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> --- tools/clang-check/main.c | 239 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 239 insertions(+) create mode 100644 tools/clang-check/main.c diff --git a/tools/clang-check/main.c b/tools/clang-check/main.c new file mode 100644 index 000000000..26ce898c3 --- /dev/null +++ b/tools/clang-check/main.c @@ -0,0 +1,239 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2021 SUSE LLC <rpalethorpe@suse.com> + * Copyright (c) 2017 Petr Vorel <pvorel@suse.cz> + * + * Entry point for the LTP static analyser. + * + * Scans the AST (Abstract Syntax Tree) generated by Clang + * twice. First pass we just collect info about the TU (Translation + * Unit). Second pass performs the checks. + * + * AST Nodes are called CXCursor by libclang. We use the library's + * visitor functions to recurse into the AST. + * + * The kind of AST node decides which checks to run on it. + * + * This program takes the same arguments the Clang compiler + * frontend does. Although some are ignored by libclang. + */ +#include <unistd.h> +#include <string.h> +#include <stdlib.h> +#include <stdio.h> +#include <clang-c/Index.h> + +#define attr_unused __attribute__((unused)) + +/* The rules for test, library and tool code are different */ +enum ltp_tu_kind { + LTP_TEST, + LTP_OTHER, +}; + +/* Holds information about the TU which we gathered on the first pass */ +static struct { + enum ltp_tu_kind tu_kind; +} tu_info; + +static const char *const ansi_red = "\033[1;31m"; +static const char *const ansi_reset = "\033[0m"; +static const char *const ansi_bold = "\033[1m"; + +static unsigned error_flag; + +/* Copied from lib/tst_ansi_color.c */ +static int color_enabled(const int fd) +{ + static int color; + + if (color) + return color - 1; + + const char *const env = getenv("LTP_COLORIZE_OUTPUT"); + + if (env) { + if (!strcmp(env, "n") || !strcmp(env, "0")) + color = 1; + + if (!strcmp(env, "y") || !strcmp(env, "1")) + color = 2; + + return color - 1; + } + + if (isatty(fd) == 0) + color = 1; + else + color = 2; + + return color - 1; +} + +static void emit_error(CXCursor offending_cursor, const char *const error_msg) +{ + CXSourceLocation loc = clang_getCursorLocation(offending_cursor); + CXFile loc_file; + unsigned loc_line, loc_column; + CXString file_name; + + error_flag = 1; + + clang_getFileLocation(loc, &loc_file, &loc_line, &loc_column, + /*offset=*/NULL); + file_name = clang_getFileName(loc_file); + + if (color_enabled(STDERR_FILENO)) { + dprintf(STDERR_FILENO, + "%s:%u:%u: %sCHECK ERROR%s: %s%s%s\n", + clang_getCString(file_name), loc_line, loc_column, + ansi_red, ansi_reset, + ansi_bold, error_msg, ansi_reset); + } else { + dprintf(STDERR_FILENO, + "%s:%u:%u: CHECK ERROR: %s\n", + clang_getCString(file_name), loc_line, loc_column, + error_msg); + } + + clang_disposeString(file_name); +} + +static int cursor_cmp_spelling(const char *const spelling, CXCursor cursor) +{ + CXString cursor_spelling = clang_getCursorSpelling(cursor); + const int ret = strcmp(spelling, clang_getCString(cursor_spelling)); + + clang_disposeString(cursor_spelling); + + return ret; +} + +static int cursor_type_cmp_spelling(const char *const spelling, CXCursor cursor) +{ + CXType ctype = clang_getCursorType(cursor); + CXString ctype_spelling = clang_getTypeSpelling(ctype); + const int ret = strcmp(spelling, clang_getCString(ctype_spelling)); + + clang_disposeString(ctype_spelling); + + return ret; +} + +/* + * Check if the TEST() macro is used inside the library. + * + * This check takes an AST node which should already be known to be a + * macro expansion kind. + * + * If the TU appears to be a test executable then the test does not + * apply. So in that case we return. + * + * If the macro expansion AST node is spelled TEST, then we emit an + * error. Otherwise do nothing. + */ +static void check_TEST_macro(CXCursor macro_cursor) +{ + if (tu_info.tu_kind == LTP_TEST) + return; + + if (!cursor_cmp_spelling("TEST", macro_cursor)) { + emit_error(macro_cursor, + "TEST() macro should not be used in library"); + } +} + +/* Second pass where we run the checks */ +static enum CXChildVisitResult check_visitor(CXCursor cursor, + attr_unused CXCursor parent, + attr_unused CXClientData client_data) +{ + CXSourceLocation loc = clang_getCursorLocation(cursor); + + if (clang_Location_isInSystemHeader(loc)) + return CXChildVisit_Continue; + + switch (clang_getCursorKind(cursor)) { + case CXCursor_MacroExpansion: + check_TEST_macro(cursor); + break; + default: + break; + } + + return CXChildVisit_Recurse; +} + +/* If we find `struct tst_test = {...}` then record that this TU is a test */ +static void info_ltp_tu_kind(CXCursor cursor) +{ + CXCursor initializer; + + if (clang_Cursor_hasVarDeclGlobalStorage(cursor) != 1) + return; + + if (cursor_cmp_spelling("test", cursor)) + return; + + if (cursor_type_cmp_spelling("struct tst_test", cursor)) + return; + + initializer = clang_Cursor_getVarDeclInitializer(cursor); + + if (!clang_Cursor_isNull(initializer)) + tu_info.tu_kind = LTP_TEST; +} + +/* First pass to collect info */ +static enum CXChildVisitResult info_visitor(CXCursor cursor, + attr_unused CXCursor parent, + attr_unused CXClientData client_data) +{ + CXSourceLocation loc = clang_getCursorLocation(cursor); + + if (clang_Location_isInSystemHeader(loc)) + return CXChildVisit_Continue; + + switch (clang_getCursorKind(cursor)) { + case CXCursor_VarDecl: + info_ltp_tu_kind(cursor); + break; + default: + break; + } + + return CXChildVisit_Continue; +} + +int main(const int argc, const char *const *const argv) +{ + CXIndex cindex = clang_createIndex(0, 1); + CXTranslationUnit tu; + CXCursor tuc; + + enum CXErrorCode ret = clang_parseTranslationUnit2( + cindex, + /*source_filename=*/NULL, + argv + 1, argc - 1, + /*unsaved_files=*/NULL, /*num_unsaved_files=*/0, + CXTranslationUnit_DetailedPreprocessingRecord, + &tu); + + if (ret != CXError_Success) { + printf("Failed to load translation unit: %d\n", ret); + return 1; + } + + tuc = clang_getTranslationUnitCursor(tu); + + tu_info.tu_kind = LTP_OTHER; + clang_visitChildren(tuc, info_visitor, NULL); + + clang_visitChildren(tuc, check_visitor, NULL); + + /* Stop leak sanitizer from complaining */ + clang_disposeTranslationUnit(tu); + clang_disposeIndex(cindex); + + return error_flag; +} -- 2.31.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 0/2] Libclang based analyzer 2021-06-04 11:14 [LTP] [RFC PATCH v2 0/2] Libclang based analyzer Richard Palethorpe 2021-06-04 11:14 ` [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system Richard Palethorpe 2021-06-04 11:14 ` [LTP] [RFC PATCH v2 2/2] Start libclang based analyzer and TEST() check Richard Palethorpe @ 2021-06-04 12:52 ` Cyril Hrubis 2021-06-04 13:50 ` Richard Palethorpe 2021-06-07 8:37 ` Joerg Vehlow 3 siblings, 1 reply; 20+ messages in thread From: Cyril Hrubis @ 2021-06-04 12:52 UTC (permalink / raw) To: ltp Hi! > Compared to the Coccinelle version it's very ugly. However I think > this will allow us to get all the low hanging fruit without creating > major problems for test developers. I had a look at the code and it does not seem to be that bad. It's longer due to some boiler plate and more explicit but not necessarily ugly. > I guess it could be run during CI if we either fix all the existing > TEST() usages in the library or add an ignore list. I already have a > Coccinelle script to help with the former. I will have a look at the patches generated by coccinelle, I supposed that we want to merge them regardless. -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 0/2] Libclang based analyzer 2021-06-04 12:52 ` [LTP] [RFC PATCH v2 0/2] Libclang based analyzer Cyril Hrubis @ 2021-06-04 13:50 ` Richard Palethorpe 0 siblings, 0 replies; 20+ messages in thread From: Richard Palethorpe @ 2021-06-04 13:50 UTC (permalink / raw) To: ltp Hello Cyril, Cyril Hrubis <chrubis@suse.cz> writes: > Hi! >> Compared to the Coccinelle version it's very ugly. However I think >> this will allow us to get all the low hanging fruit without creating >> major problems for test developers. > > I had a look at the code and it does not seem to be that bad. It's > longer due to some boiler plate and more explicit but not necessarily > ugly. I agree, but "semantic patches" are a really elgant way of matching code compared to scanning the AST with some ad-hoc logic for matching. > >> I guess it could be run during CI if we either fix all the existing >> TEST() usages in the library or add an ignore list. I already have a >> Coccinelle script to help with the former. > > I will have a look at the patches generated by coccinelle, I supposed > that we want to merge them regardless. OK, I just sent in a patch for the CGroups part. -- Thank you, Richard. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 0/2] Libclang based analyzer 2021-06-04 11:14 [LTP] [RFC PATCH v2 0/2] Libclang based analyzer Richard Palethorpe ` (2 preceding siblings ...) 2021-06-04 12:52 ` [LTP] [RFC PATCH v2 0/2] Libclang based analyzer Cyril Hrubis @ 2021-06-07 8:37 ` Joerg Vehlow 2021-06-07 9:14 ` Cyril Hrubis 2021-06-07 10:20 ` Richard Palethorpe 3 siblings, 2 replies; 20+ messages in thread From: Joerg Vehlow @ 2021-06-07 8:37 UTC (permalink / raw) To: ltp Hi, just one quick remark. I guess the whole reason for using clang over coccinelle was availability of clang on developer systems. I just wanted to quickly check your work, but had no clang installed. Build fail, even with cyril's patch for CHECK_TARGETS, because clang-c/Index.h is not found. On ubuntu 20.04, this file is part of libclang-dev, but installing it did not help either, because it is installed to an include path not know to gcc (/usr/lib/llvm-10/include/clang-c). I added it to the include path and it was found, but the next problem is, that some used functions (like clang_Cursor_getVarDeclInitializer) are only available starting with libclang 12. So in conclusion, I do not think we can assume libclang to be available for all developers and installing it is probably more work, at least when newer functions from libclang are used, than installing coccinelle. And very important for final setup: It must be possible to successfully compile ltp, without libclang/coccinelle available. There is no reason to force this libraries/tools for pure "users" of ltp. J?rg ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 0/2] Libclang based analyzer 2021-06-07 8:37 ` Joerg Vehlow @ 2021-06-07 9:14 ` Cyril Hrubis 2021-06-07 10:20 ` Richard Palethorpe 1 sibling, 0 replies; 20+ messages in thread From: Cyril Hrubis @ 2021-06-07 9:14 UTC (permalink / raw) To: ltp Hi! > So in conclusion, I do not think we can assume libclang to be available > for all developers and installing it is probably more work, at least > when newer functions from libclang are used, than installing coccinelle. > And very important for final setup: It must be possible to successfully > compile ltp, without libclang/coccinelle available. There is no reason > to force this libraries/tools for pure "users" of ltp. Indeed this is supposed to be tooling for developers and strictly optional. If this breaks builds without libclang it has to be fixed. -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 0/2] Libclang based analyzer 2021-06-07 8:37 ` Joerg Vehlow 2021-06-07 9:14 ` Cyril Hrubis @ 2021-06-07 10:20 ` Richard Palethorpe 2021-06-07 11:34 ` Joerg Vehlow 1 sibling, 1 reply; 20+ messages in thread From: Richard Palethorpe @ 2021-06-07 10:20 UTC (permalink / raw) To: ltp Hello Joerg, Joerg Vehlow <lkml@jv-coder.de> writes: > Hi, > > just one quick remark. I guess the whole reason for using clang over > coccinelle was availability of clang on developer systems. > I just wanted to quickly check your work, but had no clang > installed. Build fail, even with cyril's patch for CHECK_TARGETS, > because clang-c/Index.h is not found. Thanks for testing it. > > On ubuntu 20.04, this file is part of libclang-dev, but installing it > did not help either, because it is installed to an include path not > know to gcc (/usr/lib/llvm-10/include/clang-c). Is part of this path the same that 'clang -print-resource-dir' prints? Either way I guess we can search for this during configuration. LLVM has a CMake module (or w/e) which probably finds all this automatically. > I added it to the include path and it was found, but the next problem > is, that some used functions (like clang_Cursor_getVarDeclInitializer) > are only available starting with libclang 12. > I guess that we could replace that function by recursing further into the AST to find the initializer ourselves. Probably we can restrict ourselves to only use functions from before libclang 11. > > So in conclusion, I do not think we can assume libclang to be > available for all developers and installing it is probably more work, > at least when newer functions from libclang are used, than installing > coccinelle. IIRC Cyril said the Coccinelle package on Gentoo is not maintained anymore. AFAICT it exists, but it is on an old version. I don't think many people are interested in or want to maintain Ocaml stuff. LLVM/Clang OTOH looks to be very active. > And very important for final setup: It must be possible to > successfully compile ltp, without libclang/coccinelle available. There > is no reason to force this libraries/tools for pure "users" of ltp. > > J?rg 100% agree. These checks only make sense during development. Even then I want to ensure that everyone can run the checks with very little effort. -- Thank you, Richard. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 0/2] Libclang based analyzer 2021-06-07 10:20 ` Richard Palethorpe @ 2021-06-07 11:34 ` Joerg Vehlow 2021-06-07 13:42 ` Cyril Hrubis 2021-06-07 13:49 ` Richard Palethorpe 0 siblings, 2 replies; 20+ messages in thread From: Joerg Vehlow @ 2021-06-07 11:34 UTC (permalink / raw) To: ltp Hi Richard, On 6/7/2021 12:20 PM, Richard Palethorpe wrote: > Hello Joerg, > > On ubuntu 20.04, this file is part of libclang-dev, but installing it > did not help either, because it is installed to an include path not > know to gcc (/usr/lib/llvm-10/include/clang-c). > Is part of this path the same that 'clang -print-resource-dir' prints? > > Either way I guess we can search for this during configuration. LLVM has > a CMake module (or w/e) which probably finds all this automatically. resource dir is /usr/lib/llvm-12/lib/clang/12.0.1. The llvm-config tool can be used to find the locations of the include and lib directory. On my ubuntu, I installed clang-12 from apt.llvm.org and clang-10 from ubuntu repos. In the path there is the llvm-config tool from ubuntu pointing to /usr/lib/llvm-10/bin/llvm-config and llvm-config-10 and llvm-config-12 pointing to the respective llvm-config tool. I guess using llvm-config from the path to detect the correct include and library path would be the best way to go. If someone wants to use a different version, he can still set prepend it to the path during configuration: $ llvm-config --includedir /usr/lib/llvm-10/include $ llvm-config --libdir /usr/lib/llvm-10/lib $ PATH="/usr/lib/llvm-12/bin:$PATH" llvm-config --includedir /usr/lib/llvm-12/include Both includedir and libdir are required, to correctly link libclang. In the default library search paths, there are only versioned versiones of libclang (eg. libclang-12.so). >> I added it to the include path and it was found, but the next problem >> is, that some used functions (like clang_Cursor_getVarDeclInitializer) >> are only available starting with libclang 12. >> > I guess that we could replace that function by recursing further into > the AST to find the initializer ourselves. > > Probably we can restrict ourselves to only use functions from before > libclang 11. Sounds good, but how to force this? I don't think there is a "allow only libclang 10 symbols"... > >> So in conclusion, I do not think we can assume libclang to be >> available for all developers and installing it is probably more work, >> at least when newer functions from libclang are used, than installing >> coccinelle. > IIRC Cyril said the Coccinelle package on Gentoo is not maintained > anymore. AFAICT it exists, but it is on an old version. I don't think > many people are interested in or want to maintain Ocaml > stuff. LLVM/Clang OTOH looks to be very active. Right, it actually is removed now from gentoo portage tree ([1]). But is it used by the kernel developers? J?rg [1] https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=58395d3a0c06e060a0a40182fff4bf39f1910529 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 0/2] Libclang based analyzer 2021-06-07 11:34 ` Joerg Vehlow @ 2021-06-07 13:42 ` Cyril Hrubis 2021-06-07 13:49 ` Richard Palethorpe 1 sibling, 0 replies; 20+ messages in thread From: Cyril Hrubis @ 2021-06-07 13:42 UTC (permalink / raw) To: ltp Hi! > > IIRC Cyril said the Coccinelle package on Gentoo is not maintained > > anymore. AFAICT it exists, but it is on an old version. I don't think > > many people are interested in or want to maintain Ocaml > > stuff. LLVM/Clang OTOH looks to be very active. > Right, it actually is removed now from gentoo portage tree ([1]). But is > it used by the kernel developers? I guess that some people use it, however the choice of programming language makes everthing much harder. On some distributions it's hard to install and requires manual steps and when you hit a bug it's impossible to debug unless you know how to debug ocaml. I've tried to fix a simple bug in coccinelle once, but gave up after a few hours since I wasn't even able to gasp how the source code is structured. And for the record I used to read and write lisp and haskell just fine during my university days. So even my skills in functional programming are rusty now I did not expect that ocaml would be so alien to me... -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 20+ messages in thread
* [LTP] [RFC PATCH v2 0/2] Libclang based analyzer 2021-06-07 11:34 ` Joerg Vehlow 2021-06-07 13:42 ` Cyril Hrubis @ 2021-06-07 13:49 ` Richard Palethorpe 1 sibling, 0 replies; 20+ messages in thread From: Richard Palethorpe @ 2021-06-07 13:49 UTC (permalink / raw) To: ltp Hello Joerg, Joerg Vehlow <lkml@jv-coder.de> writes: > Hi Richard, > > On 6/7/2021 12:20 PM, Richard Palethorpe wrote: >> Hello Joerg, >> >> On ubuntu 20.04, this file is part of libclang-dev, but installing it >> did not help either, because it is installed to an include path not >> know to gcc (/usr/lib/llvm-10/include/clang-c). >> Is part of this path the same that 'clang -print-resource-dir' prints? >> >> Either way I guess we can search for this during configuration. LLVM has >> a CMake module (or w/e) which probably finds all this automatically. > resource dir is /usr/lib/llvm-12/lib/clang/12.0.1. > > The llvm-config tool can be used to find the locations of the include > and lib directory. > On my ubuntu, I installed clang-12 from apt.llvm.org and clang-10 from > ubuntu repos. > In the path there is the llvm-config tool from ubuntu pointing to > /usr/lib/llvm-10/bin/llvm-config and llvm-config-10 and llvm-config-12 > pointing to the respective llvm-config tool. > > I guess using llvm-config from the path to detect the correct include > and library path would be the best way to go. > If someone wants to use a different version, he can still set prepend > it to the path during configuration: > > $ llvm-config --includedir > /usr/lib/llvm-10/include > > $ llvm-config --libdir > /usr/lib/llvm-10/lib > > $ PATH="/usr/lib/llvm-12/bin:$PATH" llvm-config --includedir > /usr/lib/llvm-12/include > > > Both includedir and libdir are required, to correctly link > libclang. In the default library search paths, there are only > versioned versiones of libclang (eg. libclang-12.so). OK. > >>> I added it to the include path and it was found, but the next problem >>> is, that some used functions (like clang_Cursor_getVarDeclInitializer) >>> are only available starting with libclang 12. >>> >> I guess that we could replace that function by recursing further into >> the AST to find the initializer ourselves. >> >> Probably we can restrict ourselves to only use functions from before >> libclang 11. > Sounds good, but how to force this? I don't think there is a "allow > only libclang 10 symbols"... Make a list of the symbols exported by libclang 10. Then check that anything which starts with clang_ or CX is in that list. Or just compile it against libclang 10 in CI. >> >>> So in conclusion, I do not think we can assume libclang to be >>> available for all developers and installing it is probably more work, >>> at least when newer functions from libclang are used, than installing >>> coccinelle. >> IIRC Cyril said the Coccinelle package on Gentoo is not maintained >> anymore. AFAICT it exists, but it is on an old version. I don't think >> many people are interested in or want to maintain Ocaml >> stuff. LLVM/Clang OTOH looks to be very active. > Right, it actually is removed now from gentoo portage tree ([1]). But > is it used by the kernel developers? Some kernel developers use it. There are a number of checkers and some maintainers care about them while others do not. > > J?rg > > [1] > https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=58395d3a0c06e060a0a40182fff4bf39f1910529 -- Thank you, Richard. ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2021-06-14 11:09 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-04 11:14 [LTP] [RFC PATCH v2 0/2] Libclang based analyzer Richard Palethorpe 2021-06-04 11:14 ` [LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system Richard Palethorpe 2021-06-04 14:10 ` Petr Vorel 2021-06-04 14:11 ` Cyril Hrubis 2021-06-04 14:28 ` Richard Palethorpe 2021-06-04 14:33 ` Cyril Hrubis 2021-06-07 9:18 ` Joerg Vehlow 2021-06-07 9:12 ` Cyril Hrubis 2021-06-11 13:49 ` Petr Vorel 2021-06-11 14:17 ` Petr Vorel 2021-06-14 11:09 ` Richard Palethorpe 2021-06-04 11:14 ` [LTP] [RFC PATCH v2 2/2] Start libclang based analyzer and TEST() check Richard Palethorpe 2021-06-04 12:52 ` [LTP] [RFC PATCH v2 0/2] Libclang based analyzer Cyril Hrubis 2021-06-04 13:50 ` Richard Palethorpe 2021-06-07 8:37 ` Joerg Vehlow 2021-06-07 9:14 ` Cyril Hrubis 2021-06-07 10:20 ` Richard Palethorpe 2021-06-07 11:34 ` Joerg Vehlow 2021-06-07 13:42 ` Cyril Hrubis 2021-06-07 13:49 ` Richard Palethorpe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.