[GOLD][COMMITTED] Fix build breakage with -g.

[GOLD][COMMITTED] Fix build breakage with -g.

[Doug Kwan (關振德)]

Hi,

    I just commit this quickly to work around a build breakage with -g
found by Sri.  The breakage happened as two static constant integral
data members of elfcpp::Elf_sizes were referenced as external symbols.
 I read the C++ standard and it appears to me that
elfcpp::Elf_sizes<32>::rel_size and elfcpp::Elf_sizes<32>::rel_size
are integer constant expressions and the compiler should be able to
compute their values.

-Doug

2010-01-13  Doug Kwan  <dougkwan@google.com>

        * arm.cc (Arm_relobj::section_needs_reloc_stub_scanning,
        Arm_relobj::scan_sections_for_stubs): Rearrange code to avoid an
        apparent compiler problem of not folding static constant integral
        data members of elfcpp::Elf_sizes<32>.

Index: gold/arm.cc
===================================================================
RCS file: /cvs/src/src/gold/arm.cc,v
retrieving revision 1.54
diff -u -u -p -r1.54 arm.cc
--- gold/arm.cc 13 Jan 2010 21:36:47 -0000      1.54
+++ gold/arm.cc 14 Jan 2010 02:37:58 -0000
@@ -4203,9 +4203,11 @@ Arm_relobj<big_endian>::section_needs_re
   if (this->adjust_shndx(shdr.get_sh_link()) != this->symtab_shndx())
     return false;

-  const unsigned int reloc_size = (sh_type == elfcpp::SHT_REL
-                                  ? elfcpp::Elf_sizes<32>::rel_size
-                                  : elfcpp::Elf_sizes<32>::rela_size);
+  unsigned int reloc_size;
+  if (sh_type == elfcpp::SHT_REL)
+    reloc_size = elfcpp::Elf_sizes<32>::rel_size;
+  else
+    reloc_size = elfcpp::Elf_sizes<32>::rela_size;

   // Ignore reloc section with unexpected entsize or uneven size.
   // The error will be reported in the final link.
@@ -4380,9 +4382,11 @@ Arm_relobj<big_endian>::scan_sections_fo
          relinfo.reloc_shndx = i;
          relinfo.data_shndx = index;
          unsigned int sh_type = shdr.get_sh_type();
