[driver] Use regular error routines

Message ID ccafa58b-2edf-f752-284e-91140b40a7b9@acm.org
State New
Headers show
Series
  • [driver] Use regular error routines
Related show

Commit Message

Nathan Sidwell Sept. 11, 2018, 3:01 p.m.
The driver gcc.c has 2 internal error reporting routines 
perror_with_name & pfatal_with_name, which take a single string argument.

Firstly the latter forwards to the former, then calls exit after doing 
some cleanup.  However, that cleanup is already registered via atexit, 
so it'll get done anyway.

More problematic is that the single string doesn't allow context of the 
problem to be shown.  For instance we get:
    nathans@lyta:17>gcc -specs=not-a-file insn-emit.c
    gcc: error: not-a-file: No such file or directory

This patch removes those two functions and just calls error or 
fatal_error as appropriate.  We now get:
    nathans@lyta:16>./xg++ -B./ -specs=not-a-file insn-emit.c
    xg++: fatal error: cannot read spec file 'not-a-file': No such file 
or directory
    compilation terminated.

I'm not sure if pex_run can return a non-null errmsg with zero error 
code, but I've left that alone.  In the case of spec file reading, we 
were already leaving the file handle open on some of the failure cases. 
I'm not making that worse.  Likewise the message in process_command that 
concerns response files could probably be made better, but I'm not sure 
what it's trying to say.

booted & tested on x86_64-linux, installing as obvious.

nathan
-- 
Nathan Sidwell

Comments

Joseph Myers Sept. 11, 2018, 3:58 p.m. | #1
I'd expect all the cases where you pass a string containing '%s' to the 
generic diagnostic functions to use %qs instead of '%s' for quoting, to 
get appropriate locale-specific quotes.

-- 
Joseph S. Myers
joseph@codesourcery.com
Nathan Sidwell Sept. 11, 2018, 9:35 p.m. | #2
On 9/11/18 3:58 PM, Joseph Myers wrote:
> I'd expect all the cases where you pass a string containing '%s' to the

> generic diagnostic functions to use %qs instead of '%s' for quoting, to

> get appropriate locale-specific quotes.


yes of course, slipped my mind.

Committed the attached after manual testing.

nathan

-- 
Nathan Sidwell
2018-09-11  Nathan Sidwell  <nathan@acm.org>

	* gcc.c (load_specs, execute, run_attempt): Use %qs not '%s'.

Index: gcc.c
===================================================================
--- gcc.c	(revision 264216)
+++ gcc.c	(working copy)
@@ -2102,7 +2102,7 @@ load_specs (const char *filename)
     {
     failed:
       /* This leaves DESC open, but the OS will save us.  */
-      fatal_error (input_location, "cannot read spec file '%s': %m", filename);
+      fatal_error (input_location, "cannot read spec file %qs: %m", filename);
     }
 
   if (stat (filename, &statbuf) < 0)
@@ -3174,8 +3174,8 @@ execute (void)
 	{
 	  errno = err;
 	  fatal_error (input_location,
-		       err ? G_("cannot execute '%s' %s: %m")
-		       : G_("cannot execute '%s' %s"),
+		       err ? G_("cannot execute %qs: %s: %m")
+		       : G_("cannot execute %qs: %s"),
 		       string, errmsg);
 	}
 
