From b3d788a2cd35c734a683444c976abe14afc5c1c1 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Fri, 21 Feb 2020 10:50:16 -0500 Subject: [PATCH] analyzer: disable the "taint" checker by default PR analyzer/93032 tracks a false negative where we fail to report FILE * leaks within zlib/contrib/minizip/mztools.c. The underlying issue is a combinatorial explosion of states within the exploded graph. In particular, the state of the "taint" checker is exploding, leading to the analyzer bailing out. I have a patch kit under construction that fixes the state explosion issue enough for the "file" checker to report the leaks, but doing so requires disabling the "taint" checker. Given that the latter is more of a proof-of-concept, this patch disables it by default, to stop it breaking the other checkers. gcc/analyzer/ChangeLog: PR analyzer/93032 * sm.cc (make_checkers): Require the "taint" checker to be explicitly enabled. gcc/ChangeLog: PR analyzer/93032 * doc/invoke.texi (-Wnanalyzer-tainted-array-index): Note that -fanalyzer-checker=taint is also required. (-fanalyzer-checker=): Note that providing this option enables the given checker, and doing so may be required for checkers that are disabled by default. gcc/testsuite/ChangeLog: PR analyzer/93032 * gcc.dg/analyzer/pr93382.c: Add "-fanalyzer-checker=taint". * gcc.dg/analyzer/taint-1.c: Likewise. --- gcc/ChangeLog | 9 +++++++++ gcc/analyzer/ChangeLog | 6 ++++++ gcc/analyzer/sm.cc | 5 ++++- gcc/doc/invoke.texi | 12 +++++++++--- gcc/testsuite/ChangeLog | 6 ++++++ gcc/testsuite/gcc.dg/analyzer/pr93382.c | 2 ++ gcc/testsuite/gcc.dg/analyzer/taint-1.c | 2 ++ 7 files changed, 38 insertions(+), 4 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 2049bf5cf3f..13f0356c9ae 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,12 @@ +2020-02-24 David Malcolm + + PR analyzer/93032 + * doc/invoke.texi (-Wnanalyzer-tainted-array-index): Note that + -fanalyzer-checker=taint is also required. + (-fanalyzer-checker=): Note that providing this option enables the + given checker, and doing so may be required for checkers that are + disabled by default. + 2020-02-24 David Malcolm * doc/invoke.texi (-fanalyzer-verbosity=): "2" only shows diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog index 0882ec6ac3a..7511c2dc92f 100644 --- a/gcc/analyzer/ChangeLog +++ b/gcc/analyzer/ChangeLog @@ -1,3 +1,9 @@ +2020-02-24 David Malcolm + + PR analyzer/93032 + * sm.cc (make_checkers): Require the "taint" checker to be + explicitly enabled. + 2020-02-24 David Malcolm PR analyzer/93899 diff --git a/gcc/analyzer/sm.cc b/gcc/analyzer/sm.cc index e94c691c16c..b1f156fecc9 100644 --- a/gcc/analyzer/sm.cc +++ b/gcc/analyzer/sm.cc @@ -111,7 +111,10 @@ make_checkers (auto_delete_vec &out, logger *logger) { out.safe_push (make_malloc_state_machine (logger)); out.safe_push (make_fileptr_state_machine (logger)); - out.safe_push (make_taint_state_machine (logger)); + /* The "taint" checker must be explicitly enabled (as it currently + leads to state explosions that stop the other checkers working). */ + if (flag_analyzer_checker) + out.safe_push (make_taint_state_machine (logger)); out.safe_push (make_sensitive_state_machine (logger)); out.safe_push (make_signal_state_machine (logger)); diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 183c25602d6..208500c7679 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -6629,8 +6629,9 @@ no longer exists, and likely lead to a crash (or worse). @item -Wno-analyzer-tainted-array-index @opindex Wanalyzer-tainted-array-index @opindex Wno-analyzer-tainted-array-index -This warning requires @option{-fanalyzer}, which enables it; use -@option{-Wno-analyzer-tainted-array-index} to disable it. +This warning requires both @option{-fanalyzer} and +@option{-fanalyzer-checker=taint} to enable it; +use @option{-Wno-analyzer-tainted-array-index} to disable it. This diagnostic warns for paths through the code in which a value that could be under an attacker's control is used as the index @@ -8436,7 +8437,12 @@ call site, and that are sufficiently complicated (as per @item -fanalyzer-checker=@var{name} @opindex fanalyzer-checker -Restrict the analyzer to run just the named checker. +Restrict the analyzer to run just the named checker, and enable it. + +Some checkers are disabled by default (even with @option{-fanalyzer}), +such as the @code{taint} checker that implements +@option{-Wanalyzer-tainted-array-index}, and this option is required +to enable them. @item -fanalyzer-fine-grained @opindex fanalyzer-fine-grained diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index a2844a3b13a..fa451c6d4d5 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2020-02-24 David Malcolm + + PR analyzer/93032 + * gcc.dg/analyzer/pr93382.c: Add "-fanalyzer-checker=taint". + * gcc.dg/analyzer/taint-1.c: Likewise. + 2020-02-24 David Malcolm PR analyzer/93899 diff --git a/gcc/testsuite/gcc.dg/analyzer/pr93382.c b/gcc/testsuite/gcc.dg/analyzer/pr93382.c index 7d18d16e444..dae32f5a2bb 100644 --- a/gcc/testsuite/gcc.dg/analyzer/pr93382.c +++ b/gcc/testsuite/gcc.dg/analyzer/pr93382.c @@ -1,3 +1,5 @@ +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + typedef __SIZE_TYPE__ size_t; int idx; diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-1.c b/gcc/testsuite/gcc.dg/analyzer/taint-1.c index 293ce286819..549e2660284 100644 --- a/gcc/testsuite/gcc.dg/analyzer/taint-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/taint-1.c @@ -1,3 +1,5 @@ +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + #include #include #include -- 2.30.2