Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r14-3740-gb51cde34d4e750.
gcc/analyzer/ChangeLog:
PR analyzer/105899
* kf.cc (class kf_strncpy): New.
(kf_strncpy::impl_call_post): New.
(register_known_functions): Register it.
* region-model.cc (region_model::read_bytes): Handle unknown
number of bytes.
gcc/testsuite/ChangeLog:
PR analyzer/105899
* c-c++-common/analyzer/null-terminated-strings-2.c: New test.
* c-c++-common/analyzer/overlapping-buffers.c: Update dg-bogus
directives to avoid clashing with note from <string.h> that might
happen to have the same line number. Add strpncpy test coverage.
* c-c++-common/analyzer/strncpy-1.c: New test.
* gcc.dg/analyzer/null-terminated-strings-1.c
(test_filled_nonzero): New.
(void test_filled_zero): New.
(test_filled_symbolic): New.
---
gcc/analyzer/kf.cc | 182 ++++++++++++++++++
gcc/analyzer/region-model.cc | 2 +
.../analyzer/null-terminated-strings-2.c | 17 ++
.../analyzer/overlapping-buffers.c | 24 ++-
.../c-c++-common/analyzer/strncpy-1.c | 157 +++++++++++++++
.../analyzer/null-terminated-strings-1.c | 24 +++
6 files changed, 398 insertions(+), 8 deletions(-)
create mode 100644
gcc/testsuite/c-c++-common/analyzer/null-terminated-strings-2.c
create mode 100644 gcc/testsuite/c-c++-common/analyzer/strncpy-1.c
diff --git a/gcc/analyzer/kf.cc b/gcc/analyzer/kf.cc
index a62227729991..8a45c329c282 100644
--- a/gcc/analyzer/kf.cc
+++ b/gcc/analyzer/kf.cc
@@ -1375,6 +1375,186 @@ make_kf_strlen ()
return make_unique<kf_strlen> ();
}
+/* Handler for "strncpy" and "__builtin_strncpy".
+ See e.g. https://en.cppreference.com/w/c/string/byte/strncpy
+
+ extern char *strncpy (char *dst, const char *src, size_t count);
+
+ Handle this by splitting into two outcomes:
+ (a) truncated read from "src" of "count" bytes,
+ writing "count" bytes to "dst"
+ (b) read from "src" of up to (and including) the null terminator,
+ where the number of bytes read < "count" bytes,
+ writing those bytes to "dst", and zero-filling the rest,
+ up to "count". */
+
+class kf_strncpy : public builtin_known_function
+{
+public:
+ bool matches_call_types_p (const call_details &cd) const final override
+ {
+ return (cd.num_args () == 3
+ && cd.arg_is_pointer_p (0)
+ && cd.arg_is_pointer_p (1)
+ && cd.arg_is_integral_p (2));
+ }
+ enum built_in_function builtin_code () const final override
+ {
+ return BUILT_IN_STRNCPY;
+ }
+ void impl_call_post (const call_details &cd) const final override;
+};
+
+void
+kf_strncpy::impl_call_post (const call_details &cd) const
+{
+ class strncpy_call_info : public call_info
+ {
+ public:
+ strncpy_call_info (const call_details &cd,
+ const svalue *num_bytes_with_terminator_sval,
+ bool truncated_read)
+ : call_info (cd),
+ m_num_bytes_with_terminator_sval (num_bytes_with_terminator_sval),
+ m_truncated_read (truncated_read)
+ {
+ }
+
+ label_text get_desc (bool can_colorize) const final override
+ {
+ if (m_truncated_read)
+ return make_label_text (can_colorize,
+ "when %qE truncates the source string",
+ get_fndecl ());
+ else
+ return make_label_text (can_colorize,
+ "when %qE copies the full source string",
+ get_fndecl ());
+ }
+
+ bool update_model (region_model *model,
+ const exploded_edge *,
+ region_model_context *ctxt) const final override
+ {
+ const call_details cd (get_call_details (model, ctxt));
+
+ const svalue *dest_sval = cd.get_arg_svalue (0);
+ const region *dest_reg
+ = model->deref_rvalue (dest_sval, cd.get_arg_tree (0), ctxt);
+
+ const svalue *src_sval = cd.get_arg_svalue (1);
+ const region *src_reg
+ = model->deref_rvalue (src_sval, cd.get_arg_tree (1), ctxt);
+
+ const svalue *count_sval = cd.get_arg_svalue (2);
+
+ /* strncpy returns the initial param. */
+ cd.maybe_set_lhs (dest_sval);
+
+ const svalue *num_bytes_read_sval;
+ if (m_truncated_read)
+ {
+ /* Truncated read. */
+ num_bytes_read_sval = count_sval;
+
+ if (m_num_bytes_with_terminator_sval)
+ {
+ /* The terminator is after the limit. */
+ if (!model->add_constraint (m_num_bytes_with_terminator_sval,
+ GT_EXPR,
+ count_sval,
+ ctxt))
+ return false;
+ }
+ else
+ {
+ /* We don't know where the terminator is, or if there is one.
+ In theory we know that the first COUNT bytes are non-zero,
+ but we don't have a way to record that constraint. */
+ }
+ }
+ else
+ {
+ /* Full read of the src string before reaching the limit,
+ so there must be a terminator and it must be at or before
+ the limit. */
+ if (m_num_bytes_with_terminator_sval)
+ {
+ if (!model->add_constraint (m_num_bytes_with_terminator_sval,
+ LE_EXPR,
+ count_sval,
+ ctxt))
+ return false;
+ num_bytes_read_sval = m_num_bytes_with_terminator_sval;
+
+ /* First, zero-fill the dest buffer.
+ We don't need to do this for the truncation case, as
+ this fully populates the dest buffer. */
+ const region *sized_dest_reg
+ = model->get_manager ()->get_sized_region (dest_reg,
+ NULL_TREE,
+ count_sval);
+ model->zero_fill_region (sized_dest_reg, ctxt);
+ }
+ else
+ {
+ /* Don't analyze this case; the other case will
+ assume a "truncated" read up to the limit. */
+ return false;
+ }
+ }
+
+ gcc_assert (num_bytes_read_sval);
+
+ const svalue *bytes_to_copy
+ = model->read_bytes (src_reg,
+ cd.get_arg_tree (1),
+ num_bytes_read_sval,
+ ctxt);
+ cd.complain_about_overlap (0, 1, num_bytes_read_sval);
+ model->write_bytes (dest_reg,
+ num_bytes_read_sval,
+ bytes_to_copy,
+ ctxt);
+
+ return true;
+ }
+ private:
+ /* (strlen + 1) of the source string if it has a terminator,
+ or NULL for the case where UB would happen before
+ finding any terminator. */
+ const svalue *m_num_bytes_with_terminator_sval;
+
+ /* true: if this is the outcome where the limit was reached before
+ the null terminator
+ false: if the null terminator was reached before the limit. */
+ bool m_truncated_read;
+ };
+
+ /* Body of kf_strncpy::impl_call_post. */
+ if (cd.get_ctxt ())
+ {
+ /* First, scan for a null terminator as if there were no limit,
+ with a null ctxt so no errors are reported. */
+ const region_model *model = cd.get_model ();
+ const svalue *ptr_arg_sval = cd.get_arg_svalue (1);
+ const region *buf_reg
+ = model->deref_rvalue (ptr_arg_sval, cd.get_arg_tree (1), nullptr);
+ const svalue *num_bytes_with_terminator_sval
+ = model->scan_for_null_terminator (buf_reg,
+ cd.get_arg_tree (1),
+ nullptr,
+ nullptr);
+ cd.get_ctxt ()->bifurcate
+ (make_unique<strncpy_call_info> (cd, num_bytes_with_terminator_sval,
+ false));
+ cd.get_ctxt ()->bifurcate
+ (make_unique<strncpy_call_info> (cd, num_bytes_with_terminator_sval,
+ true));
+ cd.get_ctxt ()->terminate_path ();
+ }
+};
+
/* Handler for "strndup" and "__builtin_strndup". */
class kf_strndup : public builtin_known_function
@@ -1620,6 +1800,8 @@ register_known_functions (known_function_manager &kfm)
kfm.add ("__builtin___strcat_chk", make_unique<kf_strcat> (3, true));
kfm.add ("strdup", make_unique<kf_strdup> ());
kfm.add ("__builtin_strdup", make_unique<kf_strdup> ());
+ kfm.add ("strncpy", make_unique<kf_strncpy> ());
+ kfm.add ("__builtin_strncpy", make_unique<kf_strncpy> ());
kfm.add ("strndup", make_unique<kf_strndup> ());
kfm.add ("__builtin_strndup", make_unique<kf_strndup> ());
kfm.add ("strlen", make_unique<kf_strlen> ());
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 6be0ad72aaae..999480e55ef7 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -3979,6 +3979,8 @@ region_model::read_bytes (const region *src_reg,
const svalue *num_bytes_sval,
region_model_context *ctxt) const
{
+ if (num_bytes_sval->get_kind () == SK_UNKNOWN)
+ return m_mgr->get_or_create_unknown_svalue (NULL_TREE);
const region *sized_src_reg
= m_mgr->get_sized_region (src_reg, NULL_TREE, num_bytes_sval);
const svalue *src_contents_sval = get_store_value (sized_src_reg, ctxt);
diff --git a/gcc/testsuite/c-c++-common/analyzer/null-terminated-strings-2.c
b/gcc/testsuite/c-c++-common/analyzer/null-terminated-strings-2.c
new file mode 100644
index 000000000000..3bbae821cd46
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/analyzer/null-terminated-strings-2.c
@@ -0,0 +1,17 @@
+#include "../../gcc.dg/analyzer/analyzer-decls.h"
+
+char buf[16];
+
+int main (void)
+{
+ /* We should be able to assume that "buf" is all zeroes here. */
+
+ __analyzer_eval (__analyzer_get_strlen (buf) == 0); /* { dg-warning "TRUE"
"ideal" { xfail *-*-* } } */
+ /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */
+
+ buf[0] = 'a';
+ __analyzer_eval (__analyzer_get_strlen (buf) == 1); /* { dg-warning "TRUE"
"ideal" { xfail *-*-* } } */
+ /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */
+
+ return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/analyzer/overlapping-buffers.c
b/gcc/testsuite/c-c++-common/analyzer/overlapping-buffers.c
index 5808b3304cfc..f02238dfa42c 100644
--- a/gcc/testsuite/c-c++-common/analyzer/overlapping-buffers.c
+++ b/gcc/testsuite/c-c++-common/analyzer/overlapping-buffers.c
@@ -55,7 +55,7 @@ void test_memcpy_symbolic_1 (void *p, size_t n)
void * __attribute__((noinline))
call_memcpy_nonintersecting_concrete_1 (void *dest, const void *src, size_t n)
{
- return memcpy (dest, src, n); /* { dg-bogus "overlapping buffers" } */
+ return memcpy (dest, src, n); /* { dg-bogus "overlapping buffers passed as"
} */
}
void test_memcpy_nonintersecting_concrete_1 (char *p)
@@ -66,7 +66,7 @@ void test_memcpy_nonintersecting_concrete_1 (char *p)
void * __attribute__((noinline))
call_memcpy_nonintersecting_concrete_2 (void *dest, const void *src, size_t n)
{
- return memcpy (dest, src, n); /* { dg-bogus "overlapping buffers" } */
+ return memcpy (dest, src, n); /* { dg-bogus "overlapping buffers passed as"
} */
}
void test_memcpy_nonintersecting_concrete_2 (char *p)
@@ -111,7 +111,7 @@ void test_memcpy_intersecting_symbolic_1 (char *p, size_t n)
void * __attribute__((noinline))
call_memcpy_nonintersecting_symbolic_1 (void *dest, const void *src, size_t n)
{
- return memcpy (dest, src, n); /* { dg-bogus "overlapping buffers" } */
+ return memcpy (dest, src, n); /* { dg-bogus "overlapping buffers passed as"
} */
}
void test_memcpy_nonintersecting_symbolic_1 (char *p, size_t n)
@@ -122,7 +122,7 @@ void test_memcpy_nonintersecting_symbolic_1 (char *p,
size_t n)
void * __attribute__((noinline))
call_memcpy_nonintersecting_symbolic_2 (void *dest, const void *src, size_t n)
{
- return memcpy (dest, src, n); /* { dg-bogus "overlapping buffers" } */
+ return memcpy (dest, src, n); /* { dg-bogus "overlapping buffers passed as"
} */
}
void test_memcpy_nonintersecting_symbolic_2 (char *p, size_t n)
@@ -134,7 +134,7 @@ void test_memcpy_nonintersecting_symbolic_2 (char *p,
size_t n)
void * __attribute__((noinline))
call_memmove_symbolic_1 (void *dest, const void *src, size_t n)
{
- return memmove (dest, src, n); /* { dg-bogus "overlapping buffers" } */
+ return memmove (dest, src, n); /* { dg-bogus "overlapping buffers passed as"
} */
}
void test_memmove_symbolic_1 (void *p, size_t n)
@@ -142,6 +142,14 @@ void test_memmove_symbolic_1 (void *p, size_t n)
call_memmove_symbolic_1 (p, p, n);
}
-/* TODO:
- - strncpy
- */
+static char * __attribute__((noinline))
+call_strncpy_1 (char *dest, const char *src, size_t n)
+{
+ return strncpy (dest, src, n); /* { dg-warning "overlapping buffers" } */
+}
+
+void
+test_strncpy_1 (char *p, size_t n)
+{
+ call_strncpy_1 (p, p, n);
+}
diff --git a/gcc/testsuite/c-c++-common/analyzer/strncpy-1.c
b/gcc/testsuite/c-c++-common/analyzer/strncpy-1.c
new file mode 100644
index 000000000000..3ca1d81d90cb
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/analyzer/strncpy-1.c
@@ -0,0 +1,157 @@
+/* See e.g. https://en.cppreference.com/w/c/string/byte/strncpy */
+
+/* { dg-additional-options "-Wno-stringop-overflow" } */
+/* { dg-additional-options "-fpermissive" { target c++ } } */
+
+#include "../../gcc.dg/analyzer/analyzer-decls.h"
+
+typedef __SIZE_TYPE__ size_t;
+
+extern char *strncpy (char *dst, const char *src, size_t count);
+
+char *
+test_passthrough (char *dst, const char *src, size_t count)
+{
+ char *result = strncpy (dst, src, count);
+ __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */
+ return result;
+}
+
+char *
+test_null_dst (const char *src, size_t count)
+{
+ return strncpy (NULL, src, count); /* { dg-warning "use of NULL where
non-null expected" } */
+}
+
+char *
+test_null_src (char *dst, size_t count)
+{
+ return strncpy (dst, NULL, count); /* { dg-warning "use of NULL where
non-null expected" } */
+}
+
+void
+test_zero_fill (char *dst)
+{
+ strncpy (dst, "", 5);
+ __analyzer_eval (dst[0] == '\0'); /* { dg-warning "TRUE" "correct" { xfail
*-*-* } } */
+ /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */
+ __analyzer_eval (dst[1] == '\0'); /* { dg-warning "TRUE" "correct" { xfail
*-*-* } } */
+ /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */
+ __analyzer_eval (dst[2] == '\0'); /* { dg-warning "TRUE" "correct" { xfail
*-*-* } } */
+ /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */
+ __analyzer_eval (dst[3] == '\0'); /* { dg-warning "TRUE" "correct" { xfail
*-*-* } } */
+ /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */
+ __analyzer_eval (dst[4] == '\0'); /* { dg-warning "TRUE" "correct" { xfail
*-*-* } } */
+ /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */
+ __analyzer_eval (__analyzer_get_strlen (dst) == 0); /* { dg-warning "TRUE" }
*/
+ __analyzer_eval (__analyzer_get_strlen (dst + 1) == 0); /* { dg-warning
"TRUE" "correct" { xfail *-*-* } } */
+ /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */
+}
+
+char *test_unterminated_concrete_a (char *dst)
+{
+ char buf[3] = "abc"; /* { dg-warning "initializer-string for '\[^\n\]*' is
too long" "" { target c++ } } */
+ /* Should be OK to copy nothing. */
+ return strncpy (dst, buf, 0); /* { dg-bogus "" } */
+}
+
+char *test_unterminated_concrete_b (char *dst)
+{
+ char buf[3] = "abc"; /* { dg-warning "initializer-string for '\[^\n\]*' is
too long" "" { target c++ } } */
+ /* Should be OK as the count limits the accesses to valid
+ locations within src buf. */
+ return strncpy (dst, buf, 3); /* { dg-bogus "" } */
+}
+
+char *test_unterminated_concrete_c (char *dst)
+{
+ char buf[3] = "abc"; /* { dg-warning "initializer-string for '\[^\n\]*' is
too long" "" { target c++ } } */
+ /* Should warn: the count is one too high to limit the accesses
+ to within src buf. */
+ return strncpy (dst, buf, 4); /* { dg-warning "stack-based buffer over-read"
} */
+}
+
+char *test_terminated_concrete_d (char *dst)
+{
+ char buf[6];
+ __builtin_memset (buf, 'a', 3);
+ __builtin_memset (buf + 3, 'b', 3);
+
+ /* Shouldn't warn. */
+ return strncpy (dst, buf, 6); /* { dg-bogus "" } */
+}
+
+char *test_unterminated_concrete_e (char *dst)
+{
+ char buf[6];
+ __builtin_memset (buf, 'a', 3);
+ __builtin_memset (buf + 3, 'b', 3);
+
+ /* Should warn. */
+ return strncpy (dst, buf, 7); /* { dg-warning "stack-based buffer over-read"
} */
+}
+
+char *test_unterminated_symbolic (char *dst, size_t count)
+{
+ char buf[3] = "abc"; /* { dg-warning "initializer-string for '\[^\n\]*' is
too long" "" { target c++ } } */
+ return strncpy (dst, buf, count);
+}
+
+char *test_terminated_symbolic (char *dst, size_t count)
+{
+ const char *src = "abc";
+ return strncpy (dst, src, count); /* { dg-bogus "" } */
+}
+
+char *test_uninitialized_concrete_a (char *dst)
+{
+ char buf[16];
+ return strncpy (dst, buf, 0); /* { dg-bogus "" } */
+}
+
+char *test_uninitialized_concrete_b (char *dst)
+{
+ char buf[16];
+ return strncpy (dst, buf, 1); /* { dg-warning "use of uninitialized value" }
*/
+}
+
+char *test_initialized_concrete_c (char *dst)
+{
+ char buf[16];
+ buf[0] = 'a';
+ return strncpy (dst, buf, 1); /* { dg-bogus "" } */
+}
+
+char *test_uninitialized_symbolic (char *dst, size_t count)
+{
+ char buf[16];
+ return strncpy (dst, buf, count); /* { dg-warning "use of uninitialized
value" } */
+}
+
+void test_truncation_1 (const char *src)
+{
+ char buf[16];
+ strncpy (buf, src, 16);
+ /* buf might not be terminated (when strlen(src) > 16). */
+ __analyzer_get_strlen (buf); /* { dg-warning "stack-based buffer over-read"
"" { xfail *-*-* } } */
+}
+
+void test_truncation_2 (size_t count)
+{
+ char buf[16];
+ strncpy (buf, "abc", count);
+ /* buf might not be terminated (when count <= 3). */
+ __analyzer_get_strlen (buf); /* { dg-warning "stack-based buffer over-read"
"" { xfail *-*-* } } */
+}
+
+void test_too_big_concrete (void)
+{
+ char buf[10];
+ strncpy (buf, "abc", 128); /* { dg-warning "stack-based buffer overflow" } */
+}
+
+void test_too_big_symbolic (const char *src)
+{
+ char buf[10];
+ strncpy (buf, src, 128); /* { dg-warning "stack-based buffer overflow" } */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/null-terminated-strings-1.c
b/gcc/testsuite/gcc.dg/analyzer/null-terminated-strings-1.c
index ecd794a2337a..c9095fa3b94a 100644
--- a/gcc/testsuite/gcc.dg/analyzer/null-terminated-strings-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/null-terminated-strings-1.c
@@ -144,3 +144,27 @@ void test_casts (void)
__analyzer_eval (__analyzer_get_strlen (p) == 0); /* { dg-warning "UNKNOWN"
} */
__analyzer_eval (__analyzer_get_strlen (p + 1) == 0); /* { dg-warning
"UNKNOWN" } */
}
+
+void test_filled_nonzero (void)
+{
+ char buf[10];
+ __builtin_memset (buf, 'a', 10);
+ __analyzer_get_strlen (buf); /* { dg-warning "stack-based buffer over-read"
"" { xfail *-*-* } } */
+}
+
+void test_filled_zero (void)
+{
+ char buf[10];
+ __builtin_memset (buf, 0, 10);
+ __analyzer_eval (__analyzer_get_strlen (buf) == 0); /* { dg-warning "TRUE"
"correct" { xfail *-*-* } } */
+ /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */
+ __analyzer_eval (__analyzer_get_strlen (buf + 1) == 0); /* { dg-warning
"TRUE" "correct" { xfail *-*-* } } */
+ /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */
+}
+
+void test_filled_symbolic (int c)
+{
+ char buf[10];
+ __builtin_memset (buf, c, 10);
+ __analyzer_eval (__analyzer_get_strlen (buf) == 0); /* { dg-warning
"UNKNOWN" } */
+}
--
2.26.3