@@ -6887,8 +6887,8 @@ run_attempt (const char **new_argv, cons
     {
       errno = err;
       fatal_error (input_location,
-		   err ? G_ ("cannot execute '%s' %s: %m")
-		   : G_ ("cannot execute '%s' %s"),
+		   err ? G_ ("cannot execute %qs: %s: %m")
+		   : G_ ("cannot execute %qs: %s"),
 		   new_argv[0], errmsg);
     }

Patch

2018-09-11  Nathan Sidwell  <nathan@acm.org>

	* gcc.c (perror_with_name, pfatal_with_name): Delete.
	(load_specs): Use fatal_error.
	(DELETE_IF_ORDINARY, process_command): Use error.
	(execute, run_attempt): Use fatal_error.

	* gcc.dg/driver-specs.c: New.

Index: gcc.c
===================================================================
--- gcc.c	(revision 264178)
+++ gcc.c	(working copy)
@@ -372,8 +372,6 @@  static void give_switch (int, int);
 static int default_arg (const char *, int);
 static void set_multilib_dir (void);
 static void print_multilib_info (void);
-static void perror_with_name (const char *);
-static void pfatal_with_name (const char *) ATTRIBUTE_NORETURN;
 static void display_help (void);
 static void add_preprocessor_option (const char *, int);
 static void add_assembler_option (const char *, int);
@@ -2101,15 +2099,20 @@  load_specs (const char *filename)
   /* Open and stat the file.  */
   desc = open (filename, O_RDONLY, 0);
   if (desc < 0)
-    pfatal_with_name (filename);
+    {
+    failed:
+      /* This leaves DESC open, but the OS will save us.  */
+      fatal_error (input_location, "cannot read spec file '%s': %m", filename);
+    }
+
   if (stat (filename, &statbuf) < 0)
-    pfatal_with_name (filename);
+    goto failed;
 
   /* Read contents of file into BUFFER.  */
   buffer = XNEWVEC (char, statbuf.st_size + 1);
   readlen = read (desc, buffer, (unsigned) statbuf.st_size);
   if (readlen < 0)
-    pfatal_with_name (filename);
+    goto failed;
   buffer[readlen] = 0;
   close (desc);
 
@@ -2490,7 +2493,7 @@  do
     if (stat (NAME, &ST) >= 0 && S_ISREG (ST.st_mode))  \
       if (unlink (NAME) < 0)                            \
 	if (VERBOSE_FLAG)                               \
-	  perror_with_name (NAME);                      \
+	  error ("%s: %m", (NAME));			\
   } while (0)
 #endif
 
@@ -3169,13 +3172,11 @@  execute (void)
 			NULL, NULL, &err);
       if (errmsg != NULL)
 	{
-	  if (err == 0)
-	    fatal_error (input_location, errmsg);
-	  else
-	    {
-	      errno = err;
-	      pfatal_with_name (errmsg);
-	    }
+	  errno = err;
+	  fatal_error (input_location,
+		       err ? G_("cannot execute '%s' %s: %m")
+		       : G_("cannot execute '%s' %s"),
+		       string, errmsg);
 	}
 
       if (i && string != commands[i].prog)
@@ -4545,10 +4546,8 @@  process_command (unsigned int decoded_op
 
           if (strcmp (fname, "-") != 0 && access (fname, F_OK) < 0)
 	    {
-	      if (fname[0] == '@' && access (fname + 1, F_OK) < 0)
-		perror_with_name (fname + 1);
-	      else
-		perror_with_name (fname);
+	      bool resp = fname[0] == '@' && access (fname + 1, F_OK) < 0;
+	      error ("%s: %m", fname + resp);
 	    }
           else
 	    add_infile (arg, spec_lang);
@@ -6886,13 +6885,11 @@  run_attempt (const char **new_argv, cons
 		    err_temp, &err);
   if (errmsg != NULL)
     {
-      if (err == 0)
-	fatal_error (input_location, errmsg);
-      else
-	{
-	  errno = err;
-	  pfatal_with_name (errmsg);
-	}
+      errno = err;
+      fatal_error (input_location,
+		   err ? G_ ("cannot execute '%s' %s: %m")
+		   : G_ ("cannot execute '%s' %s"),
+		   new_argv[0], errmsg);
     }
 
   if (!pex_get_status (pex, 1, &exit_status))
@@ -8407,19 +8404,6 @@  save_string (const char *s, int len)
   return result;
 }
 
-void
-pfatal_with_name (const char *name)
-{
-  perror_with_name (name);
-  delete_temp_files ();
-  exit (1);
-}
-
-static void
-perror_with_name (const char *name)
-{
-  error ("%s: %m", name);
-}
 
 static inline void
 validate_switches_from_spec (const char *spec, bool user)
Index: testsuite/gcc.dg/driver-specs.c
===================================================================
--- testsuite/gcc.dg/driver-specs.c	(nonexistent)
+++ testsuite/gcc.dg/driver-specs.c	(working copy)
@@ -0,0 +1,4 @@ 
+// { dg-additional-options "-specs=not-a-file" }
+// { dg-prune-output "compilation terminated" }
+// { dg-error "cannot read spec file" "" { target *-*-* } 0 }
+