Fix Solaris bootstrap: lto-common.c, lto-dump.c format mismatches

Message ID ydd8svjxwgs.fsf@CeBiTec.Uni-Bielefeld.DE
State New
Headers show
Series
  • Fix Solaris bootstrap: lto-common.c, lto-dump.c format mismatches
Related show

Commit Message

Rainer Orth May 6, 2019, 6:35 p.m.
The recent LTO patches broke Solaris bootstrap (both sparc and x86):

/vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c: In function 'lto_file_decl_data* lto_file_read(lto_file*, std::FILE*, int*)':
/vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c:2114:32: error: format '%ld' expects argument of type 'long int', but argument 4 has type 'intptr_t' {aka 'int'} [-Werror=format=]
 2114 |       fprintf (stdout, "%2d %8ld %8ld   %s\n",
      |                             ~~~^
      |                                |
      |                                long int
      |                             %8d
 2115 |         ++i, section->start, section->len, section->name);
      |              ~~~~~~~~~~~~~~
      |                       |
      |                       intptr_t {aka int}

/vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c:2114:37: error: format '%ld' expects argument of type 'long int', but argument 5 has type 'std::size_t' {aka 'unsigned int'} [-Werror=format=]
 2114 |       fprintf (stdout, "%2d %8ld %8ld   %s\n",
      |                                  ~~~^
      |                                     |
      |                                     long int
      |                                  %8d
 2115 |         ++i, section->start, section->len, section->name);
      |                              ~~~~~~~~~~~~
      |                                       |
      |                                       std::size_t {aka unsigned int}

/vol/gcc/src/hg/trunk/local/gcc/lto/lto-dump.c: In member function 'virtual void symbol_entry::dump()':
/vol/gcc/src/hg/trunk/local/gcc/lto/lto-dump.c:63:25: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'std::size_t' {aka 'unsigned int'} [-Werror=format=]
   63 |     printf ("%s  %s  %4lu  %s  ", type_name, visibility, sz, name);
      |                      ~~~^                                ~~
      |                         |                                |
      |                         long unsigned int                std::size_t {aka unsigned int}
      |                      %4u

Fixed as follows.  i386-pc-solaris2.11 bootstrap has completed with this
patch, sparc-sun-solaris2.11 is running the testsuite and
x86_64-pc-linux-gnu is building the runtime libs.

Ok for mainline?

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University


2019-05-06  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* lto-common.c (lto_file_read): Cast section->start, section->len
	to match format.
	* lto-dump.c (symbol_entry::dump): Cast sz to match format.

Comments

Richard Biener May 6, 2019, 6:46 p.m. | #1
On May 6, 2019 8:35:15 PM GMT+02:00, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:
>The recent LTO patches broke Solaris bootstrap (both sparc and x86):

>

>/vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c: In function

>'lto_file_decl_data* lto_file_read(lto_file*, std::FILE*, int*)':

>/vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c:2114:32: error: format

>'%ld' expects argument of type 'long int', but argument 4 has type

>'intptr_t' {aka 'int'} [-Werror=format=]

> 2114 |       fprintf (stdout, "%2d %8ld %8ld   %s\n",

>      |                             ~~~^

>      |                                |

>      |                                long int

>      |                             %8d

> 2115 |         ++i, section->start, section->len, section->name);

>      |              ~~~~~~~~~~~~~~

>      |                       |

>      |                       intptr_t {aka int}

>

>/vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c:2114:37: error: format

>'%ld' expects argument of type 'long int', but argument 5 has type

>'std::size_t' {aka 'unsigned int'} [-Werror=format=]

> 2114 |       fprintf (stdout, "%2d %8ld %8ld   %s\n",

>      |                                  ~~~^

>      |                                     |

>      |                                     long int

>      |                                  %8d

> 2115 |         ++i, section->start, section->len, section->name);

>      |                              ~~~~~~~~~~~~

>      |                                       |

> |                                       std::size_t {aka unsigned int}

>

>/vol/gcc/src/hg/trunk/local/gcc/lto/lto-dump.c: In member function

>'virtual void symbol_entry::dump()':

>/vol/gcc/src/hg/trunk/local/gcc/lto/lto-dump.c:63:25: error: format

>'%lu' expects argument of type 'long unsigned int', but argument 4 has

>type 'std::size_t' {aka 'unsigned int'} [-Werror=format=]

>63 |     printf ("%s  %s  %4lu  %s  ", type_name, visibility, sz,

>name);

>      |                      ~~~^                                ~~

>      |                         |                                |

>|                         long unsigned int                std::size_t

>{aka unsigned int}

>      |                      %4u

>

>Fixed as follows.  i386-pc-solaris2.11 bootstrap has completed with

>this

>patch, sparc-sun-solaris2.11 is running the testsuite and

