[PATCH] genksyms: Duplicate function pointer type definitions segfault

[PATCH] genksyms: Duplicate function pointer type definitions segfault

[Richard Yao]

From: Richard Yao <richard.yao@clusterhq.com>

I noticed that genksyms will segfault when it sees duplicate function
pointer type declaration when I placed the same function pointer
definition in two separate headers in a local branch as an intermediate
step of some refactoring. This can be reproduced by piping the following
minimal test case into `genksyms -r /dev/null` or alternatively, putting
it into a C file attempting a build:

int (*f)();
int (*f)();

Attaching gdb to genksyms to understand this failure is useless without
changing CFLAGS to emit debuginfo. Once you have debuginfo, you will
find that the failure is that `char *s` was NULL and the program
executed `while(*s)`. At which point, further debugging requires
familiarity with compiler front end / parser development.

What happens is that flex identifies the first instance of the token "f"
as IDENT and the yacc parser adds it to the symbol table. On the second
instance, flex will identify "f" as TYPE, which triggers an error case
in the yacc parser. Given that TYPE would have been IDENT had it not
been in the symbol table, the the segmentaion fault could be avoided by
treating TYPE as IDENT in the affected rule.

Some might consider placing identical function pointer type declarations
in different headers to be poor style might consider a failure to be
beneficial. However, failing through a segmentation fault makes the
cause non-obvious and can waste the time of anyone who encounters it.

Signed-off-by: Richard Yao <richard.yao@clusterhq.com>
Acked-by: Madhuri Yechuri <madhuriyechuri@clusterhq.com>
---
 scripts/genksyms/parse.y | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/scripts/genksyms/parse.y b/scripts/genksyms/parse.y
index b9f4cf2..723ab30 100644
--- a/scripts/genksyms/parse.y
+++ b/scripts/genksyms/parse.y
@@ -303,6 +303,15 @@ direct_declarator:
 		    $$ = $1;
 		  }
 		}
+	| TYPE
+		{ if (current_name != NULL) {
+		    error_with_pos("unexpected second declaration name");
+		    YYERROR;
+		  } else {
+		    current_name = (*$1)->string;
+		    $$ = $1;
+		  }
+		}
 	| direct_declarator '(' parameter_declaration_clause ')'
 		{ $$ = $4; }
 	| direct_declarator '(' error ')'
-- 
2.3.6

Re: [PATCH] genksyms: Duplicate function pointer type definitions segfault

[Richard Yao]

On 20 July 2015 at 19:52, Richard Yao <ryao@gentoo.org> wrote:
>
> From: Richard Yao <richard.yao@clusterhq.com>
>
> I noticed that genksyms will segfault when it sees duplicate function
> pointer type declaration when I placed the same function pointer
> definition in two separate headers in a local branch as an intermediate
> step of some refactoring. This can be reproduced by piping the following
> minimal test case into `genksyms -r /dev/null` or alternatively, putting
> it into a C file attempting a build:
>
> int (*f)();
> int (*f)();


There is a typo in the commit message. This should have been:

typedef int (*f)();
typedef int (*f)();
>
>
> Attaching gdb to genksyms to understand this failure is useless without
> changing CFLAGS to emit debuginfo. Once you have debuginfo, you will
> find that the failure is that `char *s` was NULL and the program
> executed `while(*s)`. At which point, further debugging requires
> familiarity with compiler front end / parser development.
>
> What happens is that flex identifies the first instance of the token "f"
> as IDENT and the yacc parser adds it to the symbol table. On the second
> instance, flex will identify "f" as TYPE, which triggers an error case
> in the yacc parser. Given that TYPE would have been IDENT had it not
> been in the symbol table, the the segmentaion fault could be avoided by
> treating TYPE as IDENT in the affected rule.
>
> Some might consider placing identical function pointer type declarations
> in different headers to be poor style might consider a failure to be
> beneficial. However, failing through a segmentation fault makes the
> cause non-obvious and can waste the time of anyone who encounters it.
>
> Signed-off-by: Richard Yao <richard.yao@clusterhq.com>
> Acked-by: Madhuri Yechuri <madhuriyechuri@clusterhq.com>
> ---
>  scripts/genksyms/parse.y | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/scripts/genksyms/parse.y b/scripts/genksyms/parse.y
> index b9f4cf2..723ab30 100644
> --- a/scripts/genksyms/parse.y
> +++ b/scripts/genksyms/parse.y
> @@ -303,6 +303,15 @@ direct_declarator:
>                     $$ = $1;
>                   }
>                 }
> +       | TYPE
> +               { if (current_name != NULL) {
> +                   error_with_pos("unexpected second declaration name");
> +                   YYERROR;
> +                 } else {
> +                   current_name = (*$1)->string;
> +                   $$ = $1;
> +                 }
> +               }
>         | direct_declarator '(' parameter_declaration_clause ')'
>                 { $$ = $4; }
>         | direct_declarator '(' error ')'
> --
> 2.3.6
>

Re: [PATCH] genksyms: Duplicate function pointer type definitions segfault

[Michal Marek]

On 2015-07-21 01:52, Richard Yao wrote:
> From: Richard Yao <richard.yao@clusterhq.com>
> 
> I noticed that genksyms will segfault when it sees duplicate function
> pointer type declaration when I placed the same function pointer
> definition in two separate headers in a local branch as an intermediate
> step of some refactoring. This can be reproduced by piping the following
> minimal test case into `genksyms -r /dev/null` or alternatively, putting
> it into a C file attempting a build:
> 
> int (*f)();
> int (*f)();
> 
> Attaching gdb to genksyms to understand this failure is useless without
> changing CFLAGS to emit debuginfo. Once you have debuginfo, you will
> find that the failure is that `char *s` was NULL and the program
> executed `while(*s)`. At which point, further debugging requires
> familiarity with compiler front end / parser development.
> 
> What happens is that flex identifies the first instance of the token "f"
> as IDENT and the yacc parser adds it to the symbol table. On the second
> instance, flex will identify "f" as TYPE, which triggers an error case
> in the yacc parser. Given that TYPE would have been IDENT had it not
> been in the symbol table, the the segmentaion fault could be avoided by
> treating TYPE as IDENT in the affected rule.
> 
> Some might consider placing identical function pointer type declarations
> in different headers to be poor style might consider a failure to be
> beneficial. However, failing through a segmentation fault makes the
> cause non-obvious and can waste the time of anyone who encounters it.
> 
> Signed-off-by: Richard Yao <richard.yao@clusterhq.com>
> Acked-by: Madhuri Yechuri <madhuriyechuri@clusterhq.com>

Applied to kbuild.git#kbuild.

Michal