From: David Malcolm Date: Wed, 7 Oct 2020 22:34:09 +0000 (-0400) Subject: analyzer: add warnings about writes to constant regions [PR95007] X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=3175d40fc52fb8eb3c3b18cc343d773da24434fb;p=gcc.git 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 --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 +{ +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 % 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 % 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 +{ +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 (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 + +/* 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. */ +}