analyzer: use tainted_allocation_size::m_mem_space [PR105017]

gcc/analyzer/ChangeLog:
	PR analyzer/105017
	* sm-taint.cc (taint_diagnostic::subclass_equal_p): Check
	m_has_bounds as well as m_arg.
	(tainted_allocation_size::subclass_equal_p): Chain up to base
	class implementation.  Also check m_mem_space.
	(tainted_allocation_size::emit): Add note showing stack-based vs
	heap-based allocations.

gcc/testsuite/ChangeLog:
	PR analyzer/105017
	* gcc.dg/analyzer/taint-alloc-1.c: Add expected messages relating
	to heap vs stack.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>

diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc
index e2c78cdd42b225d9f7c1ae7b289f6d0a2b39e674..17669ae768562aa02a962499b393c64364ffc272 100644 (file)
--- a/gcc/analyzer/sm-taint.cc
+++ b/gcc/analyzer/sm-taint.cc
@@ -137,7 +137,9 @@ public:
 
   bool subclass_equal_p (const pending_diagnostic &base_other) const OVERRIDE
   {
-    return same_tree_p (m_arg, ((const taint_diagnostic &)base_other).m_arg);
+    const taint_diagnostic &other = (const taint_diagnostic &)base_other;
+    return (same_tree_p (m_arg, other.m_arg)
+           && m_has_bounds == other.m_has_bounds);
   }
 
   label_text describe_state_change (const evdesc::state_change &change)
@@ -523,6 +525,15 @@ public:
     return "tainted_allocation_size";
   }
 
+  bool subclass_equal_p (const pending_diagnostic &base_other) const OVERRIDE
+  {
+    if (!taint_diagnostic::subclass_equal_p (base_other))
+      return false;
+    const tainted_allocation_size &other
+      = (const tainted_allocation_size &)base_other;
+    return m_mem_space == other.m_mem_space;
+  }
+
   int get_controlling_option () const FINAL OVERRIDE
   {
     return OPT_Wanalyzer_tainted_allocation_size;
@@ -533,29 +544,32 @@ public:
     diagnostic_metadata m;
     /* "CWE-789: Memory Allocation with Excessive Size Value".  */
     m.add_cwe (789);
-    // TODO: make use of m_mem_space
+
+    bool warned;
     if (m_arg)
       switch (m_has_bounds)
        {
        default:
          gcc_unreachable ();
        case BOUNDS_NONE:
-         return warning_meta (rich_loc, m, get_controlling_option (),
-                              "use of attacker-controlled value %qE as"
-                              " allocation size without bounds checking",
-                              m_arg);
+         warned = warning_meta (rich_loc, m, get_controlling_option (),
+                                "use of attacker-controlled value %qE as"
+                                " allocation size without bounds checking",
+                                m_arg);
          break;
        case BOUNDS_UPPER:
-         return warning_meta (rich_loc, m, get_controlling_option (),
-                              "use of attacker-controlled value %qE as"
-                              " allocation size without lower-bounds checking",
-                              m_arg);
+         warned = warning_meta (rich_loc, m, get_controlling_option (),
+                                "use of attacker-controlled value %qE as"
+                                " allocation size without"
+                                " lower-bounds checking",
+                                m_arg);
          break;
        case BOUNDS_LOWER:
-         return warning_meta (rich_loc, m, get_controlling_option (),
-                              "use of attacker-controlled value %qE as"
-                              " allocation size without upper-bounds checking",
-                            m_arg);
+         warned = warning_meta (rich_loc, m, get_controlling_option (),
+                                "use of attacker-controlled value %qE as"
+                                " allocation size without"
+                                " upper-bounds checking",
+                                m_arg);
          break;
        }
     else
@@ -564,24 +578,40 @@ public:
        default:
          gcc_unreachable ();
        case BOUNDS_NONE:
-         return warning_meta (rich_loc, m, get_controlling_option (),
-                              "use of attacker-controlled value as"
-                              " allocation size without bounds"
-                              " checking");
+         warned = warning_meta (rich_loc, m, get_controlling_option (),
+                                "use of attacker-controlled value as"
+                                " allocation size without bounds"
+                                " checking");
          break;
        case BOUNDS_UPPER:
-         return warning_meta (rich_loc, m, get_controlling_option (),
-                              "use of attacker-controlled value as"
-                              " allocation size without lower-bounds"
-                              " checking");
+         warned = warning_meta (rich_loc, m, get_controlling_option (),
+                                "use of attacker-controlled value as"
+                                " allocation size without"
+                                " lower-bounds checking");
          break;
        case BOUNDS_LOWER:
-         return warning_meta (rich_loc, m, get_controlling_option (),
-                              "use of attacker-controlled value as"
-                              " allocation size without upper-bounds"
-                              " checking");
+         warned = warning_meta (rich_loc, m, get_controlling_option (),
+                                "use of attacker-controlled value as"
+                                " allocation size without"
+                                " upper-bounds checking");
          break;
        }
+    if (warned)
+      {
+       location_t loc = rich_loc->get_loc ();
+       switch (m_mem_space)
+         {
+         default:
+           break;
+         case MEMSPACE_STACK:
+           inform (loc, "stack-based allocation");
+           break;
+         case MEMSPACE_HEAP:
+           inform (loc, "heap-based allocation");
+           break;
+         }
+      }
+    return warned;
   }
 
   label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE

diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-alloc-1.c b/gcc/testsuite/gcc.dg/analyzer/taint-alloc-1.c
index bc4f63bb3bd6fd2644336d01b2f15349b9db09ed..cb2db6c69cfc4261e9d5da4d491f21e924c960f3 100644 (file)
--- a/gcc/testsuite/gcc.dg/analyzer/taint-alloc-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/taint-alloc-1.c
@@ -25,6 +25,7 @@ void *test_1 (FILE *f)
     return malloc (tmp.sz); /* { dg-warning "use of attacker-controlled value 'tmp\\.sz' as allocation size without upper-bounds checking" "warning" } */
     /* { dg-message "23: \\(\[0-9\]+\\) 'tmp.i' has an unchecked value here \\(from 'tmp'\\)" "event: tmp.i has an unchecked value" { xfail *-*-* } .-1 } */
     /* { dg-message "\\(\[0-9\]+\\) use of attacker-controlled value 'tmp\\.sz' as allocation size without upper-bounds checking" "final event" { target *-*-* } .-2 } */
+    /* { dg-message "heap-based allocation" "memory space" { target *-*-* } .-3 } */
     
     // TOOD: better messages for state changes
   }
@@ -46,6 +47,7 @@ void *test_2 (FILE *f)
       char buf[tmp.sz]; /* { dg-warning "use of attacker-controlled value 'tmp\\.sz' as allocation size without upper-bounds checking" "warning" } */
       /* { dg-message "\\(\[0-9\]+\\) 'tmp.i' has an unchecked value here \\(from 'tmp'\\)" "event: tmp.i has an unchecked value" { xfail *-*-* } .-1 } */
       /* { dg-message "\\(\[0-9\]+\\) use of attacker-controlled value 'tmp\\.sz' as allocation size without upper-bounds checking" "final event" { target *-*-* } .-2 } */
+      /* { dg-message "stack-based allocation" "memory space" { target *-*-* } .-3 } */
       fread (buf, tmp.sz, 1, f);
     }