Re: Fix 69650, bogus line numbers from libcpp

[David Malcolm <dmalcolm@redhat.com>]

On Thu, 2016-03-10 at 09:40 +0100, Bernd Schmidt wrote:
> This is a case where bogus #line directives can confuse libcpp into 
> producing nonsensical line numbers, even leading to a crash later on
> in LTO.
> 
> The following patch moves the test earlier to a point where we can
> more 
> easily recover from the error condition. I should note that I changed
> the raw fprintf (stderr) to a cpp_error call, which is a slight
> change 
> in behaviour (we don't even get to LTO anymore due to erroring out
> earlier).
> 
> Bootstrapped and tested on x86_64-linux (as always including Ada,
> which 
> failed with an earlier version of the patch). Ok?

Thanks for looking at this.

> --- libcpp/directives.c	(revision 234025)
> +++ libcpp/directives.c	(working copy)
> @@ -1046,6 +1046,19 @@ do_linemarker (cpp_reader *pfile)
>  
>    skip_rest_of_line (pfile);
>  
> +  if (reason == LC_LEAVE)
> +    {
> +      const line_map_ordinary *from;      
> +      if (MAIN_FILE_P (map)
> +	  || (new_file
> +	      && (from = INCLUDED_FROM (pfile->line_table, map)) !=
NULL
> +	      && filename_cmp (ORDINARY_MAP_FILE_NAME (from),
new_file) != 0))
> +	{
> +	  cpp_error (pfile, CPP_DL_ERROR,
> +		     "file \"%s\" left but not entered", new_file);
                                                         ^^^^^^^^
Although it looks like you're preserving the existing behavior from
when this was in linemap_add, shouldn't this be
  ORDINARY_MAP_FILE_NAME (from)
rather than new_file?  (i.e. shouldn't it report the name of the file
being *left*, rather than the one being entered?)

[...]

> Index: gcc/testsuite/gcc.dg/pr69650.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/pr69650.c	(revision 0)
> +++ gcc/testsuite/gcc.dg/pr69650.c	(working copy)
> @@ -0,0 +1,5 @@
> +/* { dg-do compile } */
> +/* { dg-options "-std=gnu99" } */
> +
> +# 9 "" 2 /* { dg-error "left but not entered" } */
> +not_a_type a; /* { dg-error "unknown type" } */

Can we also have a testcase with a non-empty filename?  I'm interested
in seeing what the exact error messages looks like.

Also, is it possible to construct a testcase that triggers
the logic in the non-MAIN_FILE_P clause? (perhaps with some
# directives for a variety of "files")?


Hope this is constructive
Dave