Message ID | 20210614115638.25467-3-rpalethorpe@suse.com |
---|---|
State | Superseded |
Headers | show |
Series | Libclang based analyzer | expand |
Hi Richie, > +#if HAVE_CLANG_C_INDEX_H > + > +#include <clang-c/Index.h> ... > +static void emit_error(const char *const error_msg) > +{ > + if (color_enabled(STDERR_FILENO)) { > + dprintf(STDERR_FILENO, > + "%sERROR%s: %s%s%s\n", > + ansi_red, ansi_reset, > + ansi_bold, error_msg, ansi_reset); > + } else { > + dprintf(STDERR_FILENO, "ERROR: %s\n", error_msg); > + } > +} ... > + if (ret != CXError_Success) { > + emit_error("Failed to parse translation unit!"); > + return 1; > + } ... > +#else > + > +int main(const attr_unused int argc, const attr_unused char *const *const argv) > +{ > + emit_error("clang-checks was not built correctly; libclang headers are not installed!\n"); emit_error() is not visible here, thus build fails. Please add it before HAVE_CLANG_C_INDEX_H. Or you could just use tst_test.h with TST_NO_DEFAULT_MAIN and here would be TST_TEST_TCONF() (+ LTP_ATTRIBUTE_UNUSED). Kind regards, Petr > + return 1; > +} > + > +#endif
Hi Richie, > Hi Richie, > > +#if HAVE_CLANG_C_INDEX_H > > + > > +#include <clang-c/Index.h> > ... > > +static void emit_error(const char *const error_msg) > > +{ > > + if (color_enabled(STDERR_FILENO)) { > > + dprintf(STDERR_FILENO, > > + "%sERROR%s: %s%s%s\n", > > + ansi_red, ansi_reset, > > + ansi_bold, error_msg, ansi_reset); > > + } else { > > + dprintf(STDERR_FILENO, "ERROR: %s\n", error_msg); > > + } > > +} > ... > > + if (ret != CXError_Success) { > > + emit_error("Failed to parse translation unit!"); > > + return 1; > > + } > ... > > +#else > > + > > +int main(const attr_unused int argc, const attr_unused char *const *const argv) > > +{ > > + emit_error("clang-checks was not built correctly; libclang headers are not installed!\n"); > emit_error() is not visible here, thus build fails. Please add it before HAVE_CLANG_C_INDEX_H. > Or you could just use tst_test.h with TST_NO_DEFAULT_MAIN and here would be TST_TEST_TCONF() > (+ LTP_ATTRIBUTE_UNUSED). ... > > +/* Copied from lib/tst_ansi_color.c */ > > +static int color_enabled(const int fd) Also you'd probably get tst_color_enabled() and other things from lib/tst_ansi_color.c for color handling for free when using tst_test.h. But that's just a minor detail. Kind regards, Petr > Kind regards, > Petr > > + return 1; > > +} > > + > > +#endif
Hello, Petr Vorel <pvorel@suse.cz> writes: > Hi Richie, > >> Hi Richie, > >> > +#if HAVE_CLANG_C_INDEX_H >> > + >> > +#include <clang-c/Index.h> >> ... > >> > +static void emit_error(const char *const error_msg) >> > +{ >> > + if (color_enabled(STDERR_FILENO)) { >> > + dprintf(STDERR_FILENO, >> > + "%sERROR%s: %s%s%s\n", >> > + ansi_red, ansi_reset, >> > + ansi_bold, error_msg, ansi_reset); >> > + } else { >> > + dprintf(STDERR_FILENO, "ERROR: %s\n", error_msg); >> > + } >> > +} >> ... >> > + if (ret != CXError_Success) { >> > + emit_error("Failed to parse translation unit!"); >> > + return 1; >> > + } >> ... > >> > +#else >> > + >> > +int main(const attr_unused int argc, const attr_unused char *const *const argv) >> > +{ >> > + emit_error("clang-checks was not built correctly; libclang headers are not installed!\n"); >> emit_error() is not visible here, thus build fails. Please add it >before HAVE_CLANG_C_INDEX_H. +1 Uhg. > >> Or you could just use tst_test.h with TST_NO_DEFAULT_MAIN and here would be TST_TEST_TCONF() >> (+ LTP_ATTRIBUTE_UNUSED). > > ... >> > +/* Copied from lib/tst_ansi_color.c */ >> > +static int color_enabled(const int fd) > > Also you'd probably get tst_color_enabled() and other things from > lib/tst_ansi_color.c for color handling for free when using > tst_test.h. We would probably have to build the ltplib with HOSTCC. I don't think we can just include the headers. It is tempting, but it also seems very circular. I can imagine someone half refactoring a library and wanting to run the checks on one translation unit. However Make would detect a dependency has changed, so would try to rebuild the checker with a broken ltplib... We could probably make it work, but having the checker depend on the thing it checks seems like a recipe for complication. Meanwhile we just get to share a few macros and string constants. > > But that's just a minor detail. > > Kind regards, > Petr > >> Kind regards, >> Petr > >> > + return 1; >> > +} >> > + >> > +#endif
Richard Palethorpe <rpalethorpe@suse.de> writes: > Hello, > > Petr Vorel <pvorel@suse.cz> writes: > >> Hi Richie, >> >>> Hi Richie, >> >>> > +#if HAVE_CLANG_C_INDEX_H >>> > + >>> > +#include <clang-c/Index.h> >>> ... >> >>> > +static void emit_error(const char *const error_msg) >>> > +{ >>> > + if (color_enabled(STDERR_FILENO)) { >>> > + dprintf(STDERR_FILENO, >>> > + "%sERROR%s: %s%s%s\n", >>> > + ansi_red, ansi_reset, >>> > + ansi_bold, error_msg, ansi_reset); >>> > + } else { >>> > + dprintf(STDERR_FILENO, "ERROR: %s\n", error_msg); >>> > + } >>> > +} >>> ... >>> > + if (ret != CXError_Success) { >>> > + emit_error("Failed to parse translation unit!"); >>> > + return 1; >>> > + } >>> ... >> >>> > +#else >>> > + >>> > +int main(const attr_unused int argc, const attr_unused char *const *const argv) >>> > +{ >>> > + emit_error("clang-checks was not built correctly; libclang headers are not installed!\n"); >>> emit_error() is not visible here, thus build fails. Please add it >>before HAVE_CLANG_C_INDEX_H. > > +1 > > Uhg. > >> >>> Or you could just use tst_test.h with TST_NO_DEFAULT_MAIN and here would be TST_TEST_TCONF() >>> (+ LTP_ATTRIBUTE_UNUSED). >> >> ... >>> > +/* Copied from lib/tst_ansi_color.c */ >>> > +static int color_enabled(const int fd) >> >> Also you'd probably get tst_color_enabled() and other things from >> lib/tst_ansi_color.c for color handling for free when using >> tst_test.h. > > We would probably have to build the ltplib with HOSTCC. I don't think we > can just include the headers. > > It is tempting, but it also seems very circular. I can imagine someone > half refactoring a library and wanting to run the checks on one > translation unit. However Make would detect a dependency has changed, so > would try to rebuild the checker with a broken ltplib... > > We could probably make it work, but having the checker depend on the > thing it checks seems like a recipe for complication. Meanwhile we just > get to share a few macros and string constants. Although we could create a tools lib with shared code for the meta data parser and maybe the future parallel executor if that does not use the test lib. > >> >> But that's just a minor detail. >> >> Kind regards, >> Petr >> >>> Kind regards, >>> Petr >> >>> > + return 1; >>> > +} >>> > + >>> > +#endif
Hi Richie, ... > >>> > +#else > >>> > + > >>> > +int main(const attr_unused int argc, const attr_unused char *const *const argv) > >>> > +{ > >>> > + emit_error("clang-checks was not built correctly; libclang headers are not installed!\n"); > >>> emit_error() is not visible here, thus build fails. Please add it > >>before HAVE_CLANG_C_INDEX_H. > > +1 > > Uhg. > >>> Or you could just use tst_test.h with TST_NO_DEFAULT_MAIN and here would be TST_TEST_TCONF() > >>> (+ LTP_ATTRIBUTE_UNUSED). > >> ... > >>> > +/* Copied from lib/tst_ansi_color.c */ > >>> > +static int color_enabled(const int fd) > >> Also you'd probably get tst_color_enabled() and other things from > >> lib/tst_ansi_color.c for color handling for free when using > >> tst_test.h. > > We would probably have to build the ltplib with HOSTCC. I don't think we > > can just include the headers. > > It is tempting, but it also seems very circular. I can imagine someone > > half refactoring a library and wanting to run the checks on one > > translation unit. However Make would detect a dependency has changed, so > > would try to rebuild the checker with a broken ltplib... > > We could probably make it work, but having the checker depend on the > > thing it checks seems like a recipe for complication. Meanwhile we just > > get to share a few macros and string constants. > Although we could create a tools lib with shared code for the meta data > parser and maybe the future parallel executor if that does not use the > test lib. Uh, I'm sorry, I missed build dependency. It's probably not worth to rewriting this part just to reuse a bit of code. We might look into that in the future, but for now please just fix a build with your original approach. Kind regards, Petr
diff --git a/tools/clang-check/main.c b/tools/clang-check/main.c new file mode 100644 index 000000000..feb6774fd --- /dev/null +++ b/tools/clang-check/main.c @@ -0,0 +1,238 @@ +// 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. We look at + * the kind of each node and decide if one or more checks can be run + * on it. If at least the kind of node matches we pass it to the + * checking function. This function may then perform further + * matching. Finally, if it finds the sub tree is the type of thing + * which needs to be checked, then it performs the check and emits an + * error if it fails. + * + * AST Nodes are called CXCursor by libclang. We use the library's + * visitor functions to recurse into the AST. + * + * 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 "../../include/config.h" + +#define attr_unused __attribute__((unused)) + +#if HAVE_CLANG_C_INDEX_H + +#include <clang-c/Index.h> + +/* The rules for test, library and tool code are different */ +enum ltp_tu_kind { + LTP_LIB, + 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(const char *const error_msg) +{ + if (color_enabled(STDERR_FILENO)) { + dprintf(STDERR_FILENO, + "%sERROR%s: %s%s%s\n", + ansi_red, ansi_reset, + ansi_bold, error_msg, ansi_reset); + } else { + dprintf(STDERR_FILENO, "ERROR: %s\n", error_msg); + } +} + +static void emit_check_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_LIB) + return; + + if (!cursor_cmp_spelling("TEST", macro_cursor)) { + emit_check_error(macro_cursor, + "TEST() macro should not be used in library"); + } +} + +/* Recursively visit each AST node and run checks based on node kind */ +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; +} + +static void collect_info_from_args(const int argc, const char *const *const argv) +{ + int i; + + for (i = 0; i < argc; i++) { + if (!strcmp("-DLTPLIB", argv[i])) { + tu_info.tu_kind = LTP_LIB; + } + } +} + +int main(const int argc, const char *const *const argv) +{ + CXIndex cindex = clang_createIndex(0, 1); + CXTranslationUnit tu; + CXCursor tuc; + enum CXErrorCode ret; + + tu_info.tu_kind = LTP_OTHER; + collect_info_from_args(argc, argv); + + 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) { + emit_error("Failed to parse translation unit!"); + return 1; + } + + tuc = clang_getTranslationUnitCursor(tu); + + clang_visitChildren(tuc, check_visitor, NULL); + + /* Stop leak sanitizer from complaining */ + clang_disposeTranslationUnit(tu); + clang_disposeIndex(cindex); + + return error_flag; +} + +#else + +int main(const attr_unused int argc, const attr_unused char *const *const argv) +{ + emit_error("clang-checks was not built correctly; libclang headers are not installed!\n"); + return 1; +} + +#endif
This uses the stable Clang C API to find usages of the TEST() macro. 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. For now we are limiting ourselves to the functions available in the LLVM-10 release. Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> --- tools/clang-check/main.c | 238 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 238 insertions(+) create mode 100644 tools/clang-check/main.c