>x86_64-pc-linux-gnu is building the runtime libs.

>

>Ok for mainline?


Can you use the PRI* format macros to match the types instead?

Ok with that change. 

Richard. 

>	Rainer
Jakub Jelinek May 6, 2019, 8:39 p.m. | #2
On Mon, May 06, 2019 at 08:46:05PM +0200, Richard Biener wrote:
> >Fixed as follows.  i386-pc-solaris2.11 bootstrap has completed with

> >this

> >patch, sparc-sun-solaris2.11 is running the testsuite and

> >x86_64-pc-linux-gnu is building the runtime libs.

> >

> >Ok for mainline?

> 

> Can you use the PRI* format macros to match the types instead?


Is that sufficiently portable though?
I mean, for PRI[diouxX]64 we redefine those macros in hwint.h if needed.
But we don't have anything similar for PRI[diouxX]PTR if inttypes.h
is not available, and for size_t there isn't even any PRI* macro at all.

	Jakub
Richard Biener May 7, 2019, 7:29 a.m. | #3
On Mon, May 6, 2019 at 10:39 PM Jakub Jelinek <jakub@redhat.com> wrote:
>

> On Mon, May 06, 2019 at 08:46:05PM +0200, Richard Biener wrote:

> > >Fixed as follows.  i386-pc-solaris2.11 bootstrap has completed with

> > >this

> > >patch, sparc-sun-solaris2.11 is running the testsuite and

> > >x86_64-pc-linux-gnu is building the runtime libs.

> > >

> > >Ok for mainline?

> >

> > Can you use the PRI* format macros to match the types instead?

>

> Is that sufficiently portable though?

> I mean, for PRI[diouxX]64 we redefine those macros in hwint.h if needed.

> But we don't have anything similar for PRI[diouxX]PTR if inttypes.h

> is not available, and for size_t there isn't even any PRI* macro at all.


Use those that hwint.h provides - casting the value should be done as a last
resort.  Adding PRI[diouxX]PTR macros in hwint.h might be useful, I merely
added those that I wanted to use.

True, size_t is always a problem :/  Having something in hwint.h would
be useful though - I see the C standard is lacking here.

Richard.

>         Jakub
Rainer Orth May 7, 2019, 7:32 a.m. | #4
Hi Richard,

> On Mon, May 6, 2019 at 10:39 PM Jakub Jelinek <jakub@redhat.com> wrote:

>>

>> On Mon, May 06, 2019 at 08:46:05PM +0200, Richard Biener wrote:

>> > >Fixed as follows.  i386-pc-solaris2.11 bootstrap has completed with

>> > >this

>> > >patch, sparc-sun-solaris2.11 is running the testsuite and

>> > >x86_64-pc-linux-gnu is building the runtime libs.

>> > >

>> > >Ok for mainline?

>> >

>> > Can you use the PRI* format macros to match the types instead?

>>

>> Is that sufficiently portable though?

>> I mean, for PRI[diouxX]64 we redefine those macros in hwint.h if needed.

>> But we don't have anything similar for PRI[diouxX]PTR if inttypes.h

>> is not available, and for size_t there isn't even any PRI* macro at all.

>

> Use those that hwint.h provides - casting the value should be done as a last

> resort.  Adding PRI[diouxX]PTR macros in hwint.h might be useful, I merely

> added those that I wanted to use.

>

> True, size_t is always a problem :/  Having something in hwint.h would

> be useful though - I see the C standard is lacking here.


this is what I bootstrapped successfully last night on
i386-pc-solaris2.11 and x86_64-pc-linux-gnu.  I didn't feel like adding
PRI?PTR fallback definitions for the single use of intptr_t, though, and
am not really sure this is an improvement over my original patch.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University


2019-05-06  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* lto-common.c (lto_file_read): Print section->start as int64_t,
	section->len as uint64_t.
	* lto-dump.c (symbol_entry::dump): Print sz as uint64_t.
# HG changeset patch
# Parent  0694f1a35e195359100e42257e2e292a930a8829
Fix Solaris bootstrap: lto-common.c, lto-dump.c format mismatches

