diff mbox

Fix libcpp ICE (PR preprocessor/60736)

Message ID 20151118230225.GO5675@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 18, 2015, 11:02 p.m. UTC 
Hi!

In some cases, e.g. when using #include <stdc-predef.h> with
/usr/include/ not containing that header, open_file_failed is called
with NULL file->path; trying to print that doesn't really work, we should
use file->name in that case instead.  Though in other cases open_file_failed
is called with both file->name and file->path non-NULL and in that case
file->path is preferrable for the diagnostics.

While looking at this, I've noticed a localization problem.
cpp_errno function hasn't been using _() on the message it printed,
actually, it would localize the "%s: %s" string and not localize
the first string and localize the second one (xstrerror).
As the argument is called msgid, the arguments have been extracted
into the *.pot files.
Furthermore, cpp_errno has been called in some cases with a string literal,
which should be translated, and in other cases with a filename, which should
not be translated.

So, appart from fixing the first issue by using file->path ? file->path : file->name
in 3 spots instead of just file->path, the patch introduces a new function
like cpp_errno that does not translate the argument (meant to be used with
filenames) and cpp_errno itself (which localizes the argument).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/5.3?

2015-11-18  Jakub Jelinek  <jakub@redhat.com>

	PR preprocessor/60736
	* include/cpplib.h (cpp_errno_filename): New prototype.
	* errors.c (cpp_errno): Don't handle msgid "" specially, use
	_(msgid) instead of msgid as argument to cpp_error.
	(cpp_errno_filename): New function.
	* files.c (read_file_guts): Use cpp_errno_filename instead of
	cpp_errno.
	(open_file_failed): Likewise.  Use file->name if file->path is NULL
	in diagnostics.


	Jakub

Comments

Bernd Schmidt Nov. 19, 2015, 12:41 a.m. UTC | #1 
On 11/19/2015 12:02 AM, Jakub Jelinek wrote:
> 2015-11-18  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR preprocessor/60736
> 	* include/cpplib.h (cpp_errno_filename): New prototype.
> 	* errors.c (cpp_errno): Don't handle msgid "" specially, use
> 	_(msgid) instead of msgid as argument to cpp_error.
> 	(cpp_errno_filename): New function.
> 	* files.c (read_file_guts): Use cpp_errno_filename instead of
> 	cpp_errno.
> 	(open_file_failed): Likewise.  Use file->name if file->path is NULL
> 	in diagnostics.

Ok. Is it worth it to put Ian's mini-testcase of "#include 
<stdc-predef.h>" into the testsuite?


Bernd
Jakub Jelinek Nov. 19, 2015, 12:55 a.m. UTC | #2 
On Thu, Nov 19, 2015 at 01:41:52AM +0100, Bernd Schmidt wrote:
> On 11/19/2015 12:02 AM, Jakub Jelinek wrote:
> >2015-11-18  Jakub Jelinek  <jakub@redhat.com>
> >
> >	PR preprocessor/60736
> >	* include/cpplib.h (cpp_errno_filename): New prototype.
> >	* errors.c (cpp_errno): Don't handle msgid "" specially, use
> >	_(msgid) instead of msgid as argument to cpp_error.
> >	(cpp_errno_filename): New function.
> >	* files.c (read_file_guts): Use cpp_errno_filename instead of
> >	cpp_errno.
> >	(open_file_failed): Likewise.  Use file->name if file->path is NULL
> >	in diagnostics.
> 
> Is it worth it to put Ian's mini-testcase of "#include <stdc-predef.h>"
> into the testsuite?

Well, it behaves differently when stdc-predef.h is present in /usr/include
and when it is not, so it would have to say dg-prune-output the diagnostics
or something similar.

	Jakub
diff mbox

Patch

--- libcpp/include/cpplib.h.jj	2015-11-14 19:36:30.000000000 +0100
+++ libcpp/include/cpplib.h	2015-11-18 15:27:47.668327139 +0100
@@ -986,6 +986,9 @@  extern bool cpp_warning_syshdr (cpp_read
 /* Output a diagnostic with "MSGID: " preceding the
    error string of errno.  No location is printed.  */
 extern bool cpp_errno (cpp_reader *, int, const char *msgid);
+/* Similarly, but with "FILENAME: " instead of "MSGID: ", where
+   the filename is not localized.  */
+extern bool cpp_errno_filename (cpp_reader *, int, const char *filename);
 
 /* Same as cpp_error, except additionally specifies a position as a
    (translation unit) physical line and physical column.  If the line is
--- libcpp/errors.c.jj	2015-11-14 19:36:30.000000000 +0100
+++ libcpp/errors.c	2015-11-18 15:36:57.091571158 +0100
@@ -230,8 +230,18 @@  cpp_warning_with_line_syshdr (cpp_reader
 bool
 cpp_errno (cpp_reader *pfile, int level, const char *msgid)
 {
-  if (msgid[0] == '\0')
-    msgid = _("stdout");
+  return cpp_error (pfile, level, "%s: %s", _(msgid), xstrerror (errno));
+}
+
+/* Print a warning or error, depending on the value of LEVEL.  Include
+   information from errno.  Unlike cpp_errno, the argument is a filename
+   that is not localized, but "" is replaced with localized "stdout".  */
+
+bool
+cpp_errno_filename (cpp_reader *pfile, int level, const char *filename)
+{
+  if (filename[0] == '\0')
+    filename = _("stdout");
 
-  return cpp_error (pfile, level, "%s: %s", msgid, xstrerror (errno));
+  return cpp_error (pfile, level, "%s: %s", filename, xstrerror (errno));
 }
--- libcpp/files.c.jj	2015-05-19 15:52:28.000000000 +0200
+++ libcpp/files.c	2015-11-18 15:28:25.749789558 +0100
@@ -715,7 +715,7 @@  read_file_guts (cpp_reader *pfile, _cpp_
 
   if (count < 0)
     {
-      cpp_errno (pfile, CPP_DL_ERROR, file->path);
+      cpp_errno_filename (pfile, CPP_DL_ERROR, file->path);
       free (buf);
       return false;
     }
@@ -1053,7 +1053,8 @@  open_file_failed (cpp_reader *pfile, _cp
       /* If the preprocessor output (other than dependency information) is
          being used, we must also flag an error.  */
       if (CPP_OPTION (pfile, deps.need_preprocessor_output))
-	cpp_errno (pfile, CPP_DL_FATAL, file->path);
+	cpp_errno_filename (pfile, CPP_DL_FATAL,
+			    file->path ? file->path : file->name);
     }
   else
     {
@@ -1067,9 +1068,11 @@  open_file_failed (cpp_reader *pfile, _cp
       if (CPP_OPTION (pfile, deps.style) == DEPS_NONE
           || print_dep
           || CPP_OPTION (pfile, deps.need_preprocessor_output))
-	cpp_errno (pfile, CPP_DL_FATAL, file->path);
+	cpp_errno_filename (pfile, CPP_DL_FATAL,
+			    file->path ? file->path : file->name);
       else
-	cpp_errno (pfile, CPP_DL_WARNING, file->path);
+	cpp_errno_filename (pfile, CPP_DL_WARNING,
+			    file->path ? file->path : file->name);
     }
 }