* [LTP] [RFC PATCH 0/2] Libclang based analyzer
@ 2021-06-03 15:48 Richard Palethorpe
2021-06-03 15:48 ` [LTP] [RFC PATCH 1/2] Add 'make checks' and clang-checks to build system Richard Palethorpe
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Richard Palethorpe @ 2021-06-03 15:48 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.
Richard Palethorpe (2):
Add 'make checks' and clang-checks to build system
Start libclang based analyzer and TEST() check
configure.ac | 2 +
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-checks/.gitignore | 1 +
tools/clang-checks/Makefile | 13 ++
tools/clang-checks/main.c | 218 +++++++++++++++++++++++++++++
10 files changed, 264 insertions(+), 1 deletion(-)
create mode 100644 tools/clang-checks/.gitignore
create mode 100644 tools/clang-checks/Makefile
create mode 100644 tools/clang-checks/main.c
--
2.31.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [LTP] [RFC PATCH 1/2] Add 'make checks' and clang-checks to build system 2021-06-03 15:48 [LTP] [RFC PATCH 0/2] Libclang based analyzer Richard Palethorpe @ 2021-06-03 15:48 ` Richard Palethorpe 2021-06-04 6:06 ` Petr Vorel 2021-06-03 15:48 ` [LTP] [RFC PATCH 2/2] Start libclang based analyzer and TEST() check Richard Palethorpe 2021-06-04 6:20 ` [LTP] [RFC PATCH 0/2] Libclang based analyzer Petr Vorel 2 siblings, 1 reply; 12+ messages in thread From: Richard Palethorpe @ 2021-06-03 15:48 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-checks/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' --- configure.ac | 2 ++ 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-checks/.gitignore | 1 + tools/clang-checks/Makefile | 13 +++++++++++++ 9 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 tools/clang-checks/.gitignore create mode 100644 tools/clang-checks/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/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..13f1104ed 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-checks/main +CHECK_FLAGS ?= -resource-dir $(shell $(CLANG) -print-resource-dir) + +ifeq ($(dir $(CHECK)),$(abs_top_srcdir)/tools/clang-checks/) +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..f99e63acd 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-checks.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..32b3b0f84 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-checks.mk APICMDS_DIR := $(abs_top_builddir)/tools/apicmds diff --git a/tools/clang-checks/.gitignore b/tools/clang-checks/.gitignore new file mode 100644 index 000000000..ba2906d06 --- /dev/null +++ b/tools/clang-checks/.gitignore @@ -0,0 +1 @@ +main diff --git a/tools/clang-checks/Makefile b/tools/clang-checks/Makefile new file mode 100644 index 000000000..f132ba527 --- /dev/null +++ b/tools/clang-checks/Makefile @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +# 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] 12+ messages in thread
* [LTP] [RFC PATCH 1/2] Add 'make checks' and clang-checks to build system 2021-06-03 15:48 ` [LTP] [RFC PATCH 1/2] Add 'make checks' and clang-checks to build system Richard Palethorpe @ 2021-06-04 6:06 ` Petr Vorel 2021-06-04 6:12 ` Petr Vorel 0 siblings, 1 reply; 12+ messages in thread From: Petr Vorel @ 2021-06-04 6:06 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-checks/main which will be a > libclang based tool. By default this is ran by 'make check'. Compilation does not work from tools directory: $ cd tools/ && make ../include/mk/testcases.mk:25: ../include/mk/clang-checks.mk: No such file or directory make: *** No rule to make target '../include/mk/clang-checks.mk'. Stop. (works from tools/clang-checks/) But even with compiled tools/clang-checks/main I'm not able to find how this is supposed to be run, none of these work for me, what am I missing? $ make autotools && ./configure $ make check make: *** No rule to make target 'check'. Stop. $ make check-clang make: *** No rule to make target 'check-clang'. Stop. $ cd lib; make check ../include/mk/lib.mk:29: ../include/mk/clang-checks.mk: No such file or directory make: *** No rule to make target '../include/mk/clang-checks.mk'. Stop. $ cd ../testcases/kernel/syscalls/fchown/ make check ../../../../include/mk/testcases.mk:25: ../../../../include/mk/clang-checks.mk: No such file or directory make: *** No rule to make target '../../../../include/mk/clang-checks.mk'. Stop. $ tools/clang-checks/main Failed to load translation unit: 4 => maybe print some help info when running without parameters? Kind regards, Petr > 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' ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [RFC PATCH 1/2] Add 'make checks' and clang-checks to build system 2021-06-04 6:06 ` Petr Vorel @ 2021-06-04 6:12 ` Petr Vorel 2021-06-04 8:42 ` Richard Palethorpe 0 siblings, 1 reply; 12+ messages in thread From: Petr Vorel @ 2021-06-04 6:12 UTC (permalink / raw) To: ltp Hi Richie, > 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-checks/main which will be a > > libclang based tool. By default this is ran by 'make check'. > Compilation does not work from tools directory: > $ cd tools/ && make > ../include/mk/testcases.mk:25: ../include/mk/clang-checks.mk: No such file or directory > make: *** No rule to make target '../include/mk/clang-checks.mk'. Stop. I guess you forget to add git add include/mk/clang-checks.mk, right? Kind regards, Petr ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [RFC PATCH 1/2] Add 'make checks' and clang-checks to build system 2021-06-04 6:12 ` Petr Vorel @ 2021-06-04 8:42 ` Richard Palethorpe 2021-06-04 8:55 ` Petr Vorel 0 siblings, 1 reply; 12+ messages in thread From: Richard Palethorpe @ 2021-06-04 8:42 UTC (permalink / raw) To: ltp Hello, Petr Vorel <pvorel@suse.cz> writes: > Hi Richie, > >> 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-checks/main which will be a >> > libclang based tool. By default this is ran by 'make check'. > >> Compilation does not work from tools directory: > >> $ cd tools/ && make >> ../include/mk/testcases.mk:25: ../include/mk/clang-checks.mk: No such file or directory >> make: *** No rule to make target '../include/mk/clang-checks.mk'. Stop. > > I guess you forget to add git add include/mk/clang-checks.mk, right? Argg, yes, sorry for wasting your time. This is my fault for not cleaning up my LTP directory. > > Kind regards, > Petr -- Thank you, Richard. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [RFC PATCH 1/2] Add 'make checks' and clang-checks to build system 2021-06-04 8:42 ` Richard Palethorpe @ 2021-06-04 8:55 ` Petr Vorel 0 siblings, 0 replies; 12+ messages in thread From: Petr Vorel @ 2021-06-04 8:55 UTC (permalink / raw) To: ltp Hi Richie, > > I guess you forget to add git add include/mk/clang-checks.mk, right? > Argg, yes, sorry for wasting your time. This is my fault for not > cleaning up my LTP directory. NP, looking forward to v2. Kind regards, Petr ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [RFC PATCH 2/2] Start libclang based analyzer and TEST() check 2021-06-03 15:48 [LTP] [RFC PATCH 0/2] Libclang based analyzer Richard Palethorpe 2021-06-03 15:48 ` [LTP] [RFC PATCH 1/2] Add 'make checks' and clang-checks to build system Richard Palethorpe @ 2021-06-03 15:48 ` Richard Palethorpe 2021-06-04 6:20 ` [LTP] [RFC PATCH 0/2] Libclang based analyzer Petr Vorel 2 siblings, 0 replies; 12+ messages in thread From: Richard Palethorpe @ 2021-06-03 15:48 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. --- tools/clang-checks/main.c | 218 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 218 insertions(+) create mode 100644 tools/clang-checks/main.c diff --git a/tools/clang-checks/main.c b/tools/clang-checks/main.c new file mode 100644 index 000000000..22df30b35 --- /dev/null +++ b/tools/clang-checks/main.c @@ -0,0 +1,218 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2021 SUSE LLC <rpalethorpe@suse.com> + * + * Entry point for the LTP static analyser. + * + * Scans the AST generated by Clang twice. First pass we just collect + * info about the TU (Translation Unit). Second pass performs the + * checks. + * + * This program takes the same arguments the Clang compiler does. + */ +#include <unistd.h> +#include <string.h> +#include <stdlib.h> +#include <stdio.h> +#include <clang-c/Index.h> + +#define attr_unused __attribute__((unused)) + +enum ltp_tu_kind { + LTP_TEST, + LTP_OTHER, +}; + +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; + +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 */ +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] 12+ messages in thread
* [LTP] [RFC PATCH 0/2] Libclang based analyzer 2021-06-03 15:48 [LTP] [RFC PATCH 0/2] Libclang based analyzer Richard Palethorpe 2021-06-03 15:48 ` [LTP] [RFC PATCH 1/2] Add 'make checks' and clang-checks to build system Richard Palethorpe 2021-06-03 15:48 ` [LTP] [RFC PATCH 2/2] Start libclang based analyzer and TEST() check Richard Palethorpe @ 2021-06-04 6:20 ` Petr Vorel 2021-06-04 9:03 ` Richard Palethorpe 2 siblings, 1 reply; 12+ messages in thread From: Petr Vorel @ 2021-06-04 6:20 UTC (permalink / raw) To: ltp Hi Richie, > 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. +1. FYI yesterday I sent patch to add make check{,-c,shell}, but for running C/shell API tests: https://patchwork.ozlabs.org/project/ltp/list/?series=247103 Taking 2 latests commits + adding clang-devel / libclang-dev packages makes CI running. https://github.com/pevik/ltp/commit/b2427f39ddb15c97761208a605637e0da6fe66ca The only missing piece is include/mk/clang-checks.mk (this patchset not even compile now). > Richard Palethorpe (2): > Add 'make checks' and clang-checks to build system make check ... clang-check (to avoid confusion). > Start libclang based analyzer and TEST() check > configure.ac | 2 + > 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-checks/.gitignore | 1 + > tools/clang-checks/Makefile | 13 ++ > tools/clang-checks/main.c | 218 +++++++++++++++++++++++++++++ I'd name it tools/clang-check/ (and include/mk/clang-check.mk), but that's just personal opinion. Kind regards, Petr > 10 files changed, 264 insertions(+), 1 deletion(-) > create mode 100644 tools/clang-checks/.gitignore > create mode 100644 tools/clang-checks/Makefile > create mode 100644 tools/clang-checks/main.c ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [RFC PATCH 0/2] Libclang based analyzer 2021-06-04 6:20 ` [LTP] [RFC PATCH 0/2] Libclang based analyzer Petr Vorel @ 2021-06-04 9:03 ` Richard Palethorpe 2021-06-04 9:15 ` Petr Vorel 0 siblings, 1 reply; 12+ messages in thread From: Richard Palethorpe @ 2021-06-04 9:03 UTC (permalink / raw) To: ltp Hello Petr, Petr Vorel <pvorel@suse.cz> writes: > Hi Richie, > >> 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. > +1. > > FYI yesterday I sent patch to add make check{,-c,shell}, but for running C/shell API tests: > https://patchwork.ozlabs.org/project/ltp/list/?series=247103 So I guess we have a name collision here. Something to consider is that GNU Make defines 'checks' as running self tests. So perhaps you are correct to use that word. I could rename the static analyses to 'analyze' or 'lint'. OTOH it might be better if running the self tests is called 'make run-{c,shell}-api-tests', because only a few people need to do that. Whereas it is intended that all contributors run the static analyses checks. Although, if someone runs 'make check' in the lib directory, then it makes sense to run the C API tests as well as do the analyses. Or not? > > Taking 2 latests commits + adding clang-devel / libclang-dev packages makes CI > running. > > https://github.com/pevik/ltp/commit/b2427f39ddb15c97761208a605637e0da6fe66ca Thanks, I guess this shows that libclang is well supported. > > The only missing piece is include/mk/clang-checks.mk (this patchset not even > compile now). > >> Richard Palethorpe (2): >> Add 'make checks' and clang-checks to build system > make check ... clang-check (to avoid confusion). > >> Start libclang based analyzer and TEST() check > >> configure.ac | 2 + >> 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-checks/.gitignore | 1 + >> tools/clang-checks/Makefile | 13 ++ >> tools/clang-checks/main.c | 218 +++++++++++++++++++++++++++++ > I'd name it tools/clang-check/ (and include/mk/clang-check.mk), but that's just > personal opinion. Yeah, we do not want a mixture of check and checks. I should probably just make it 'check' as it saves typing one letter. > > Kind regards, > Petr > >> 10 files changed, 264 insertions(+), 1 deletion(-) >> create mode 100644 tools/clang-checks/.gitignore >> create mode 100644 tools/clang-checks/Makefile >> create mode 100644 tools/clang-checks/main.c -- Thank you, Richard. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [RFC PATCH 0/2] Libclang based analyzer 2021-06-04 9:03 ` Richard Palethorpe @ 2021-06-04 9:15 ` Petr Vorel 2021-06-04 11:34 ` Cyril Hrubis 0 siblings, 1 reply; 12+ messages in thread From: Petr Vorel @ 2021-06-04 9:15 UTC (permalink / raw) To: ltp Hi Richie, > > FYI yesterday I sent patch to add make check{,-c,shell}, but for running C/shell API tests: > > https://patchwork.ozlabs.org/project/ltp/list/?series=247103 > So I guess we have a name collision here. Something to consider is that > GNU Make defines 'checks' as running self tests. So perhaps you are > correct to use that word. > I could rename the static analyses to 'analyze' or 'lint'. OTOH it might > be better if running the self tests is called 'make > run-{c,shell}-api-tests', because only a few people need to do > that. Whereas it is intended that all contributors run the static > analyses checks. run-{c,shell}-api-tests is IMHO too long, but I was thinking about check-{,c,shell}-api. But maybe you're right, let's wait for others opinions. > Although, if someone runs 'make check' in the lib directory, then it > makes sense to run the C API tests as well as do the analyses. Or not? Yes, I'd be for having make check, which would run all check targets, which can be also run separately: make check-clang make check-c make check-shell (or whatever we name these clang and C/shell API tests targets) > > Taking 2 latests commits + adding clang-devel / libclang-dev packages makes CI > > running. > > https://github.com/pevik/ltp/commit/b2427f39ddb15c97761208a605637e0da6fe66ca > Thanks, I guess this shows that libclang is well supported. Yes, it looks to be even in old clang 3.5. > > The only missing piece is include/mk/clang-checks.mk (this patchset not even > > compile now). > >> Richard Palethorpe (2): > >> Add 'make checks' and clang-checks to build system > > make check ... clang-check (to avoid confusion). > >> Start libclang based analyzer and TEST() check > >> configure.ac | 2 + > >> 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-checks/.gitignore | 1 + > >> tools/clang-checks/Makefile | 13 ++ > >> tools/clang-checks/main.c | 218 +++++++++++++++++++++++++++++ > > I'd name it tools/clang-check/ (and include/mk/clang-check.mk), but that's just > > personal opinion. > Yeah, we do not want a mixture of check and checks. I should probably > just make it 'check' as it saves typing one letter. +1 Kind regards, Petr ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [RFC PATCH 0/2] Libclang based analyzer 2021-06-04 9:15 ` Petr Vorel @ 2021-06-04 11:34 ` Cyril Hrubis 2021-06-04 12:51 ` Petr Vorel 0 siblings, 1 reply; 12+ messages in thread From: Cyril Hrubis @ 2021-06-04 11:34 UTC (permalink / raw) To: ltp Hi! > run-{c,shell}-api-tests is IMHO too long, but I was thinking about > check-{,c,shell}-api. But maybe you're right, let's wait for others opinions. I would vote for tests to be executed by 'make test' and the checks that Ritchie implements should be started by 'make check'. Even the kernel script is called checkpatch.pl so this would be consistent with the terms used in Linux kernel. -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [RFC PATCH 0/2] Libclang based analyzer 2021-06-04 11:34 ` Cyril Hrubis @ 2021-06-04 12:51 ` Petr Vorel 0 siblings, 0 replies; 12+ messages in thread From: Petr Vorel @ 2021-06-04 12:51 UTC (permalink / raw) To: ltp > Hi! > > run-{c,shell}-api-tests is IMHO too long, but I was thinking about > > check-{,c,shell}-api. But maybe you're right, let's wait for others opinions. > I would vote for tests to be executed by 'make test' and the checks that > Ritchie implements should be started by 'make check'. > Even the kernel script is called checkpatch.pl so this would be > consistent with the terms used in Linux kernel. Sure. It's probably better not mixing these two anyway. It'd be good to run all these in GitHub Actions and implement make help. Kind regards, Petr ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-06-04 12:51 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-06-03 15:48 [LTP] [RFC PATCH 0/2] Libclang based analyzer Richard Palethorpe 2021-06-03 15:48 ` [LTP] [RFC PATCH 1/2] Add 'make checks' and clang-checks to build system Richard Palethorpe 2021-06-04 6:06 ` Petr Vorel 2021-06-04 6:12 ` Petr Vorel 2021-06-04 8:42 ` Richard Palethorpe 2021-06-04 8:55 ` Petr Vorel 2021-06-03 15:48 ` [LTP] [RFC PATCH 2/2] Start libclang based analyzer and TEST() check Richard Palethorpe 2021-06-04 6:20 ` [LTP] [RFC PATCH 0/2] Libclang based analyzer Petr Vorel 2021-06-04 9:03 ` Richard Palethorpe 2021-06-04 9:15 ` Petr Vorel 2021-06-04 11:34 ` Cyril Hrubis 2021-06-04 12:51 ` Petr Vorel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox