[LTP] [RFC PATCH v2 0/2] Libclang based analyzer

[LTP] [RFC PATCH v2 0/2] Libclang based analyzer

[Richard Palethorpe]

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

[LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system

[Richard Palethorpe]

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

[LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system

[Petr Vorel]

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

[LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system

[Cyril Hrubis]

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

[LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system

[Richard Palethorpe]

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.

[LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system

[Cyril Hrubis]

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

[LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system

[Joerg Vehlow]

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

[LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system

[Cyril Hrubis]

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

[LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system

[Petr Vorel]

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

[LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system

[Petr Vorel]

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

[LTP] [RFC PATCH v2 1/2] Add 'make check' and clang-check to build system

[Richard Palethorpe]

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.

[LTP] [RFC PATCH v2 2/2] Start libclang based analyzer and TEST() check

[Richard Palethorpe]

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

[LTP] [RFC PATCH v2 0/2] Libclang based analyzer

[Cyril Hrubis]

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

[LTP] [RFC PATCH v2 0/2] Libclang based analyzer

[Richard Palethorpe]

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.

[LTP] [RFC PATCH v2 0/2] Libclang based analyzer

[Joerg Vehlow]

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

[LTP] [RFC PATCH v2 0/2] Libclang based analyzer

[Cyril Hrubis]

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

[LTP] [RFC PATCH v2 0/2] Libclang based analyzer

[Richard Palethorpe]

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.

[LTP] [RFC PATCH v2 0/2] Libclang based analyzer

[Joerg Vehlow]

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

[LTP] [RFC PATCH v2 0/2] Libclang based analyzer

[Cyril Hrubis]

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

[LTP] [RFC PATCH v2 0/2] Libclang based analyzer

[Richard Palethorpe]

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.