From 5e00ad3ffbfb4df7242c313a0d836f5b538eb2fb Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Wed, 11 Nov 2020 21:16:45 -0500 Subject: [PATCH] analyzer: warn on invalid shift counts [PR97424] This patch implements -Wanalyzer-shift-count-negative and -Wanalyzer-shift-count-overflow, analogous to the C/C++ warnings -Wshift-count-negative and -Wshift-count-overflow, but implemented via interprocedural path analysis rather than via parsing in a front end, and thus capable of detecting interprocedural cases that the warnings implemented in the front ends can miss. gcc/analyzer/ChangeLog: PR tree-optimization/97424 * analyzer.opt (Wanalyzer-shift-count-negative): New. (Wanalyzer-shift-count-overflow): New. * region-model.cc (class shift_count_negative_diagnostic): New. (class shift_count_overflow_diagnostic): New. (region_model::get_gassign_result): Complain about shift counts that are negative or are >= the operand's type's width. gcc/ChangeLog: PR tree-optimization/97424 * doc/invoke.texi (Static Analyzer Options): Add -Wno-analyzer-shift-count-negative and -Wno-analyzer-shift-count-overflow. (-Wno-analyzer-shift-count-negative): New. (-Wno-analyzer-shift-count-overflow): New. gcc/testsuite/ChangeLog: PR tree-optimization/97424 * gcc.dg/analyzer/invalid-shift-1.c: New test. --- gcc/analyzer/analyzer.opt | 8 ++ gcc/analyzer/region-model.cc | 102 ++++++++++++++++++ gcc/doc/invoke.texi | 33 ++++++ .../gcc.dg/analyzer/invalid-shift-1.c | 34 ++++++ 4 files changed, 177 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/analyzer/invalid-shift-1.c diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt index c9df6dc7673..bbf9e429c99 100644 --- a/gcc/analyzer/analyzer.opt +++ b/gcc/analyzer/analyzer.opt @@ -98,6 +98,14 @@ Wanalyzer-null-dereference Common Var(warn_analyzer_null_dereference) Init(1) Warning Warn about code paths in which a NULL pointer is dereferenced. +Wanalyzer-shift-count-negative +Common Var(warn_analyzer_shift_count_negative) Init(1) Warning +Warn about code paths in which a shift with negative count is attempted. + +Wanalyzer-shift-count-overflow +Common Var(warn_analyzer_shift_count_overflow) Init(1) Warning +Warn about code paths in which a shift with count >= width of type is attempted. + Wanalyzer-stale-setjmp-buffer Common Var(warn_analyzer_stale_setjmp_buffer) Init(1) Warning Warn about code paths in which a longjmp rewinds to a jmp_buf saved in a stack frame that has returned. diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index d30047b279d..57c657bf6b8 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -363,6 +363,88 @@ private: enum poison_kind m_pkind; }; +/* A subclass of pending_diagnostic for complaining about shifts + by negative counts. */ + +class shift_count_negative_diagnostic +: public pending_diagnostic_subclass +{ +public: + shift_count_negative_diagnostic (const gassign *assign, tree count_cst) + : m_assign (assign), m_count_cst (count_cst) + {} + + const char *get_kind () const FINAL OVERRIDE + { + return "shift_count_negative_diagnostic"; + } + + bool operator== (const shift_count_negative_diagnostic &other) const + { + return (m_assign == other.m_assign + && same_tree_p (m_count_cst, other.m_count_cst)); + } + + bool emit (rich_location *rich_loc) FINAL OVERRIDE + { + return warning_at (rich_loc, OPT_Wanalyzer_shift_count_negative, + "shift by negative count (%qE)", m_count_cst); + } + + label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE + { + return ev.formatted_print ("shift by negative amount here (%qE)", m_count_cst); + } + +private: + const gassign *m_assign; + tree m_count_cst; +}; + +/* A subclass of pending_diagnostic for complaining about shifts + by counts >= the width of the operand type. */ + +class shift_count_overflow_diagnostic +: public pending_diagnostic_subclass +{ +public: + shift_count_overflow_diagnostic (const gassign *assign, + int operand_precision, + tree count_cst) + : m_assign (assign), m_operand_precision (operand_precision), + m_count_cst (count_cst) + {} + + const char *get_kind () const FINAL OVERRIDE + { + return "shift_count_overflow_diagnostic"; + } + + bool operator== (const shift_count_overflow_diagnostic &other) const + { + return (m_assign == other.m_assign + && m_operand_precision == other.m_operand_precision + && same_tree_p (m_count_cst, other.m_count_cst)); + } + + bool emit (rich_location *rich_loc) FINAL OVERRIDE + { + return warning_at (rich_loc, OPT_Wanalyzer_shift_count_overflow, + "shift by count (%qE) >= precision of type (%qi)", + m_count_cst, m_operand_precision); + } + + label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE + { + return ev.formatted_print ("shift by count %qE here", m_count_cst); + } + +private: + const gassign *m_assign; + int m_operand_precision; + tree m_count_cst; +}; + /* If ASSIGN is a stmt that can be modelled via set_value (lhs_reg, SVALUE, CTXT) for some SVALUE, get the SVALUE. @@ -514,6 +596,26 @@ region_model::get_gassign_result (const gassign *assign, const svalue *rhs1_sval = get_rvalue (rhs1, ctxt); const svalue *rhs2_sval = get_rvalue (rhs2, ctxt); + if (ctxt && (op == LSHIFT_EXPR || op == RSHIFT_EXPR)) + { + /* "INT34-C. Do not shift an expression by a negative number of bits + or by greater than or equal to the number of bits that exist in + the operand." */ + if (const tree rhs2_cst = rhs2_sval->maybe_get_constant ()) + if (TREE_CODE (rhs2_cst) == INTEGER_CST) + { + if (tree_int_cst_sgn (rhs2_cst) < 0) + ctxt->warn (new shift_count_negative_diagnostic + (assign, rhs2_cst)); + else if (compare_tree_int (rhs2_cst, + TYPE_PRECISION (TREE_TYPE (rhs1))) + >= 0) + ctxt->warn (new shift_count_overflow_diagnostic + (assign, TYPE_PRECISION (TREE_TYPE (rhs1)), + rhs2_cst)); + } + } + const svalue *sval_binop = m_mgr->get_or_create_binop (TREE_TYPE (lhs), op, rhs1_sval, rhs2_sval); diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 553cc07e330..69bf1fa89dd 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -425,6 +425,8 @@ Objective-C and Objective-C++ Dialects}. -Wno-analyzer-null-dereference @gol -Wno-analyzer-possible-null-argument @gol -Wno-analyzer-possible-null-dereference @gol +-Wno-analyzer-shift-count-negative @gol +-Wno-analyzer-shift-count-overflow @gol -Wno-analyzer-stale-setjmp-buffer @gol -Wno-analyzer-tainted-array-index @gol -Wanalyzer-too-complex @gol @@ -8897,6 +8899,8 @@ Enabling this option effectively enables the following warnings: -Wanalyzer-possible-null-dereference @gol -Wanalyzer-null-argument @gol -Wanalyzer-null-dereference @gol +-Wanalyzer-shift-count-negative @gol +-Wanalyzer-shift-count-overflow @gol -Wanalyzer-stale-setjmp-buffer @gol -Wanalyzer-tainted-array-index @gol -Wanalyzer-unsafe-call-within-signal-handler @gol @@ -9030,6 +9034,35 @@ This warning requires @option{-fanalyzer}, which enables it; use This diagnostic warns for paths through the code in which a value known to be NULL is dereferenced. +@item -Wno-analyzer-shift-count-negative +@opindex Wanalyzer-shift-count-negative +@opindex Wno-analyzer-shift-count-negative +This warning requires @option{-fanalyzer}, which enables it; use +@option{-Wno-analyzer-shift-count-negative} to disable it. + +This diagnostic warns for paths through the code in which a +shift is attempted with a negative count. It is analogous to +the @option{-Wshift-count-negative} diagnostic implemented in +the C/C++ front ends, but is implemented based on analyzing +interprocedural paths, rather than merely parsing the syntax tree. +However, the analyzer does not prioritize detection of such paths, so +false negatives are more likely relative to other warnings. + +@item -Wno-analyzer-shift-count-overflow +@opindex Wanalyzer-shift-count-overflow +@opindex Wno-analyzer-shift-count-overflow +This warning requires @option{-fanalyzer}, which enables it; use +@option{-Wno-analyzer-shift-count-overflow} to disable it. + +This diagnostic warns for paths through the code in which a +shift is attempted with a count greater than or equal to the +precision of the operand's type. It is analogous to +the @option{-Wshift-count-overflow} diagnostic implemented in +the C/C++ front ends, but is implemented based on analyzing +interprocedural paths, rather than merely parsing the syntax tree. +However, the analyzer does not prioritize detection of such paths, so +false negatives are more likely relative to other warnings. + @item -Wno-analyzer-stale-setjmp-buffer @opindex Wanalyzer-stale-setjmp-buffer @opindex Wno-analyzer-stale-setjmp-buffer diff --git a/gcc/testsuite/gcc.dg/analyzer/invalid-shift-1.c b/gcc/testsuite/gcc.dg/analyzer/invalid-shift-1.c new file mode 100644 index 00000000000..08e52728748 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/invalid-shift-1.c @@ -0,0 +1,34 @@ +/* PR tree-optimization/97424. */ + +#include + +static inline uint32_t +_dl_hwcaps_subdirs_build_bitmask (int subdirs, int active) +{ + /* Leading subdirectories that are not active. */ + int inactive = subdirs - active; + if (inactive == 32) + return 0; + + uint32_t mask; + if (subdirs != 32) + mask = (1 << subdirs) - 1; /* { dg-message "shift by count \\('33'\\) >= precision of type \\('\[0-9\]+'\\)" } */ + else + mask = -1; + return mask ^ ((1U << inactive) - 1); /* { dg-message "shift by negative count \\('-1'\\)" } */ +} + +void f1 (int); + +void +f2 (void) +{ + f1 (_dl_hwcaps_subdirs_build_bitmask (1, 2)); + f1 (_dl_hwcaps_subdirs_build_bitmask (33, 31)); +} + +static int __attribute__((noinline)) op3 (int op, int c) { return op << c; } /* { dg-message "shift by negative count \\('-1'\\)" } */ +int test_3 (void) { return op3 (1, -1); } + +static int __attribute__((noinline)) op4 (int op, int c) { return op << c; } +int test_4 (void) { return op4 (1, 0); } -- 2.30.2