Re: Patch for ICF string inline bug for SHT_REL sections.

[Sriraman Tallam <tmsriram at google dot com>]

Hi,

    Here is the new patch with the changes and the test case added.
Please let me know what you think.

	* arm.cc (Target_arm<big_endian>::gc_process_relocs): Add template
	parameter to the call to gold::gc_process_relocs.
	* i386.cc (Target_i386<big_endian>::gc_process_relocs): Add template
	parameter to the call to gold::gc_process_relocs.
	* x86_64.cc (Target_x86_64<big_endian>::gc_process_relocs): Add template
	parameter to the call to gold::gc_process_relocs.
	* powerpc.cc (Target_powerpc<big_endian>::gc_process_relocs): Add
	template parameter to the call to gold::gc_process_relocs.
	* sparc.cc (Target_sparc<big_endian>::gc_process_relocs): Add template
	parameter to the call to gold::gc_process_relocs.
	* gc.h (get_embedded_addend_size): New function.
	(gc_process_relocs): Save the size of the reloc for use by ICF.
	* icf.cc (get_section_contents): Get the addend from the text section
	for SHT_REL relocation sections.
	* icf.h (Icf::Reloc_addend_size_info): New typedef.
	(Icf::Reloc_info): Add new member reloc_addend_size_info.
	* testsuite/Makefile.am (icf_sht_rel_addend_test): New test.
	* testsuite/icf_sht_rel_addend_test.sh: New file.
	* testsuite/icf_sht_rel_addend_test_1.cc: New file.
	* testsuite/icf_sht_rel_addend_test_2.cc: New file.


Thanks,
-Sri.

On Mon, Jul 19, 2010 at 12:16 AM, Ian Lance Taylor <iant@google.com> wrote:
> Sriraman Tallam <tmsriram@google.com> writes:
>
>> 2010-07-18 ?Sriraman Tallam ?<tmsriram@google.com>
>>
>> ? ? ? * gc.h (gc_process_relocs): Save the section header type for use by
>> ? ? ? ICF.
>> ? ? ? * icf.cc (get_section_contents): Get the addend from the text section
>> ? ? ? for SHT_REL relocation sections.
>> ? ? ? * icf.h (Icf::Sh_type_info): New typedef.
>> ? ? ? (Icf::Reloc_info): Add new member sh_type_info.
>
> Can we add a test case?
>
>> + ? ? ? ? ? ? ? // For SHT_REL relocation sections, the addend is stored in the
>> + ? ? ? ? ? ? ? // relocation offset in the text section as a 32-bit value.
>
> That's not true in general. ?It's not even true on x86. ?The R_386_16
> reloc stores a 16-bit value in the section. ?What you need here is
> something similar to Relocatable_relocs::Reloc_strategy, in which the
> general code asks the target for the size of the addend, among other
> things. ?By default on i386 that uses Relocatable_size_for_reloc, which
> you should be able to reuse for this.
>
> Also I note that saving the relocation section type for each reloc is
> overkill, though it may not matter much.
>
> I'm not sure you need strings to show the problem, I think it would
> occur in code like
>
> int a[100];
> int f1() { return a[1]; }
> int f2() { return a[2]; }
>
> Ian
>

Attachment: icf_addend_sht_rel_bug.txt
Description: Text document