diff --git a/gcc/lto/lto-common.c b/gcc/lto/lto-common.c
--- a/gcc/lto/lto-common.c
+++ b/gcc/lto/lto-common.c
@@ -2111,8 +2111,9 @@ lto_file_read (lto_file *file, FILE *res
     fprintf (stdout, "\n    LTO Object Name: %s\n", file->filename);
     fprintf (stdout, "\nNo.    Offset    Size       Section Name\n\n");
     for (section = section_list.first; section != NULL; section = section->next)
-      fprintf (stdout, "%2d %8ld %8ld   %s\n",
-	       ++i, section->start, section->len, section->name);
+      fprintf (stdout, "%2d %8" PRId64 " %8" PRIu64 "   %s\n",
+	       ++i, (int64_t) section->start, (uint64_t) section->len,
+	       section->name);
   }
 
   /* Find all sub modules in the object and put their sections into new hash
diff --git a/gcc/lto/lto-dump.c b/gcc/lto/lto-dump.c
--- a/gcc/lto/lto-dump.c
+++ b/gcc/lto/lto-dump.c
@@ -60,7 +60,8 @@ struct symbol_entry
     const char *type_name = node->get_symtab_type_string ();
     const char *visibility = node->get_visibility_string ();
     size_t sz = get_size ();
-    printf ("%s  %s  %4lu  %s  ", type_name, visibility, sz, name);
+    printf ("%s  %s  %4" PRIu64 "  %s  ", type_name, visibility, (uint64_t) sz,
+	    name);
   }
 };
Richard Biener May 7, 2019, 7:37 a.m. | #5
On Tue, May 7, 2019 at 9:32 AM Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>

> Hi Richard,

>

> > On Mon, May 6, 2019 at 10:39 PM Jakub Jelinek <jakub@redhat.com> wrote:

> >>

> >> On Mon, May 06, 2019 at 08:46:05PM +0200, Richard Biener wrote:

> >> > >Fixed as follows.  i386-pc-solaris2.11 bootstrap has completed with

> >> > >this

> >> > >patch, sparc-sun-solaris2.11 is running the testsuite and

> >> > >x86_64-pc-linux-gnu is building the runtime libs.

> >> > >

> >> > >Ok for mainline?

> >> >

> >> > Can you use the PRI* format macros to match the types instead?

> >>

> >> Is that sufficiently portable though?

> >> I mean, for PRI[diouxX]64 we redefine those macros in hwint.h if needed.

> >> But we don't have anything similar for PRI[diouxX]PTR if inttypes.h

> >> is not available, and for size_t there isn't even any PRI* macro at all.

> >

> > Use those that hwint.h provides - casting the value should be done as a last

> > resort.  Adding PRI[diouxX]PTR macros in hwint.h might be useful, I merely

> > added those that I wanted to use.

> >

> > True, size_t is always a problem :/  Having something in hwint.h would

> > be useful though - I see the C standard is lacking here.

>

> this is what I bootstrapped successfully last night on

> i386-pc-solaris2.11 and x86_64-pc-linux-gnu.  I didn't feel like adding

> PRI?PTR fallback definitions for the single use of intptr_t, though, and

> am not really sure this is an improvement over my original patch.


Hmm, indeed.  I think in the end some printing support for size_t
is needed.  The use of intptr_t itself is probably somewhat bogus
and off_t should have been used (with the very same issue of course).
Or both should have been [u]int64_t from the start. Anyway sth to clean
up for LTO folks.

Patch is OK.

Richard.

>         Rainer

>

> --

> -----------------------------------------------------------------------------

> Rainer Orth, Center for Biotechnology, Bielefeld University

>

>

> 2019-05-06  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

>

>         * lto-common.c (lto_file_read): Print section->start as int64_t,

>         section->len as uint64_t.

>         * lto-dump.c (symbol_entry::dump): Print sz as uint64_t.

>

Patch

# HG changeset patch
# Parent  48cc59b91c89bd75e69175e99802075208e54e51
Fix Solaris bootstrap: lto-common.c, lto-dump.c format mismatches

diff --git a/gcc/lto/lto-common.c b/gcc/lto/lto-common.c
--- a/gcc/lto/lto-common.c
+++ b/gcc/lto/lto-common.c
@@ -2111,8 +2111,9 @@  lto_file_read (lto_file *file, FILE *res
     fprintf (stdout, "\n    LTO Object Name: %s\n", file->filename);
     fprintf (stdout, "\nNo.    Offset    Size       Section Name\n\n");
     for (section = section_list.first; section != NULL; section = section->next)
-      fprintf (stdout, "%2d %8ld %8ld   %s\n",
-	       ++i, section->start, section->len, section->name);
+      fprintf (stdout, "%2d %8ld %8lu   %s\n",
+	       ++i, (long) section->start, (unsigned long) section->len,
+	       section->name);
   }
 
   /* Find all sub modules in the object and put their sections into new hash
diff --git a/gcc/lto/lto-dump.c b/gcc/lto/lto-dump.c
--- a/gcc/lto/lto-dump.c
+++ b/gcc/lto/lto-dump.c
@@ -60,7 +60,8 @@  struct symbol_entry
     const char *type_name = node->get_symtab_type_string ();
     const char *visibility = node->get_visibility_string ();
     size_t sz = get_size ();
-    printf ("%s  %s  %4lu  %s  ", type_name, visibility, sz, name);
+    printf ("%s  %s  %4lu  %s  ", type_name, visibility, (unsigned long) sz,
+	    name);
   }
 };