diff options
author | David Malcolm <dmalcolm@redhat.com> | 2020-11-11 21:16:45 -0500 |
---|---|---|
committer | David Malcolm <dmalcolm@redhat.com> | 2020-11-11 21:16:45 -0500 |
commit | 5e00ad3ffbfb4df7242c313a0d836f5b538eb2fb (patch) | |
tree | f06accd1c2401b59cc964b54a4cc174512940b61 /gcc | |
parent | 0f5f9ed5e5a041b636cc002451b1e8b2295f8e4f (diff) | |
download | gcc-5e00ad3ffbfb4df7242c313a0d836f5b538eb2fb.zip gcc-5e00ad3ffbfb4df7242c313a0d836f5b538eb2fb.tar.gz gcc-5e00ad3ffbfb4df7242c313a0d836f5b538eb2fb.tar.bz2 |
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.
Diffstat (limited to 'gcc')
-rw-r--r-- | gcc/analyzer/analyzer.opt | 8 | ||||
-rw-r--r-- | gcc/analyzer/region-model.cc | 102 | ||||
-rw-r--r-- | gcc/doc/invoke.texi | 33 | ||||
-rw-r--r-- | gcc/testsuite/gcc.dg/analyzer/invalid-shift-1.c | 34 |
4 files changed, 177 insertions, 0 deletions
diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt index c9df6dc..bbf9e42 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 d30047b..57c657b 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<shift_count_negative_diagnostic> +{ +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<shift_count_overflow_diagnostic> +{ +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 553cc07..69bf1fa 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 0000000..08e5272 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/invalid-shift-1.c @@ -0,0 +1,34 @@ +/* PR tree-optimization/97424. */ + +#include <stdint.h> + +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); } |