-         const unsigned int reloc_size = (sh_type == elfcpp::SHT_REL
-                                          ? elfcpp::Elf_sizes<32>::rel_size
-                                          : elfcpp::Elf_sizes<32>::rela_size);
+         unsigned int reloc_size;
+         if (sh_type == elfcpp::SHT_REL)
+           reloc_size = elfcpp::Elf_sizes<32>::rel_size;
+         else
+           reloc_size = elfcpp::Elf_sizes<32>::rela_size;

          Output_section* os = out_sections[index];
          arm_target->scan_section_for_stubs(&relinfo, sh_type, prelocs,

Re: [GOLD][COMMITTED] Fix build breakage with -g.

[Doug Kwan (關振德)]

[+crowl]

I did a bit more research and read the C++ standard again, especially
in [class.static.data].  Paragraph 4 of it says the following:

"If a static data member is of const integral or const enumeration
type, its declaration in the class definition can specify a
constant-initializer which shall be ben integral constant expression
(5.19).  In that case, the member can appear in integral constant
expressions within its scope.  The member shall still be defined in a
namespace scope if it is used in the program and the namespace scope
definition shall not contain an initializer."

Since the static const integral data members of
elfcpp::Elf_sizes<size> are used in many places outside of the class
scope, I think we should add definitions of them in the elfcpp
namespace.

-Doug

2010/1/13 Doug Kwan (關振德) <dougkwan@google.com>:
> Hi,
>
>    I just commit this quickly to work around a build breakage with -g
> found by Sri.  The breakage happened as two static constant integral
> data members of elfcpp::Elf_sizes were referenced as external symbols.
>  I read the C++ standard and it appears to me that
> elfcpp::Elf_sizes<32>::rel_size and elfcpp::Elf_sizes<32>::rel_size
> are integer constant expressions and the compiler should be able to
> compute their values.
>
> -Doug
>
> 2010-01-13  Doug Kwan  <dougkwan@google.com>
>
>        * arm.cc (Arm_relobj::section_needs_reloc_stub_scanning,
>        Arm_relobj::scan_sections_for_stubs): Rearrange code to avoid an
>        apparent compiler problem of not folding static constant integral
>        data members of elfcpp::Elf_sizes<32>.
>
> Index: gold/arm.cc
> ===================================================================
> RCS file: /cvs/src/src/gold/arm.cc,v
> retrieving revision 1.54
> diff -u -u -p -r1.54 arm.cc
> --- gold/arm.cc 13 Jan 2010 21:36:47 -0000      1.54
> +++ gold/arm.cc 14 Jan 2010 02:37:58 -0000
> @@ -4203,9 +4203,11 @@ Arm_relobj<big_endian>::section_needs_re
>   if (this->adjust_shndx(shdr.get_sh_link()) != this->symtab_shndx())
>     return false;
>
> -  const unsigned int reloc_size = (sh_type == elfcpp::SHT_REL
> -                                  ? elfcpp::Elf_sizes<32>::rel_size
> -                                  : elfcpp::Elf_sizes<32>::rela_size);
> +  unsigned int reloc_size;
> +  if (sh_type == elfcpp::SHT_REL)
> +    reloc_size = elfcpp::Elf_sizes<32>::rel_size;
> +  else
> +    reloc_size = elfcpp::Elf_sizes<32>::rela_size;
>
>   // Ignore reloc section with unexpected entsize or uneven size.
>   // The error will be reported in the final link.
> @@ -4380,9 +4382,11 @@ Arm_relobj<big_endian>::scan_sections_fo
>          relinfo.reloc_shndx = i;
>          relinfo.data_shndx = index;
>          unsigned int sh_type = shdr.get_sh_type();
> -         const unsigned int reloc_size = (sh_type == elfcpp::SHT_REL
> -                                          ? elfcpp::Elf_sizes<32>::rel_size
> -                                          : elfcpp::Elf_sizes<32>::rela_size);
> +         unsigned int reloc_size;
> +         if (sh_type == elfcpp::SHT_REL)
> +           reloc_size = elfcpp::Elf_sizes<32>::rel_size;
> +         else
> +           reloc_size = elfcpp::Elf_sizes<32>::rela_size;
>
>          Output_section* os = out_sections[index];
>          arm_target->scan_section_for_stubs(&relinfo, sh_type, prelocs,
>

Re: [GOLD][COMMITTED] Fix build breakage with -g.

[Ian Lance Taylor]

"Doug Kwan (關振德)" <dougkwan@google.com> writes:

> I did a bit more research and read the C++ standard again, especially
> in [class.static.data].  Paragraph 4 of it says the following:
>
> "If a static data member is of const integral or const enumeration
> type, its declaration in the class definition can specify a
> constant-initializer which shall be ben integral constant expression
> (5.19).  In that case, the member can appear in integral constant
> expressions within its scope.  The member shall still be defined in a
> namespace scope if it is used in the program and the namespace scope
> definition shall not contain an initializer."
>
> Since the static const integral data members of
> elfcpp::Elf_sizes<size> are used in many places outside of the class
> scope, I think we should add definitions of them in the elfcpp
> namespace.

Yes, it's technically required but I've been avoiding it because I
don't particularly want to add any .cc files in elfcpp.  It works fine
with g++ as long as you avoid the case you just noticed of ?:.  I
think it fails there because of an old g++ extension in which ?: is an
lvalue.  In C++0x the definitions are not required.

Ian

Re: [GOLD][COMMITTED] Fix build breakage with -g.

[Lawrence Crowl]

On 1/13/10, Ian Lance Taylor <iant@google.com> wrote:
> "Doug Kwan (關振德)" <dougkwan@google.com> writes:
> > I did a bit more research and read the C++ standard again, especially
> > in [class.static.data].  Paragraph 4 of it says the following:
> >
> > "If a static data member is of const integral or const enumeration
> > type, its declaration in the class definition can specify a
> > constant-initializer which shall be ben integral constant expression
> > (5.19).  In that case, the member can appear in integral constant
> > expressions within its scope.  The member shall still be defined in a
> > namespace scope if it is used in the program and the namespace scope
> > definition shall not contain an initializer."
> >
> > Since the static const integral data members of
> > elfcpp::Elf_sizes<size> are used in many places outside of the class
> > scope, I think we should add definitions of them in the elfcpp
> > namespace.
>
> Yes, it's technically required but I've been avoiding it because I
> don't particularly want to add any .cc files in elfcpp.  It works fine
> with g++ as long as you avoid the case you just noticed of ?:.  I
> think it fails there because of an old g++ extension in which ?: is an
> lvalue.  In C++0x the definitions are not required.

In C++98 and C++0x an ?: expression can return an lvalue.  This behavior
has not changed.  Something more subtle must be going on to not require
the definition.

BTW, is it the intent that Gold only be compilable by g++?

-- 
Lawrence Crowl

Re: [GOLD][COMMITTED] Fix build breakage with -g.

[Ian Lance Taylor]

Lawrence Crowl <crowl@google.com> writes:

> BTW, is it the intent that Gold only be compilable by g++?

That is not a goal, but I don't plan to add any portability fixes
until somebody actually tries building it with some other compiler and
sends me patches.

In the case at hand I expect that most compilers do not require these
constants to have a definition, because they are simple integer
constants which can always be inlined.

Ian