public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r11-3829] analyzer: add warnings about writes to constant regions [PR95007]
@ 2020-10-12 16:06 David Malcolm
0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2020-10-12 16:06 UTC (permalink / raw)
To: gcc-cvs
https://gcc.gnu.org/g:3175d40fc52fb8eb3c3b18cc343d773da24434fb
commit r11-3829-g3175d40fc52fb8eb3c3b18cc343d773da24434fb
Author: David Malcolm <dmalcolm@redhat.com>
Date: Wed Oct 7 18:34:09 2020 -0400
analyzer: add warnings about writes to constant regions [PR95007]
This patch adds two new warnings:
-Wanalyzer-write-to-const
-Wanalyzer-write-to-string-literal
for code paths where the analyzer detects a write to a constant region.
As noted in the documentation part of the patch, the analyzer doesn't
prioritize detection of such writes, in that the state-merging logic
will blithely lose the distinction between const and non-const regions.
Hence false negatives are likely to arise due to state-merging.
However, if the analyzer does happen to spot such a write, it seems worth
reporting, hence this patch.
gcc/analyzer/ChangeLog:
* analyzer.opt (Wanalyzer-write-to-const): New.
(Wanalyzer-write-to-string-literal): New.
* region-model-impl-calls.cc (region_model::impl_call_memcpy):
Call check_for_writable_region.
(region_model::impl_call_memset): Likewise.
(region_model::impl_call_strcpy): Likewise.
* region-model.cc (class write_to_const_diagnostic): New.
(class write_to_string_literal_diagnostic): New.
(region_model::check_for_writable_region): New.
(region_model::set_value): Call check_for_writable_region.
* region-model.h (region_model::check_for_writable_region): New
decl.
gcc/ChangeLog:
* doc/invoke.texi: Document -Wanalyzer-write-to-const and
-Wanalyzer-write-to-string-literal.
gcc/testsuite/ChangeLog:
PR c/83347
PR middle-end/90404
PR analyzer/95007
* gcc.dg/analyzer/write-to-const-1.c: New test.
* gcc.dg/analyzer/write-to-string-literal-1.c: New test.
Diff:
---
gcc/analyzer/analyzer.opt | 8 ++
gcc/analyzer/region-model-impl-calls.cc | 6 ++
gcc/analyzer/region-model.cc | 117 ++++++++++++++++++++-
gcc/analyzer/region-model.h | 3 +
gcc/doc/invoke.texi | 28 +++++
gcc/testsuite/gcc.dg/analyzer/write-to-const-1.c | 29 +++++
.../gcc.dg/analyzer/write-to-string-literal-1.c | 58 ++++++++++
7 files changed, 248 insertions(+), 1 deletion(-)
diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index a4d384211f3..c9df6dc7673 100644
--- a/gcc/analyzer/analyzer.opt
+++ b/gcc/analyzer/analyzer.opt
@@ -114,6 +114,14 @@ Wanalyzer-use-of-pointer-in-stale-stack-frame
Common Var(warn_analyzer_use_of_pointer_in_stale_stack_frame) Init(1) Warning
Warn about code paths in which a pointer to a stale stack frame is used.
+Wanalyzer-write-to-const
+Common Var(warn_analyzer_write_to_const) Init(1) Warning
+Warn about code paths which attempt to write to a const object.
+
+Wanalyzer-write-to-string-literal
+Common Var(warn_analyzer_write_to_string_literal) Init(1) Warning
+Warn about code paths which attempt to write to a string literal.
+
Wanalyzer-too-complex
Common Var(warn_analyzer_too_complex) Init(0) Warning
Warn if the code is too complicated for the analyzer to fully explore.
diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc
index 009b8c3ecb0..ef84e638992 100644
--- a/gcc/analyzer/region-model-impl-calls.cc
+++ b/gcc/analyzer/region-model-impl-calls.cc
@@ -305,6 +305,8 @@ region_model::impl_call_memcpy (const call_details &cd)
return;
}
+ check_for_writable_region (dest_reg, cd.get_ctxt ());
+
/* Otherwise, mark region's contents as unknown. */
mark_region_as_unknown (dest_reg);
}
@@ -346,6 +348,8 @@ region_model::impl_call_memset (const call_details &cd)
}
}
+ check_for_writable_region (dest_reg, cd.get_ctxt ());
+
/* Otherwise, mark region's contents as unknown. */
mark_region_as_unknown (dest_reg);
return false;
@@ -397,6 +401,8 @@ region_model::impl_call_strcpy (const call_details &cd)
cd.maybe_set_lhs (dest_sval);
+ check_for_writable_region (dest_reg, cd.get_ctxt ());
+
/* For now, just mark region's contents as unknown. */
mark_region_as_unknown (dest_reg);
}
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index a88a295a241..480f25a3a4b 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -1532,16 +1532,131 @@ region_model::deref_rvalue (const svalue *ptr_sval, tree ptr_tree,
return m_mgr->get_symbolic_region (ptr_sval);
}
+/* A subclass of pending_diagnostic for complaining about writes to
+ constant regions of memory. */
+
+class write_to_const_diagnostic
+: public pending_diagnostic_subclass<write_to_const_diagnostic>
+{
+public:
+ write_to_const_diagnostic (const region *reg, tree decl)
+ : m_reg (reg), m_decl (decl)
+ {}
+
+ const char *get_kind () const FINAL OVERRIDE
+ {
+ return "write_to_const_diagnostic";
+ }
+
+ bool operator== (const write_to_const_diagnostic &other) const
+ {
+ return (m_reg == other.m_reg
+ && m_decl == other.m_decl);
+ }
+
+ bool emit (rich_location *rich_loc) FINAL OVERRIDE
+ {
+ bool warned = warning_at (rich_loc, OPT_Wanalyzer_write_to_const,
+ "write to %<const%> object %qE", m_decl);
+ if (warned)
+ inform (DECL_SOURCE_LOCATION (m_decl), "declared here");
+ return warned;
+ }
+
+ label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
+ {
+ return ev.formatted_print ("write to %<const%> object %qE here", m_decl);
+ }
+
+private:
+ const region *m_reg;
+ tree m_decl;
+};
+
+/* A subclass of pending_diagnostic for complaining about writes to
+ string literals. */
+
+class write_to_string_literal_diagnostic
+: public pending_diagnostic_subclass<write_to_string_literal_diagnostic>
+{
+public:
+ write_to_string_literal_diagnostic (const region *reg)
+ : m_reg (reg)
+ {}
+
+ const char *get_kind () const FINAL OVERRIDE
+ {
+ return "write_to_string_literal_diagnostic";
+ }
+
+ bool operator== (const write_to_string_literal_diagnostic &other) const
+ {
+ return m_reg == other.m_reg;
+ }
+
+ bool emit (rich_location *rich_loc) FINAL OVERRIDE
+ {
+ return warning_at (rich_loc, OPT_Wanalyzer_write_to_string_literal,
+ "write to string literal");
+ /* Ideally we would show the location of the STRING_CST as well,
+ but it is not available at this point. */
+ }
+
+ label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
+ {
+ return ev.formatted_print ("write to string literal here");
+ }
+
+private:
+ const region *m_reg;
+};
+
+/* Use CTXT to warn If DEST_REG is a region that shouldn't be written to. */
+
+void
+region_model::check_for_writable_region (const region* dest_reg,
+ region_model_context *ctxt) const
+{
+ /* Fail gracefully if CTXT is NULL. */
+ if (!ctxt)
+ return;
+
+ const region *base_reg = dest_reg->get_base_region ();
+ switch (base_reg->get_kind ())
+ {
+ default:
+ break;
+ case RK_DECL:
+ {
+ const decl_region *decl_reg = as_a <const decl_region *> (base_reg);
+ tree decl = decl_reg->get_decl ();
+ /* Warn about writes to const globals.
+ Don't warn for writes to const locals, and params in particular,
+ since we would warn in push_frame when setting them up (e.g the
+ "this" param is "T* const"). */
+ if (TREE_READONLY (decl)
+ && is_global_var (decl))
+ ctxt->warn (new write_to_const_diagnostic (dest_reg, decl));
+ }
+ break;
+ case RK_STRING:
+ ctxt->warn (new write_to_string_literal_diagnostic (dest_reg));
+ break;
+ }
+}
+
/* Set the value of the region given by LHS_REG to the value given
by RHS_SVAL. */
void
region_model::set_value (const region *lhs_reg, const svalue *rhs_sval,
- region_model_context */*ctxt*/)
+ region_model_context *ctxt)
{
gcc_assert (lhs_reg);
gcc_assert (rhs_sval);
+ check_for_writable_region (lhs_reg, ctxt);
+
m_store.set_value (m_mgr->get_store_manager(), lhs_reg, rhs_sval,
BK_direct);
}
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index cfeac8d6951..234ca16bcef 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -2736,6 +2736,9 @@ class region_model
bool called_from_main_p () const;
const svalue *get_initial_value_for_global (const region *reg) const;
+ void check_for_writable_region (const region* dest_reg,
+ region_model_context *ctxt) const;
+
/* Storing this here to avoid passing it around everywhere. */
region_model_manager *const m_mgr;
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 307f4f5426c..c8281ecf502 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -429,6 +429,8 @@ Objective-C and Objective-C++ Dialects}.
-Wno-analyzer-use-after-free @gol
-Wno-analyzer-use-of-pointer-in-stale-stack-frame @gol
-Wno-analyzer-use-of-uninitialized-value @gol
+-Wno-analyzer-write-to-const @gol
+-Wno-analyzer-write-to-string-literal @gol
}
@item C and Objective-C-only Warning Options
@@ -8801,6 +8803,8 @@ Enabling this option effectively enables the following warnings:
-Wanalyzer-unsafe-call-within-signal-handler @gol
-Wanalyzer-use-after-free @gol
-Wanalyzer-use-of-pointer-in-stale-stack-frame @gol
+-Wanalyzer-write-to-const @gol
+-Wanalyzer-write-to-string-literal @gol
}
This option is only available if GCC was configured with analyzer
@@ -8983,6 +8987,30 @@ to disable it.
This diagnostic warns for paths through the code in which a pointer
is dereferenced that points to a variable in a stale stack frame.
+@item -Wno-analyzer-write-to-const
+@opindex Wanalyzer-write-to-const
+@opindex Wno-analyzer-write-to-const
+This warning requires @option{-fanalyzer}, which enables it; use
+@option{-Wno-analyzer-write-to-const}
+to disable it.
+
+This diagnostic warns for paths through the code in which the analyzer
+detects an attempt to write through a pointer to a @code{const} object.
+However, the analyzer does not prioritize detection of such paths, so
+false negatives are more likely relative to other warnings.
+
+@item -Wno-analyzer-write-to-string-literal
+@opindex Wanalyzer-write-to-string-literal
+@opindex Wno-analyzer-write-to-string-literal
+This warning requires @option{-fanalyzer}, which enables it; use
+@option{-Wno-analyzer-write-to-string-literal}
+to disable it.
+
+This diagnostic warns for paths through the code in which the analyzer
+detects an attempt to write through a pointer to a string literal.
+However, the analyzer does not prioritize detection of such paths, so
+false negatives are more likely relative to other warnings.
+
@end table
Pertinent parameters for controlling the exploration are:
diff --git a/gcc/testsuite/gcc.dg/analyzer/write-to-const-1.c b/gcc/testsuite/gcc.dg/analyzer/write-to-const-1.c
new file mode 100644
index 00000000000..dc724e29185
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/write-to-const-1.c
@@ -0,0 +1,29 @@
+/* PR middle-end/90404 */
+
+const int c1 = 20; /* { dg-message "declared here" } */
+int test_1 (void)
+{
+ *((int*) &c1) = 10; /* { dg-warning "write to 'const' object 'c1'" } */
+ return c1;
+}
+
+/* Example of writing to a subregion (an element within a const array). */
+
+const int c2[10]; /* { dg-message "declared here" } */
+int test_2 (void)
+{
+ ((int*) &c2)[5] = 10; /* { dg-warning "write to 'const' object 'c2'" } */
+ return c2[5];
+}
+
+const char s3[] = "012.45"; /* { dg-message "declared here" } */
+int test_3 (void)
+{
+ char *p = __builtin_strchr (s3, '.');
+ *p = 0; /* { dg-warning "write to 'const' object 's3'" } */
+
+ if (__builtin_strlen (p) != 3)
+ __builtin_abort ();
+
+ return s3[3] == 0;
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/write-to-string-literal-1.c b/gcc/testsuite/gcc.dg/analyzer/write-to-string-literal-1.c
new file mode 100644
index 00000000000..092500e066f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/write-to-string-literal-1.c
@@ -0,0 +1,58 @@
+#include <string.h>
+
+/* PR analyzer/95007. */
+
+void test_1 (void)
+{
+ char *s = "foo";
+ s[0] = 'g'; /* { dg-warning "write to string literal" } */
+}
+
+/* PR c/83347. */
+
+void test_2 (void)
+{
+ memcpy ("abc", "def", 3); /* { dg-warning "write to string literal" } */
+}
+
+static char * __attribute__((noinline))
+called_by_test_3 (void)
+{
+ return (char *)"foo";
+}
+
+void test_3 (void)
+{
+ char *s = called_by_test_3 ();
+ s[1] = 'a'; /* { dg-warning "write to string literal" } */
+}
+
+static char * __attribute__((noinline))
+called_by_test_4 (int flag)
+{
+ if (flag)
+ return (char *)"foo";
+ else
+ return (char *)"bar";
+}
+
+void test_4 (void)
+{
+ char *s = called_by_test_4 (0);
+ s[1] = 'z'; /* { dg-warning "write to string literal" } */
+}
+
+static char * __attribute__((noinline))
+called_by_test_5 (int flag)
+{
+ if (flag)
+ return (char *)"foo";
+ else
+ return (char *)"bar";
+}
+
+void test_5 (int flag)
+{
+ char *s = called_by_test_5 (flag);
+ s[1] = 'z'; /* We miss this one, unless we disable state merging. */
+}
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2020-10-12 16:06 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-12 16:06 [gcc r11-3829] analyzer: add warnings about writes to constant regions [PR95007] David Malcolm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).