Add known_function handlers for fdopen and freopen. Both check that
the mode string is null-terminated, and both participate in the
FILE-pointer state machine so the analyzer can detect leaks and
double-fclose.
For freopen, the handler returns the stream argument (since freopen
returns the same stream pointer on success). This is an initial
implementation: success/failure bifurcation is not yet implemented, so
the failure path where the original stream is closed cannot be modeled
without false positives.
Also move fopen registration from the POSIX section to the C standard
section, and fix minor formatting issues in kf_fopen.
Bootstrapped and tested on x86_64-pc-linux-gnu.
gcc/analyzer/ChangeLog:
* kf.cc (class kf_fopen): Fix formatting.
(class kf_fdopen): New known_function handler.
(class kf_freopen): New known_function handler.
(register_known_functions): Register kf_fdopen and kf_freopen.
Move fopen from POSIX to C standard section.
* sm-file.cc (fileptr_state_machine::on_stmt): Handle fdopen
and freopen in the FILE-pointer state machine.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/fopen-1.c: Add test_null_mode.
* gcc.dg/analyzer/fdopen-1.c: New test.
* gcc.dg/analyzer/freopen-1.c: New test.
Signed-off-by: Tomas Ortin Fernandez (quanrong) <[email protected]>
---
gcc/analyzer/kf.cc | 78 ++++++++++++++-
gcc/analyzer/sm-file.cc | 4 +-
gcc/testsuite/gcc.dg/analyzer/fdopen-1.c | 57 +++++++++++
gcc/testsuite/gcc.dg/analyzer/fopen-1.c | 8 ++
gcc/testsuite/gcc.dg/analyzer/freopen-1.c | 114 ++++++++++++++++++++++
5 files changed, 255 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/fdopen-1.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/freopen-1.c
diff --git a/gcc/analyzer/kf.cc b/gcc/analyzer/kf.cc
index 0434ab0f0bf..63365f2ef91 100644
--- a/gcc/analyzer/kf.cc
+++ b/gcc/analyzer/kf.cc
@@ -530,16 +530,19 @@ kf_error::impl_call_pre (const call_details &cd) const
class kf_fopen : public known_function
{
public:
- bool matches_call_types_p (const call_details &cd) const final override
+ bool
+ matches_call_types_p (const call_details &cd) const final override
{
- return (cd.num_args () == 2
- && cd.arg_is_pointer_p (0)
+ return (cd.num_args () == 2 && cd.arg_is_pointer_p (0)
&& cd.arg_is_pointer_p (1));
}
- void impl_call_pre (const call_details &cd) const final override
+ void
+ impl_call_pre (const call_details &cd) const final override
{
cd.check_for_null_terminated_string_arg (0);
+ /* TODO: mode must not be NULL, but
check_for_null_terminated_string_arg
+ doesn't yet warn on NULL pointers. */
cd.check_for_null_terminated_string_arg (1);
cd.set_any_lhs_with_defaults ();
@@ -548,6 +551,69 @@ public:
}
};
+/* Handler for "fdopen":
+ FILE *fdopen (int fd, const char *mode); */
+
+class kf_fdopen : public known_function
+{
+public:
+ bool
+ matches_call_types_p (const call_details &cd) const final override
+ {
+ return (cd.num_args () == 2 && cd.arg_is_integral_p (0)
+ && cd.arg_is_pointer_p (1));
+ }
+
+ void
+ impl_call_pre (const call_details &cd) const final override
+ {
+ /* TODO: check that fd is valid: warn on negative constants and
+ integrate with the fd state machine to detect use of closed or
+ invalid file descriptors. */
+ /* TODO: mode must not be NULL, but
check_for_null_terminated_string_arg
+ doesn't yet warn on NULL pointers. */
+ cd.check_for_null_terminated_string_arg (1);
+ cd.set_any_lhs_with_defaults ();
+ }
+};
+
+/* Handler for "freopen":
+ FILE *freopen (const char *path, const char *mode, FILE *stream);
+
+ The path argument can be NULL (reopen same file with new mode).
+
+ On success freopen returns stream. On failure it returns NULL and
+ the original stream is closed. Without success/failure bifurcation
+ we cannot distinguish these paths, so we do not transition stream
+ to "closed" in the state machine (doing so would cause false
+ positives on the success path where stream is still usable).
+
+ TODO: add success/failure bifurcation so that the failure path can
+ transition stream to "closed." */
+
+class kf_freopen : public 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_pointer_p (2));
+ }
+
+ void
+ impl_call_pre (const call_details &cd) const final override
+ {
+ /* path (arg 0) may be NULL (reopen same file with new mode). */
+ if (!zerop (cd.get_arg_tree (0)))
+ cd.check_for_null_terminated_string_arg (0);
+ /* TODO: mode must not be NULL, but
check_for_null_terminated_string_arg
+ doesn't yet warn on NULL pointers. */
+ cd.check_for_null_terminated_string_arg (1);
+ cd.maybe_set_lhs (cd.get_arg_svalue (2));
+ }
+};
+
/* Handler for "free", after sm-handling.
If the ptr points to an underlying heap region, delete the region,
@@ -2942,8 +3008,10 @@ register_known_functions (known_function_manager
&kfm,
kfm.add ("__builtin_alloca", std::make_unique<kf_alloca> ());
kfm.add ("calloc", std::make_unique<kf_calloc> ());
kfm.add ("__builtin_calloc", std::make_unique<kf_calloc> ());
+ kfm.add ("fopen", std::make_unique<kf_fopen> ());
kfm.add ("free", std::make_unique<kf_free> ());
kfm.add ("__builtin_free", std::make_unique<kf_free> ());
+ kfm.add ("freopen", std::make_unique<kf_freopen> ());
kfm.add ("malloc", std::make_unique<kf_malloc> ());
kfm.add ("__builtin_malloc", std::make_unique<kf_malloc> ());
kfm.add ("memcpy",
@@ -2994,7 +3062,7 @@ register_known_functions (known_function_manager &kfm,
/* Known POSIX functions, and some non-standard extensions. */
{
- kfm.add ("fopen", std::make_unique<kf_fopen> ());
+ kfm.add ("fdopen", std::make_unique<kf_fdopen> ());
kfm.add ("mkdtemp", std::make_unique<kf_mktemp_simple> (
kf_mktemp_family::outcome::null_ptr));
kfm.add ("mkostemp", std::make_unique<kf_mkostemp> ());
diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc
index b8a0e2bda78..980ac005610 100644
--- a/gcc/analyzer/sm-file.cc
+++ b/gcc/analyzer/sm-file.cc
@@ -410,7 +410,9 @@ fileptr_state_machine::on_stmt (sm_context &sm_ctxt,
if (const gcall *call = dyn_cast <const gcall *> (stmt))
if (tree callee_fndecl = sm_ctxt.get_fndecl_for_call (*call))
{
- if (is_named_call_p (callee_fndecl, "fopen", *call, 2))
+ if (is_named_call_p (callee_fndecl, "fopen", *call, 2)
+ || is_named_call_p (callee_fndecl, "fdopen", *call, 2)
+ || is_named_call_p (callee_fndecl, "freopen", *call, 3))
{
tree lhs = gimple_call_lhs (call);
if (lhs)
diff --git a/gcc/testsuite/gcc.dg/analyzer/fdopen-1.c
b/gcc/testsuite/gcc.dg/analyzer/fdopen-1.c
new file mode 100644
index 00000000000..e4213ba3d99
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/fdopen-1.c
@@ -0,0 +1,57 @@
+/* { dg-prune-output "-Wuse-after-free" } */
+
+#include "analyzer-decls.h"
+
+typedef struct FILE FILE;
+FILE *fdopen (int fd, const char *mode);
+int fclose (FILE *);
+
+FILE *
+test_passthrough (int fd, const char *mode)
+{
+ return fdopen (fd, mode);
+}
+
+FILE *
+test_null_mode (int fd)
+{
+ return fdopen (fd, NULL); /* TODO: should warn about NULL mode. */
+}
+
+FILE *
+test_simple_r (int fd)
+{
+ return fdopen (fd, "r");
+}
+
+FILE *
+test_unterminated_mode (int fd)
+{
+ char buf[3] = "abc";
+ return fdopen (fd, buf); /* { dg-warning "stack-based buffer
over-read" } */
+ /* { dg-message "while looking for null terminator for argument 2
\\('&buf'\\) of 'fdopen'..." "event" { target *-*-* } .-1 } */
+}
+
+FILE *
+test_uninitialized_mode (int fd)
+{
+ char buf[10];
+ return fdopen (fd, buf); /* { dg-warning "use of uninitialized value
'buf\\\[0\\\]'" } */
+ /* { dg-message "while looking for null terminator for argument 2
\\('&buf'\\) of 'fdopen'..." "event" { target *-*-* } .-1 } */
+}
+
+void
+test_leak (int fd)
+{
+ FILE *f = fdopen (fd, "r");
+} /* { dg-warning "leak of FILE 'f'" } */
+
+void
+test_double_fclose (int fd)
+{
+ FILE *f = fdopen (fd, "r");
+ if (!f)
+ return;
+ fclose (f);
+ fclose (f); /* { dg-warning "double 'fclose' of FILE 'f'" } */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/fopen-1.c
b/gcc/testsuite/gcc.dg/analyzer/fopen-1.c
index f59d9efd3e1..2c7177374fa 100644
--- a/gcc/testsuite/gcc.dg/analyzer/fopen-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/fopen-1.c
@@ -2,9 +2,17 @@
string too long.
C++ compatible parts have been moved into
c-c++-common/analyzer/fopen-2.c */
+#include "analyzer-decls.h"
+
typedef struct FILE FILE;
FILE *fopen (const char *pathname, const char *mode);
+FILE *
+test_null_mode (const char *pathname)
+{
+ return fopen (pathname, NULL); /* TODO: should warn about NULL mode. */
+}
+
FILE *
test_unterminated_pathname (const char *mode)
{
diff --git a/gcc/testsuite/gcc.dg/analyzer/freopen-1.c
b/gcc/testsuite/gcc.dg/analyzer/freopen-1.c
new file mode 100644
index 00000000000..d7b8d1a4331
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/freopen-1.c
@@ -0,0 +1,114 @@
+/* { dg-prune-output "-Wuse-after-free" } */
+
+#include "analyzer-decls.h"
+
+typedef struct FILE FILE;
+FILE *freopen (const char *pathname, const char *mode, FILE *stream);
+int fclose (FILE *);
+int feof (FILE *);
+
+FILE *
+test_passthrough (const char *pathname, const char *mode, FILE *stream)
+{
+ return freopen (pathname, mode, stream);
+}
+
+FILE *
+test_null_pathname (const char *mode, FILE *stream)
+{
+ return freopen (NULL, mode, stream);
+}
+
+FILE *
+test_null_mode (const char *pathname, FILE *stream)
+{
+ return freopen (pathname, NULL, stream); /* TODO: should warn about
NULL mode. */
+}
+
+FILE *
+test_simple_r (const char *pathname, FILE *stream)
+{
+ return freopen (pathname, "r", stream);
+}
+
+FILE *
+test_swapped_args (FILE *stream)
+{
+ return freopen ("r", "foo.txt", stream); /* TODO: would be nice to
detect this. */
+}
+
+FILE *
+test_unterminated_mode (const char *pathname, FILE *stream)
+{
+ char buf[3] = "abc";
+ return freopen (pathname, buf, stream); /* { dg-warning "stack-based
buffer over-read" } */
+ /* { dg-message "while looking for null terminator for argument 2
\\('&buf'\\) of 'freopen'..." "event" { target *-*-* } .-1 } */
+}
+
+FILE *
+test_uninitialized_pathname (const char *mode, FILE *stream)
+{
+ char buf[10];
+ return freopen (buf, mode, stream); /* { dg-warning "use of
uninitialized value 'buf\\\[0\\\]'" } */
+ /* { dg-message "while looking for null terminator for argument 1
\\('&buf'\\) of 'freopen'..." "event" { target *-*-* } .-1 } */
+}
+
+FILE *
+test_uninitialized_mode (const char *pathname, FILE *stream)
+{
+ char buf[10];
+ return freopen (pathname, buf, stream); /* { dg-warning "use of
uninitialized value 'buf\\\[0\\\]'" } */
+ /* { dg-message "while looking for null terminator for argument 2
\\('&buf'\\) of 'freopen'..." "event" { target *-*-* } .-1 } */
+}
+
+void
+test_no_leak (const char *pathname, const char *mode, FILE *stream)
+{
+ /* Not a leak, f is the same as stream. */
+ FILE *f = freopen (pathname, mode, stream);
+}
+
+void
+test_double_fclose (const char *pathname, const char *mode, FILE *stream)
+{
+ FILE *f = freopen (pathname, mode, stream);
+ if (!f)
+ return;
+ fclose (f);
+ fclose (f); /* { dg-warning "double 'fclose' of FILE 'f'" } */
+}
+
+void
+test_fclose_old_stream (const char *pathname, const char *mode, FILE
*stream)
+{
+ FILE *f = freopen (pathname, mode, stream);
+ if (!f)
+ return;
+ /* f and stream alias the same FILE object, so this is the first
close. */
+ fclose (stream);
+ fclose (f); /* { dg-warning "double 'fclose' of FILE 'f'" } */
+}
+
+void
+test_freopen_after_fclose (const char *pathname, const char *mode,
+ FILE *stream)
+{
+ fclose (stream);
+ /* TODO: should warn about passing a closed stream to freopen. */
+ FILE *f = freopen (pathname, mode, stream);
+ if (f)
+ fclose (f); /* TODO: should warn, but the LHS transition overwrites
+ stream's m_closed state, needs success/failure
+ bifurcation. */
+}
+
+/* f and stream alias the same FILE object, so using stream here is
valid. */
+void
+test_use_old_stream (const char *pathname, const char *mode, FILE *stream)
+{
+ FILE *f = freopen (pathname, mode, stream);
+ if (!f)
+ return;
+ feof (stream);
+ fclose (f);
+}
--
2.52.0
