diff mbox series

lto: fix error handling for -Wl,-plugin-opt=debug

Message ID 7e837179-e25b-76d4-ad21-3ee6b2326bea@suse.cz
State New
Headers show
Series lto: fix error handling for -Wl,-plugin-opt=debug | expand

Commit Message

Martin Liška Feb. 2, 2022, 2:30 p.m. UTC
When one uses something like: -Wl,-plugin-opt=debug,
we end up with lto1 WPA invocation that has 'debug'
on command line. We interpret that as input filename.

The patch moves resolution checking later so that we end up with
a reasonable error message:

lto1: error: open debug failed: No such file or directory
lto1: fatal error: errors during merging of translation units

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

	PR lto/104333

gcc/lto/ChangeLog:

	* lto-common.cc (read_cgraph_and_symbols): Move resolution
	checking for number of files later and report a reasonable
	error message.
---
  gcc/lto/lto-common.cc | 14 ++++++++------
  1 file changed, 8 insertions(+), 6 deletions(-)

Comments

Richard Biener Feb. 2, 2022, 2:38 p.m. UTC | #1
On Wed, Feb 2, 2022 at 3:31 PM Martin Liška <mliska@suse.cz> wrote:
>
> When one uses something like: -Wl,-plugin-opt=debug,
> we end up with lto1 WPA invocation that has 'debug'
> on command line. We interpret that as input filename.
>
> The patch moves resolution checking later so that we end up with
> a reasonable error message:
>
> lto1: error: open debug failed: No such file or directory

I think almost all of these errors should be fatal_error, not error ()
since we cannot really recover.  That would make ..

> lto1: fatal error: errors during merging of translation units
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
>
>         PR lto/104333
>
> gcc/lto/ChangeLog:
>
>         * lto-common.cc (read_cgraph_and_symbols): Move resolution
>         checking for number of files later and report a reasonable
>         error message.
> ---
>   gcc/lto/lto-common.cc | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/lto/lto-common.cc b/gcc/lto/lto-common.cc
> index 4d6686b0b99..fbcd4742ee4 100644
> --- a/gcc/lto/lto-common.cc
> +++ b/gcc/lto/lto-common.cc
> @@ -2704,6 +2704,7 @@ read_cgraph_and_symbols (unsigned nfiles, const char **fnames)
>   {
>     unsigned int i, last_file_ix;
>     FILE *resolution;
> +  unsigned resolution_objects = 0;
>     int count = 0;
>     struct lto_file_decl_data **decl_data;
>     symtab_node *snode;
> @@ -2726,18 +2727,14 @@ read_cgraph_and_symbols (unsigned nfiles, const char **fnames)
>     if (resolution_file_name)
>       {
>         int t;
> -      unsigned num_objects;
>
>         resolution = fopen (resolution_file_name, "r");
>         if (resolution == NULL)
>         fatal_error (input_location,
>                      "could not open symbol resolution file: %m");
>
> -      t = fscanf (resolution, "%u", &num_objects);
> +      t = fscanf (resolution, "%u", &resolution_objects);
>         gcc_assert (t == 1);
> -
> -      /* True, since the plugin splits the archives.  */
> -      gcc_assert (num_objects == nfiles);
>       }
>     symtab->state = LTO_STREAMING;
>
> @@ -2806,7 +2803,12 @@ read_cgraph_and_symbols (unsigned nfiles, const char **fnames)
>     lto_register_canonical_types_for_odr_types ();
>
>     if (resolution_file_name)
> -    fclose (resolution);
> +    {
> +      /* True, since the plugin splits the archives.  */
> +      if (!seen_error ())

... checking for seen_error () unnecessary.

> +       gcc_assert (resolution_objects == nfiles);
> +      fclose (resolution);
> +    }
>
>     /* Show the LTO report before launching LTRANS.  */
>     if (flag_lto_report || (flag_wpa && flag_lto_report_wpa))
> --
> 2.34.1
>
Martin Liška Feb. 2, 2022, 2:48 p.m. UTC | #2
On 2/2/22 15:38, Richard Biener wrote:
> ... checking for seen_error () unnecessary.

Sure, so something like this?

Ready to be installed?
Thanks,
Martin
Richard Biener Feb. 2, 2022, 2:50 p.m. UTC | #3
On Wed, Feb 2, 2022 at 3:48 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 2/2/22 15:38, Richard Biener wrote:
> > ... checking for seen_error () unnecessary.
>
> Sure, so something like this?

yes, I think so.

> Ready to be installed?

OK.

> Thanks,
> Martin
diff mbox series

Patch

diff --git a/gcc/lto/lto-common.cc b/gcc/lto/lto-common.cc
index 4d6686b0b99..fbcd4742ee4 100644
--- a/gcc/lto/lto-common.cc
+++ b/gcc/lto/lto-common.cc
@@ -2704,6 +2704,7 @@  read_cgraph_and_symbols (unsigned nfiles, const char **fnames)
  {
    unsigned int i, last_file_ix;
    FILE *resolution;
+  unsigned resolution_objects = 0;
    int count = 0;
    struct lto_file_decl_data **decl_data;
    symtab_node *snode;
@@ -2726,18 +2727,14 @@  read_cgraph_and_symbols (unsigned nfiles, const char **fnames)
    if (resolution_file_name)
      {
        int t;
-      unsigned num_objects;
  
        resolution = fopen (resolution_file_name, "r");
        if (resolution == NULL)
  	fatal_error (input_location,
  		     "could not open symbol resolution file: %m");
  
-      t = fscanf (resolution, "%u", &num_objects);
+      t = fscanf (resolution, "%u", &resolution_objects);
        gcc_assert (t == 1);
-
-      /* True, since the plugin splits the archives.  */
-      gcc_assert (num_objects == nfiles);
      }
    symtab->state = LTO_STREAMING;
  
@@ -2806,7 +2803,12 @@  read_cgraph_and_symbols (unsigned nfiles, const char **fnames)
    lto_register_canonical_types_for_odr_types ();
  
    if (resolution_file_name)
-    fclose (resolution);
+    {
+      /* True, since the plugin splits the archives.  */
+      if (!seen_error ())
+	gcc_assert (resolution_objects == nfiles);
+      fclose (resolution);
+    }
  
    /* Show the LTO report before launching LTRANS.  */
    if (flag_lto_report || (flag_wpa && flag_lto_report_wpa))