[gcc r11-4930] analyzer: warn on invalid shift counts [PR97424]

[gcc r11-4930] analyzer: warn on invalid shift counts [PR97424]

[David Malcolm]

https://gcc.gnu.org/g:5e00ad3ffbfb4df7242c313a0d836f5b538eb2fb

commit r11-4930-g5e00ad3ffbfb4df7242c313a0d836f5b538eb2fb
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Nov 11 21:16:45 2020 -0500

    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.

Diff:
---
 gcc/analyzer/analyzer.opt                       |   8 ++
 gcc/analyzer/region-model.cc                    | 102 ++++++++++++++++++++++++
 gcc/doc/invoke.texi                             |  33 ++++++++
 gcc/testsuite/gcc.dg/analyzer/invalid-shift-1.c |  34 ++++++++
 4 files changed, 177 insertions(+)

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<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 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 <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); }