Hi Mike,
> Le 30 déc. 2013 à 22:01, Mike Sullivan <mike.sulli...@oracle.com> a écrit :
>
> I'm looking at updating the bison in Solaris and one of the requirements
> is to run an internal tool (parfait) on it to check for problems. It seems
> to have found that while symbol_list_n_get() can return NULL, there are
> a few callers that just dereference the return value and don't check for that.
>
> Here's the output from a run on 2.7.1, but it looks like 3.0.2 hasn't changed
> in that respect:
>
> Error: Null pointer dereference (CWE 476)
> Write to null pointer symbol_list_n_get(effective_rule, n)
> at line 798 of
> components/bison/build/amd64/src/../../../src/scan-code.l in function
> 'handle_action_dollar'.
> Function symbol_list_n_get may return constant 'NULL' at line 193,
> called at line 798.
> Null pointer introduced at line 193 of
> components/bison/build/amd64/src/symlist.c in function 'symbol_list_n_get'.
> Error: Null pointer dereference (CWE 476)
> Read from null pointer symbol_list_n_get(...)
> at line 268 of components/bison/build/amd64/src/reader.c in function
> 'symbol_should_be_used'.
> Function symbol_list_n_get may return constant 'NULL' at line 193,
> called at line 268.
> Null pointer introduced at line 193 of
> components/bison/build/amd64/src/symlist.c in function 'symbol_list_n_get'.
> Error: Null pointer dereference (CWE 476)
> Read from null pointer symbol_list_n_get(...)
> at line 536 of components/bison/build/amd64/src/reader.c in function
> 'packgram'.
> Function symbol_list_n_get may return constant 'NULL' at line 193,
> called at line 536.
> Null pointer introduced at line 193 of
> components/bison/build/amd64/src/symlist.c in function 'symbol_list_n_get'.
>
> from a quick look it does appear to be correct, but I don't know if at the
> times those calls occur that a NULL would ever really be returned. So they
> could be false positives, but it seems worth reporting just in case :)
Thanks for the report. Well, it's good if that's the only
things the tool sees, since I guess it means it also validated
our grammar parser which is written in Bison, so in a way, it
also validated (some of) our generated parsers.
I'm about to install the following patch to address this issue.
Thanks a lot.
commit 401e8068e95da9c34deb92b22649cd00a7d507e8
Author: Akim Demaille <a...@lrde.epita.fr>
Date: Fri Jan 9 14:21:09 2015 +0100
bison: use warnings from static code analysis
A static analysis tool reports that some callers of symbol_list_n_get
might get NULL and not handle it properly. This is not the case, yet
we can suppress this pattern.
Reported by Mike Sullivan.
<https://lists.gnu.org/archive/html/bug-bison/2013-12/msg00027.html>
* src/symlist.c (symbol_list_n_get): Rename as...
(symbol_list_n_get0): this.
Adjust dependencies.
(symbol_list_n_get): New, cannot return 0.
* src/symlist.h: Adjust documentation.
* src/scan-code.l: Style change.
diff --git a/THANKS b/THANKS
index d5feb16..4fc9984 100644
--- a/THANKS
+++ b/THANKS
@@ -91,6 +91,7 @@ Michel d'Hooge michel.dho...@gmail.com
Michiel De Wilde mdewilde.agil...@gmail.com
Mickael Labau laba...@epita.fr
Mike Castle dalg...@ix.netcom.com
+Mike Sullivan mike.sulli...@oracle.com
Neil Booth ne...@earthling.net
Nelson H. F. Beebe be...@math.utah.edu
Nick Bowler nbow...@elliptictech.com
diff --git a/src/scan-code.l b/src/scan-code.l
index f90916c..308d1d0 100644
--- a/src/scan-code.l
+++ b/src/scan-code.l
@@ -711,7 +711,7 @@ handle_action_dollar (symbol_list *rule, char *text,
location dollar_loc)
"]b4_rhs_value(%d, %d, ", effective_rule_length, n);
obstack_quote (&obstack_for_string, type_name);
obstack_sgrow (&obstack_for_string, ")[");
- if (n > 0)
+ if (0 < n)
symbol_list_n_get (effective_rule, n)->action_props.is_value_used =
true;
break;
diff --git a/src/symlist.c b/src/symlist.c
index b58cf5c..89ae29a 100644
--- a/src/symlist.c
+++ b/src/symlist.c
@@ -170,8 +170,10 @@ symbol_list_length (symbol_list const *l)
| Get item N in symbol list L. |
`------------------------------*/
-symbol_list *
-symbol_list_n_get (symbol_list *l, int n)
+/* Same as symbol_list_n_get, but returns 0 on invalid calls. */
+
+static symbol_list *
+symbol_list_n_get0 (symbol_list *l, int n)
{
int i;
@@ -189,6 +191,14 @@ symbol_list_n_get (symbol_list *l, int n)
return l;
}
+symbol_list *
+symbol_list_n_get (symbol_list *l, int n)
+{
+ l = symbol_list_n_get0 (l, n);
+ aver (l);
+ return l;
+}
+
/*--------------------------------------------------------------.
| Get the data type (alternative in the union) of the value for |
diff --git a/src/symlist.h b/src/symlist.h
index 639b593..b63fc51 100644
--- a/src/symlist.h
+++ b/src/symlist.h
@@ -116,7 +116,8 @@ void symbol_list_free (symbol_list *list);
/** Return the length of \c l. */
int symbol_list_length (symbol_list const *l);
-/** Get item \c n in symbol list \c l. */
+/** Get item \c n in symbol list \c l.
+ ** Never returns NULL, aborts if invalid call. */
symbol_list *symbol_list_n_get (symbol_list *l, int n);
/* Get the data type (alternative in the union) of the value for
