This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] gold: fix some signed-unsigned comparison warnings
On Thu, Nov 1, 2012 at 1:57 PM, Roland McGrath <mcgrathr@google.com> wrote:
> On Thu, Nov 1, 2012 at 1:41 PM, Cary Coutant <ccoutant@google.com> wrote:
>> Seems to me that offset_in_output_section is misdeclared here. As
>> passed from relocate_relocs(), the actual parameter is declared as an
>> Elf_Addr, which is unsigned. I think offset_in_output_section should
>> be Arm_address here.
>
> It seems to me the really proper type here is Elf_Off (albeit that is
> actually identical to Elf_Addr), since what this refers to an offset in an
> ELF file. In fact, I think most of the uses of off_t throughout gold
> really ought to be Elf_Off, but I'm not trying to clean everything up
> right now.
In gold, a file offset should normally be off_t. It's true that it
can be Elf_Off in templatized code, but there is a fair amount of
non-templatized code in gold that deals with file offsets, and for
those it should be off_t.
However, when we have a view in a section, an offset into the section
should be section_offset_type, not off_t. I agree this is confusing,
but it makes a significant performance difference when building on
32-bit systems to use section_offset_type rather than off_t when
appropriate.
I think it's pretty hard to justify changing the type of
offset_in_output_section in Target::relocate_special_relocatable
without also changing the type of offset_in_output_section in
Target::relocate_relocs. This is templatized code so we could make
them both Elf_Off. Or we could make them both section_offset_type.
The other changes (dwarf_reader.cc, output.cc, incremental.cc) are fine.
Thanks.
Ian