public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] elfcpp: Add Sym::Sym(unsigned char*) constructor
@ 2020-12-19 14:57 H.J. Lu
  2020-12-19 19:59 ` Cary Coutant
  0 siblings, 1 reply; 5+ messages in thread
From: H.J. Lu @ 2020-12-19 14:57 UTC (permalink / raw)
  To: binutils

GCC 11 failed to build gold at -O0 due to -Wmaybe-uninitialized change
in GCC 11:

    In addition, passing a pointer (or in C++, a reference) to an
uninitialized object to a const-qualified function argument is also
diagnosed by this warning. (-Wuninitialized is issued for built-in
functions known to read the object.) Annotating the function with
attribute access (none) indicates that the argument isn’t used to
access the object and avoids the warning (see Common Function Attributes).

Add Sym::Sym(unsigned char*) constructor to support

  const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
  unsigned char symbuf[sym_size];
  elfcpp::Sym<size, big_endian> sym(symbuf);

	PR gold/27097
	elfcpp.h (Sym::Sym(unsigned char*)): New.
---
 elfcpp/elfcpp.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/elfcpp/elfcpp.h b/elfcpp/elfcpp.h
index 428ecb8935..5ed9711dfa 100644
--- a/elfcpp/elfcpp.h
+++ b/elfcpp/elfcpp.h
@@ -1533,6 +1533,10 @@ class Sym
     : p_(reinterpret_cast<const internal::Sym_data<size>*>(p))
   { }
 
+  Sym(unsigned char* p)
+    : p_(reinterpret_cast<const internal::Sym_data<size>*>(p))
+  { }
+
   template<typename File>
   Sym(File* file, typename File::Location loc)
     : p_(reinterpret_cast<const internal::Sym_data<size>*>(
-- 
2.29.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] elfcpp: Add Sym::Sym(unsigned char*) constructor
  2020-12-19 14:57 [PATCH] elfcpp: Add Sym::Sym(unsigned char*) constructor H.J. Lu
@ 2020-12-19 19:59 ` Cary Coutant
  2020-12-19 20:12   ` H.J. Lu
  0 siblings, 1 reply; 5+ messages in thread
From: Cary Coutant @ 2020-12-19 19:59 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

Does this patch fix the problem?

--- a/gold/plugin.cc
+++ b/gold/plugin.cc
@@ -1397,8 +1397,8 @@ Sized_pluginobj<size,
big_endian>::do_add_symbols(Symbol_table* symtab,
 {
   const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
   unsigned char symbuf[sym_size];
-  elfcpp::Sym<size, big_endian> sym(symbuf);
   elfcpp::Sym_write<size, big_endian> osym(symbuf);
+  elfcpp::Sym<size, big_endian> sym(symbuf);

   Plugin_recorder* recorder = parameters->options().plugins()->recorder();
   if (recorder != NULL)

If not that, then how about this:

--- a/gold/plugin.cc
+++ b/gold/plugin.cc
@@ -1397,7 +1397,6 @@ Sized_pluginobj<size,
big_endian>::do_add_symbols(Symbol_table* symtab,
 {
   const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
   unsigned char symbuf[sym_size];
-  elfcpp::Sym<size, big_endian> sym(symbuf);
   elfcpp::Sym_write<size, big_endian> osym(symbuf);

   Plugin_recorder* recorder = parameters->options().plugins()->recorder();
@@ -1480,6 +1479,7 @@ Sized_pluginobj<size,
big_endian>::do_add_symbols(Symbol_table* symtab,
       osym.put_st_other(vis, 0);
       osym.put_st_shndx(shndx);

+      elfcpp::Sym<size, big_endian> sym(symbuf);
       this->symbols_[i] =
         symtab->add_from_pluginobj<size, big_endian>(this, name, ver, &sym);
     }

(Sorry, I don't have GCC 11 available to test.)

I don't think adding a new constructor is the right solution.

-cary

On Sat, Dec 19, 2020 at 6:57 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> GCC 11 failed to build gold at -O0 due to -Wmaybe-uninitialized change
> in GCC 11:
>
>     In addition, passing a pointer (or in C++, a reference) to an
> uninitialized object to a const-qualified function argument is also
> diagnosed by this warning. (-Wuninitialized is issued for built-in
> functions known to read the object.) Annotating the function with
> attribute access (none) indicates that the argument isn’t used to
> access the object and avoids the warning (see Common Function Attributes).
>
> Add Sym::Sym(unsigned char*) constructor to support
>
>   const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
>   unsigned char symbuf[sym_size];
>   elfcpp::Sym<size, big_endian> sym(symbuf);
>
>         PR gold/27097
>         elfcpp.h (Sym::Sym(unsigned char*)): New.
> ---
>  elfcpp/elfcpp.h | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/elfcpp/elfcpp.h b/elfcpp/elfcpp.h
> index 428ecb8935..5ed9711dfa 100644
> --- a/elfcpp/elfcpp.h
> +++ b/elfcpp/elfcpp.h
> @@ -1533,6 +1533,10 @@ class Sym
>      : p_(reinterpret_cast<const internal::Sym_data<size>*>(p))
>    { }
>
> +  Sym(unsigned char* p)
> +    : p_(reinterpret_cast<const internal::Sym_data<size>*>(p))
> +  { }
> +
>    template<typename File>
>    Sym(File* file, typename File::Location loc)
>      : p_(reinterpret_cast<const internal::Sym_data<size>*>(
> --
> 2.29.2
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] elfcpp: Add Sym::Sym(unsigned char*) constructor
  2020-12-19 19:59 ` Cary Coutant
