From b7028f060c6760b336b416897412e327ded12ab5 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Mon, 31 Aug 2020 15:55:45 -0400 Subject: [PATCH] analyzer: stricter handling of non-pure builtins [PR96798] Amongst other things PR analyzer/96798 notes that region_model::on_call_pre treats any builtin that hasn't been coded yet as a no-op (albeit with an unknown return value), which is wrong for non-pure builtins. This patch updates that function's handling of such builtins so that it instead conservatively assumes that any escaped/reachable regions can be affected by the call, and implements enough handling of specific builtins to avoid regressing the testsuite (I hope). gcc/analyzer/ChangeLog: PR analyzer/96798 * region-model-impl-calls.cc (region_model::impl_call_memcpy): New. (region_model::impl_call_strcpy): New. * region-model.cc (region_model::on_call_pre): Flag unhandled builtins that are non-pure as having unknown side-effects. Implement BUILT_IN_MEMCPY, BUILT_IN_MEMCPY_CHK, BUILT_IN_STRCPY, BUILT_IN_STRCPY_CHK, BUILT_IN_FPRINTF, BUILT_IN_FPRINTF_UNLOCKED, BUILT_IN_PUTC, BUILT_IN_PUTC_UNLOCKED, BUILT_IN_FPUTC, BUILT_IN_FPUTC_UNLOCKED, BUILT_IN_FPUTS, BUILT_IN_FPUTS_UNLOCKED, BUILT_IN_FWRITE, BUILT_IN_FWRITE_UNLOCKED, BUILT_IN_PRINTF, BUILT_IN_PRINTF_UNLOCKED, BUILT_IN_PUTCHAR, BUILT_IN_PUTCHAR_UNLOCKED, BUILT_IN_PUTS, BUILT_IN_PUTS_UNLOCKED, BUILT_IN_VFPRINTF, BUILT_IN_VPRINTF. * region-model.h (region_model::impl_call_memcpy): New decl. (region_model::impl_call_strcpy): New decl. gcc/testsuite/ChangeLog: PR analyzer/96798 * gcc.dg/analyzer/memcpy-1.c: New test. * gcc.dg/analyzer/strcpy-1.c: New test. --- gcc/analyzer/region-model-impl-calls.cc | 39 +++++++++++++++++++++ gcc/analyzer/region-model.cc | 36 ++++++++++++++++++++ gcc/analyzer/region-model.h | 2 ++ gcc/testsuite/gcc.dg/analyzer/memcpy-1.c | 43 ++++++++++++++++++++++++ gcc/testsuite/gcc.dg/analyzer/strcpy-1.c | 18 ++++++++++ 5 files changed, 138 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/analyzer/memcpy-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/strcpy-1.c diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc index fa85035b22d..6582ffb3c95 100644 --- a/gcc/analyzer/region-model-impl-calls.cc +++ b/gcc/analyzer/region-model-impl-calls.cc @@ -276,6 +276,30 @@ region_model::impl_call_malloc (const call_details &cd) return true; } +/* Handle the on_call_pre part of "memcpy" and "__builtin_memcpy". */ + +void +region_model::impl_call_memcpy (const call_details &cd) +{ + const svalue *dest_sval = cd.get_arg_svalue (0); + const svalue *num_bytes_sval = cd.get_arg_svalue (2); + + const region *dest_reg = deref_rvalue (dest_sval, cd.get_arg_tree (0), + cd.get_ctxt ()); + + cd.maybe_set_lhs (dest_sval); + + if (tree num_bytes = num_bytes_sval->maybe_get_constant ()) + { + /* "memcpy" of zero size is a no-op. */ + if (zerop (num_bytes)) + return; + } + + /* Otherwise, mark region's contents as unknown. */ + mark_region_as_unknown (dest_reg); +} + /* Handle the on_call_pre part of "memset" and "__builtin_memset". */ bool @@ -353,6 +377,21 @@ region_model::impl_call_operator_delete (const call_details &cd) return false; } +/* Handle the on_call_pre part of "strcpy" and "__builtin_strcpy_chk". */ + +void +region_model::impl_call_strcpy (const call_details &cd) +{ + const svalue *dest_sval = cd.get_arg_svalue (0); + const region *dest_reg = deref_rvalue (dest_sval, cd.get_arg_tree (0), + cd.get_ctxt ()); + + cd.maybe_set_lhs (dest_sval); + + /* For now, just mark region's contents as unknown. */ + mark_region_as_unknown (dest_reg); +} + /* Handle the on_call_pre part of "strlen". Return true if the LHS is updated. */ diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 03c7daf0d1e..75f4eae3083 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -658,6 +658,8 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt) switch (DECL_UNCHECKED_FUNCTION_CODE (callee_fndecl)) { default: + if (!DECL_PURE_P (callee_fndecl)) + unknown_side_effects = true; break; case BUILT_IN_ALLOCA: case BUILT_IN_ALLOCA_WITH_ALIGN: @@ -672,20 +674,54 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt) break; case BUILT_IN_MALLOC: return impl_call_malloc (cd); + case BUILT_IN_MEMCPY: + case BUILT_IN_MEMCPY_CHK: + impl_call_memcpy (cd); + return false; case BUILT_IN_MEMSET: case BUILT_IN_MEMSET_CHK: impl_call_memset (cd); return false; break; + case BUILT_IN_STRCPY: + case BUILT_IN_STRCPY_CHK: + impl_call_strcpy (cd); + return false; case BUILT_IN_STRLEN: if (impl_call_strlen (cd)) return false; break; + + /* Stdio builtins. */ + case BUILT_IN_FPRINTF: + case BUILT_IN_FPRINTF_UNLOCKED: + case BUILT_IN_PUTC: + case BUILT_IN_PUTC_UNLOCKED: + case BUILT_IN_FPUTC: + case BUILT_IN_FPUTC_UNLOCKED: + case BUILT_IN_FPUTS: + case BUILT_IN_FPUTS_UNLOCKED: + case BUILT_IN_FWRITE: + case BUILT_IN_FWRITE_UNLOCKED: + case BUILT_IN_PRINTF: + case BUILT_IN_PRINTF_UNLOCKED: + case BUILT_IN_PUTCHAR: + case BUILT_IN_PUTCHAR_UNLOCKED: + case BUILT_IN_PUTS: + case BUILT_IN_PUTS_UNLOCKED: + case BUILT_IN_VFPRINTF: + case BUILT_IN_VPRINTF: + /* These stdio builtins have external effects that are out + of scope for the analyzer: we only want to model the effects + on the return value. */ + break; } else if (gimple_call_internal_p (call)) switch (gimple_call_internal_fn (call)) { default: + if (!DECL_PURE_P (callee_fndecl)) + unknown_side_effects = true; break; case IFN_BUILTIN_EXPECT: return impl_call_builtin_expect (cd); diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 4707293175a..1bb9798ae58 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -2554,7 +2554,9 @@ class region_model bool impl_call_calloc (const call_details &cd); void impl_call_free (const call_details &cd); bool impl_call_malloc (const call_details &cd); + void impl_call_memcpy (const call_details &cd); bool impl_call_memset (const call_details &cd); + void impl_call_strcpy (const call_details &cd); bool impl_call_strlen (const call_details &cd); bool impl_call_operator_new (const call_details &cd); bool impl_call_operator_delete (const call_details &cd); diff --git a/gcc/testsuite/gcc.dg/analyzer/memcpy-1.c b/gcc/testsuite/gcc.dg/analyzer/memcpy-1.c new file mode 100644 index 00000000000..f120eac19b3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/memcpy-1.c @@ -0,0 +1,43 @@ +#include +#include "analyzer-decls.h" + +void *test_1 (void *dst, void *src, size_t n) +{ + void *result = memcpy (dst, src, n); + __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */ + return result; +} + +void *test_1a (void *dst, void *src, size_t n) +{ + void *result = __memcpy_chk (dst, src, n, -1); + __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */ + return result; +} + +void test_2 (int i) +{ + int j; + memcpy (&j, &i, sizeof (int)); + __analyzer_eval (i == j); /* { dg-warning "TRUE" } */ +} + +void test_2a (int i) +{ + int j; + __memcpy_chk (&j, &i, sizeof (int), sizeof (int)); + __analyzer_eval (i == j); /* { dg-warning "TRUE" } */ +} + +void test_3 (void *src, size_t n) +{ + char buf[40], other[40]; + buf[0] = 'a'; + other[0] = 'b'; + __analyzer_eval (buf[0] == 'a'); /* { dg-warning "TRUE" } */ + __analyzer_eval (other[0] == 'b'); /* { dg-warning "TRUE" } */ + + memcpy (buf, src, n); + __analyzer_eval (buf[0] == 'a'); /* { dg-warning "UNKNOWN" } */ + __analyzer_eval (other[0] == 'b'); /* { dg-warning "TRUE" } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/strcpy-1.c b/gcc/testsuite/gcc.dg/analyzer/strcpy-1.c new file mode 100644 index 00000000000..ed5bab98e4b --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/strcpy-1.c @@ -0,0 +1,18 @@ +#include +#include "analyzer-decls.h" + +char * +test_1 (char *dst, char *src) +{ + char *result = strcpy (dst, src); + __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */ + return result; +} + +char * +test_1a (char *dst, char *src) +{ + char *result = __strcpy_chk (dst, src, -1); + __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */ + return result; +} -- 2.30.2