diff mbox series

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 | expand

Commit Message

Rainer Orth May 6, 2019, 6:35 p.m. UTC 
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

Comments

Richard Biener May 6, 2019, 6:46 p.m. UTC | #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. UTC | #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. UTC | #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. UTC | #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
Richard Biener May 7, 2019, 7:37 a.m. UTC | #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.
>
diff mbox series

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);
   }
 };