@ 2020-12-19 20:12   ` H.J. Lu
  2020-12-19 21:02     ` Cary Coutant
  0 siblings, 1 reply; 5+ messages in thread
From: H.J. Lu @ 2020-12-19 20:12 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Binutils

[-- Attachment #1: Type: text/plain, Size: 3264 bytes --]

On Sat, Dec 19, 2020 at 11:59 AM Cary Coutant <ccoutant@gmail.com> wrote:
>
> Does this patch fix the problem?
>
> --- a/gold/plugin.cc
> +++ b/gold/plugin.cc
> @@ -1397,8 +1397,8 @@ Sized_pluginobj<size,
> big_endian>::do_add_symbols(Symbol_table* symtab,
>  {
>    const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
>    unsigned char symbuf[sym_size];
> -  elfcpp::Sym<size, big_endian> sym(symbuf);
>    elfcpp::Sym_write<size, big_endian> osym(symbuf);
> +  elfcpp::Sym<size, big_endian> sym(symbuf);
>
>    Plugin_recorder* recorder = parameters->options().plugins()->recorder();
>    if (recorder != NULL)
>
> If not that, then how about this:
>
> --- a/gold/plugin.cc
> +++ b/gold/plugin.cc
> @@ -1397,7 +1397,6 @@ Sized_pluginobj<size,
> big_endian>::do_add_symbols(Symbol_table* symtab,
>  {
>    const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
>    unsigned char symbuf[sym_size];
> -  elfcpp::Sym<size, big_endian> sym(symbuf);
>    elfcpp::Sym_write<size, big_endian> osym(symbuf);
>
>    Plugin_recorder* recorder = parameters->options().plugins()->recorder();
> @@ -1480,6 +1479,7 @@ Sized_pluginobj<size,
> big_endian>::do_add_symbols(Symbol_table* symtab,
>        osym.put_st_other(vis, 0);
>        osym.put_st_shndx(shndx);
>
> +      elfcpp::Sym<size, big_endian> sym(symbuf);
>        this->symbols_[i] =
>          symtab->add_from_pluginobj<size, big_endian>(this, name, ver, &sym);
>      }

Both fixed the build.  Here is the complete patch to fox GCC 11 build.

> (Sorry, I don't have GCC 11 available to test.)
>
> I don't think adding a new constructor is the right solution.
>
> -cary
>
> On Sat, Dec 19, 2020 at 6:57 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > GCC 11 failed to build gold at -O0 due to -Wmaybe-uninitialized change
> > in GCC 11:
> >
> >     In addition, passing a pointer (or in C++, a reference) to an
> > uninitialized object to a const-qualified function argument is also
> > diagnosed by this warning. (-Wuninitialized is issued for built-in
> > functions known to read the object.) Annotating the function with
> > attribute access (none) indicates that the argument isn’t used to
> > access the object and avoids the warning (see Common Function Attributes).
> >
> > Add Sym::Sym(unsigned char*) constructor to support
> >
> >   const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
> >   unsigned char symbuf[sym_size];
> >   elfcpp::Sym<size, big_endian> sym(symbuf);
> >
> >         PR gold/27097
> >         elfcpp.h (Sym::Sym(unsigned char*)): New.
> > ---
> >  elfcpp/elfcpp.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/elfcpp/elfcpp.h b/elfcpp/elfcpp.h
> > index 428ecb8935..5ed9711dfa 100644
> > --- a/elfcpp/elfcpp.h
> > +++ b/elfcpp/elfcpp.h
> > @@ -1533,6 +1533,10 @@ class Sym
> >      : p_(reinterpret_cast<const internal::Sym_data<size>*>(p))
> >    { }
> >
> > +  Sym(unsigned char* p)
> > +    : p_(reinterpret_cast<const internal::Sym_data<size>*>(p))
> > +  { }
> > +
> >    template<typename File>
> >    Sym(File* file, typename File::Location loc)
> >      : p_(reinterpret_cast<const internal::Sym_data<size>*>(
> > --
> > 2.29.2
> >



-- 
H.J.

[-- Attachment #2: pr27097.patch --]
[-- Type: text/x-patch, Size: 2310 bytes --]

diff --git a/gold/incremental.cc b/gold/incremental.cc
index 1f2ae5b87b..525c92cff6 100644
--- a/gold/incremental.cc
+++ b/gold/incremental.cc
@@ -2129,7 +2129,6 @@ Sized_relobj_incr<size, big_endian>::do_add_symbols(
 {
   const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
   unsigned char symbuf[sym_size];
-  elfcpp::Sym<size, big_endian> sym(symbuf);
   elfcpp::Sym_write<size, big_endian> osym(symbuf);
 
   typedef typename elfcpp::Elf_types<size>::Elf_WXword Elf_size_type;
@@ -2196,6 +2195,7 @@ Sized_relobj_incr<size, big_endian>::do_add_symbols(
       osym.put_st_other(gsym.get_st_other());
       osym.put_st_shndx(shndx);
 
+      elfcpp::Sym<size, big_endian> sym(symbuf);
       Symbol* res = symtab->add_from_incrobj(this, name, NULL, &sym);
 
       if (shndx != elfcpp::SHN_UNDEF)
@@ -2730,7 +2730,6 @@ Sized_incr_dynobj<size, big_endian>::do_add_symbols(
 {
   const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
   unsigned char symbuf[sym_size];
-  elfcpp::Sym<size, big_endian> sym(symbuf);
   elfcpp::Sym_write<size, big_endian> osym(symbuf);
 
   unsigned int nsyms = this->input_reader_.get_global_symbol_count();
@@ -2795,6 +2794,7 @@ Sized_incr_dynobj<size, big_endian>::do_add_symbols(
       osym.put_st_other(gsym.get_st_other());
       osym.put_st_shndx(shndx);
 
+      elfcpp::Sym<size, big_endian> sym(symbuf);
       Sized_symbol<size>* res =
 	  symtab->add_from_incrobj<size, big_endian>(this, name, NULL, &sym);
       this->symbols_[i] = res;
diff --git a/gold/plugin.cc b/gold/plugin.cc
index fd37957e73..5f5da5dcea 100644
--- a/gold/plugin.cc
+++ b/gold/plugin.cc
@@ -1397,7 +1397,6 @@ Sized_pluginobj<size, big_endian>::do_add_symbols(Symbol_table* symtab,
 {
   const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
   unsigned char symbuf[sym_size];
-  elfcpp::Sym<size, big_endian> sym(symbuf);
   elfcpp::Sym_write<size, big_endian> osym(symbuf);
 
   Plugin_recorder* recorder = parameters->options().plugins()->recorder();
@@ -1480,6 +1479,7 @@ Sized_pluginobj<size, big_endian>::do_add_symbols(Symbol_table* symtab,
       osym.put_st_other(vis, 0);
       osym.put_st_shndx(shndx);
 
+      elfcpp::Sym<size, big_endian> sym(symbuf);
       this->symbols_[i] =
         symtab->add_from_pluginobj<size, big_endian>(this, name, ver, &sym);
     }

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] elfcpp: Add Sym::Sym(unsigned char*) constructor
  2020-12-19 20:12   ` H.J. Lu
@ 2020-12-19 21:02     ` Cary Coutant
  2020-12-19 21:31       ` [PATCH] gold: Move sym declaration just before use H.J. Lu
  0 siblings, 1 reply; 5+ messages in thread
From: Cary Coutant @ 2020-12-19 21:02 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

> Both fixed the build.  Here is the complete patch to fox GCC 11 build.

This is OK, with ChangeLog entry. Thanks!

-cary

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] gold: Move sym declaration just before use
  2020-12-19 21:02     ` Cary Coutant
@ 2020-12-19 21:31       ` H.J. Lu
  0 siblings, 0 replies; 5+ messages in thread
From: H.J. Lu @ 2020-12-19 21:31 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Binutils

[-- Attachment #1: Type: text/plain, Size: 262 bytes --]

On Sat, Dec 19, 2020 at 1:02 PM Cary Coutant <ccoutant@gmail.com> wrote:
>
> > Both fixed the build.  Here is the complete patch to fox GCC 11 build.
>
> This is OK, with ChangeLog entry. Thanks!
>
> -cary

This is the patch I am checking in.

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-gold-Move-sym-declaration-just-before-use.patch --]
[-- Type: text/x-patch, Size: 3501 bytes --]

From 68b09ebb5df333e2bac42ed3f8ff7c7e7ab925a1 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 19 Dec 2020 13:30:39 -0800
Subject: [PATCH] gold: Move sym declaration just before use

Move sym declaration just before use to avoid -Wmaybe-uninitialized
warning from GCC 11.

	PR gold/27097
	* incremental.cc (Sized_relobj_incr::do_add_symbols): Move sym
	declaration just before use.
	(Sized_incr_dynobj::do_add_symbols): Likewise.
	* plugin.cc (Sized_pluginobj::do_add_symbols): Likewise.
---
 gold/ChangeLog      | 8 ++++++++
 gold/incremental.cc | 4 ++--
 gold/plugin.cc      | 2 +-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/gold/ChangeLog b/gold/ChangeLog
index 77b1f886c3..9614a87f76 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,11 @@
+2020-12-19  H.J. Lu  <hjl.tools@gmail.com>
+
+	PR gold/27097
+	* incremental.cc (Sized_relobj_incr::do_add_symbols): Move sym
+	declaration just before use.
+	(Sized_incr_dynobj::do_add_symbols): Likewise.
+	* plugin.cc (Sized_pluginobj::do_add_symbols): Likewise.
+
 2020-12-15  Cary Coutant  <ccoutant@gmail.com>
 
 	* dwp.cc (class Dwo_file): Use new Ehdr::get_ei_osabi and
diff --git a/gold/incremental.cc b/gold/incremental.cc
index 1f2ae5b87b..525c92cff6 100644
--- a/gold/incremental.cc
+++ b/gold/incremental.cc
@@ -2129,7 +2129,6 @@ Sized_relobj_incr<size, big_endian>::do_add_symbols(
 {
   const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
   unsigned char symbuf[sym_size];
-  elfcpp::Sym<size, big_endian> sym(symbuf);
   elfcpp::Sym_write<size, big_endian> osym(symbuf);
 
   typedef typename elfcpp::Elf_types<size>::Elf_WXword Elf_size_type;
@@ -2196,6 +2195,7 @@ Sized_relobj_incr<size, big_endian>::do_add_symbols(
       osym.put_st_other(gsym.get_st_other());
       osym.put_st_shndx(shndx);
 
+      elfcpp::Sym<size, big_endian> sym(symbuf);
       Symbol* res = symtab->add_from_incrobj(this, name, NULL, &sym);
 
       if (shndx != elfcpp::SHN_UNDEF)
@@ -2730,7 +2730,6 @@ Sized_incr_dynobj<size, big_endian>::do_add_symbols(
 {
   const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
   unsigned char symbuf[sym_size];
-  elfcpp::Sym<size, big_endian> sym(symbuf);
   elfcpp::Sym_write<size, big_endian> osym(symbuf);
 
   unsigned int nsyms = this->input_reader_.get_global_symbol_count();
@@ -2795,6 +2794,7 @@ Sized_incr_dynobj<size, big_endian>::do_add_symbols(
       osym.put_st_other(gsym.get_st_other());
       osym.put_st_shndx(shndx);
 
+      elfcpp::Sym<size, big_endian> sym(symbuf);
       Sized_symbol<size>* res =
 	  symtab->add_from_incrobj<size, big_endian>(this, name, NULL, &sym);
       this->symbols_[i] = res;
diff --git a/gold/plugin.cc b/gold/plugin.cc
index fd37957e73..5f5da5dcea 100644
--- a/gold/plugin.cc
+++ b/gold/plugin.cc
@@ -1397,7 +1397,6 @@ Sized_pluginobj<size, big_endian>::do_add_symbols(Symbol_table* symtab,
 {
   const int sym_size = elfcpp::Elf_sizes<size>::sym_size;
   unsigned char symbuf[sym_size];
-  elfcpp::Sym<size, big_endian> sym(symbuf);
   elfcpp::Sym_write<size, big_endian> osym(symbuf);
 
   Plugin_recorder* recorder = parameters->options().plugins()->recorder();
@@ -1480,6 +1479,7 @@ Sized_pluginobj<size, big_endian>::do_add_symbols(Symbol_table* symtab,
       osym.put_st_other(vis, 0);
       osym.put_st_shndx(shndx);
 
+      elfcpp::Sym<size, big_endian> sym(symbuf);
       this->symbols_[i] =
         symtab->add_from_pluginobj<size, big_endian>(this, name, ver, &sym);
     }
-- 
2.29.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-12-19 21:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-19 14:57 [PATCH] elfcpp: Add Sym::Sym(unsigned char*) constructor H.J. Lu
2020-12-19 19:59 ` Cary Coutant
2020-12-19 20:12   ` H.J. Lu
2020-12-19 21:02     ` Cary Coutant
2020-12-19 21:31       ` [PATCH] gold: Move sym declaration just before use H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).