This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
RE: [Patch, microblaze, gas, bfd] PR/14736 Correct adjustment of global symbols after relax
- From: David Holsgrove <david dot holsgrove at xilinx dot com>
- To: Michael Eager <eager at eagerm dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>, John Williams <jwilliams at xilinx dot com>, Vinod Kathail <vinodk at xilinx dot com>, Tom Shui <tshui at xilinx dot com>, Vidhumouli Hunsigida <vidhum at xilinx dot com>, Nagaraju Mekala <nmekala at xilinx dot com>, "Edgar E. Iglesias" <edgar dot iglesias at gmail dot com>
- Date: Thu, 13 Dec 2012 13:26:18 +0000
- Subject: RE: [Patch, microblaze, gas, bfd] PR/14736 Correct adjustment of global symbols after relax
- References: <30395d74-d0a8-48c7-ab3d-8171cdfaedc0@DB3EHSMHS001.ehs.local>,<50B6C23C.9050306@eagerm.com>
Hi Michael,
>On 11/22/2012 04:05 AM, David Holsgrove wrote:
>>
>> Fixup symbol sizes after linker relaxation
>>
>> Correct an off-by one when comparing relaxation addresses
>> with symbol start.
>>
>> Also addresses PR/14736 where clang gave warning;
>> use of unary operator that may be intended as
>> compound assignment (-=)
>>
>> binutils/bfd/Changelog
>>
>> 2012-11-22 Edgar E. Iglesias <edgar.iglesias@gmail.com>
>>
>> * elf32-microblaze.c (calc_size_fixup): New
>> (calc_fixup): Use calc_size_fixup to adjust
>> object size
>>
>> binutils/gas/testsuite/Changelog
>>
>> 2012-11-22 David Holsgrove <david.holsgrove@xilinx.com>
>>
>> * gas/microblaze/relax_size.exp: New file - test
>> object size after linker relaxation
>> * gas/microblaze/relax_size.s: Likewise
>> * gas/microblaze/relax_size.elf: Likewise
>
>calc_size_fixup() is almost the same as calc_fixup(). Comments say they
>do the same thing. Are both needed? When would the values returned be different?
>
Thanks again for the review, the patch has been reworked to collapse the extra functionality that
calc_size_fixup added (the ability to calculate fixup for the displacement between two points,
instead of from 0) into the original calc_fixup function.
All instances where the old calc_fixup call took place have been amended to specify a 0 size.
>
>+ int count, count_size;
>+
>+ count = calc_fixup (isym->st_value, sec);
>+ count_size = calc_size_fixup (isym->st_value, isym->st_size, sec);
>
>The values returned by calc_fixup() and calc_size_fixup() are not counts, but
>the total size of the fixups. Please use more descriptive names (2 places).
Resolved in attached patch.
thanks,
David
>
>--
>Michael Eager eager@eagercon.com
>1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
>
>
From bb5b5c49c6fb00fa1fc8da5c8911b1978cce6e26 Mon Sep 17 00:00:00 2001
From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Date: Thu, 17 Nov 2011 23:44:38 +0100
Subject: [PATCH] bfd/ * bfd/elf32-microblaze.c: Correct adjustment of global
symbols
Fixup symbol sizes after linker relaxation
This bug didn't affect code generation, but would have
affected debuggers that got the wrong view of things.
Correct an off-by one when comparing relaxation addresses
with symbol start.
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Signed-off-by: David Holsgrove <david.holsgrove@xilinx.com>
---
bfd/elf32-microblaze.c | 55 +++++++++++++++-----------
gas/testsuite/gas/microblaze/relax_size.elf | 32 +++++++++++++++
gas/testsuite/gas/microblaze/relax_size.exp | 25 ++++++++++++
gas/testsuite/gas/microblaze/relax_size.s | 7 ++++
gas/testsuite/gas/microblaze/relax_size2.elf | 34 ++++++++++++++++
gas/testsuite/gas/microblaze/relax_size2.s | 11 ++++++
6 files changed, 141 insertions(+), 23 deletions(-)
create mode 100644 gas/testsuite/gas/microblaze/relax_size.elf
create mode 100644 gas/testsuite/gas/microblaze/relax_size.exp
create mode 100644 gas/testsuite/gas/microblaze/relax_size.s
create mode 100644 gas/testsuite/gas/microblaze/relax_size2.elf
create mode 100644 gas/testsuite/gas/microblaze/relax_size2.s
diff --git a/bfd/elf32-microblaze.c b/bfd/elf32-microblaze.c
index 9d0a7ca..8aafe72 100644
--- a/bfd/elf32-microblaze.c
+++ b/bfd/elf32-microblaze.c
@@ -1600,8 +1600,9 @@ microblaze_elf_merge_private_bfd_data (bfd * ibfd, bfd * obfd)
/* Calculate fixup value for reference. */
static int
-calc_fixup (bfd_vma addr, asection *sec)
+calc_fixup (bfd_vma start, bfd_vma size, asection *sec)
{
+ bfd_vma end = start + size;
int i, fixup = 0;
if (sec == NULL || sec->relax == NULL)
@@ -1610,11 +1611,12 @@ calc_fixup (bfd_vma addr, asection *sec)
/* Look for addr in relax table, total fixup value. */
for (i = 0; i < sec->relax_count; i++)
{
- if (addr <= sec->relax[i].addr)
+ if (end <= sec->relax[i].addr)
break;
+ if ((end != start) && (start > sec->relax[i].addr))
+ continue;
fixup += sec->relax[i].size;
}
-
return fixup;
}
@@ -1827,7 +1829,7 @@ microblaze_elf_relax_section (bfd *abfd,
bfd_vma nraddr;
/* Get the new reloc address. */
- nraddr = irel->r_offset - calc_fixup (irel->r_offset, sec);
+ nraddr = irel->r_offset - calc_fixup (irel->r_offset, 0, sec);
switch ((enum elf_microblaze_reloc_type) ELF32_R_TYPE (irel->r_info))
{
default:
@@ -1845,7 +1847,7 @@ microblaze_elf_relax_section (bfd *abfd,
/* Only handle relocs against .text. */
if (isym->st_shndx == shndx
&& ELF32_ST_TYPE (isym->st_info) == STT_SECTION)
- irel->r_addend -= calc_fixup (irel->r_addend, sec);
+ irel->r_addend -= calc_fixup (irel->r_addend, 0, sec);
}
break;
case R_MICROBLAZE_NONE:
@@ -1855,8 +1857,8 @@ microblaze_elf_relax_section (bfd *abfd,
int sfix, efix;
bfd_vma target_address;
target_address = irel->r_addend + irel->r_offset;
- sfix = calc_fixup (irel->r_offset, sec);
- efix = calc_fixup (target_address, sec);
+ sfix = calc_fixup (irel->r_offset, 0, sec);
+ efix = calc_fixup (target_address, 0, sec);
irel->r_addend -= (efix - sfix);
/* Should use HOWTO. */
microblaze_bfd_write_imm_value_32 (abfd, contents + irel->r_offset,
@@ -1870,8 +1872,8 @@ microblaze_elf_relax_section (bfd *abfd,
int sfix, efix;
bfd_vma target_address;
target_address = irel->r_addend + irel->r_offset + INST_WORD_SIZE;
- sfix = calc_fixup (irel->r_offset + INST_WORD_SIZE, sec);
- efix = calc_fixup (target_address, sec);
+ sfix = calc_fixup (irel->r_offset + INST_WORD_SIZE, 0, sec);
+ efix = calc_fixup (target_address, 0, sec);
irel->r_addend -= (efix - sfix);
microblaze_bfd_write_imm_value_32 (abfd, contents + irel->r_offset
+ INST_WORD_SIZE, irel->r_addend);
@@ -1934,7 +1936,7 @@ microblaze_elf_relax_section (bfd *abfd,
}
}
- irelscan->r_addend -= calc_fixup (irelscan->r_addend, sec);
+ irelscan->r_addend -= calc_fixup (irelscan->r_addend, 0, sec);
}
else if (ELF32_R_TYPE (irelscan->r_info) == (int) R_MICROBLAZE_32_SYM_OP_SYM)
{
@@ -1965,6 +1967,7 @@ microblaze_elf_relax_section (bfd *abfd,
}
irelscan->r_addend -= calc_fixup (irel->r_addend
+ isym->st_value,
+ 0,
sec);
}
}
@@ -2005,7 +2008,7 @@ microblaze_elf_relax_section (bfd *abfd,
unsigned long instr = bfd_get_32 (abfd, ocontents + irelscan->r_offset);
immediate = instr & 0x0000ffff;
target_address = immediate;
- offset = calc_fixup (target_address, sec);
+ offset = calc_fixup (target_address, 0, sec);
immediate -= offset;
irelscan->r_addend -= offset;
microblaze_bfd_write_imm_value_32 (abfd, ocontents + irelscan->r_offset,
@@ -2052,7 +2055,7 @@ microblaze_elf_relax_section (bfd *abfd,
+ INST_WORD_SIZE);
immediate = (instr_hi & 0x0000ffff) << 16;
immediate |= (instr_lo & 0x0000ffff);
- offset = calc_fixup (irelscan->r_addend, sec);
+ offset = calc_fixup (irelscan->r_addend, 0, sec);
immediate -= offset;
irelscan->r_addend -= offset;
}
@@ -2097,7 +2100,7 @@ microblaze_elf_relax_section (bfd *abfd,
immediate = (instr_hi & 0x0000ffff) << 16;
immediate |= (instr_lo & 0x0000ffff);
target_address = immediate;
- offset = calc_fixup (target_address, sec);
+ offset = calc_fixup (target_address, 0, sec);
immediate -= offset;
irelscan->r_addend -= offset;
microblaze_bfd_write_imm_value_64 (abfd, ocontents
@@ -2112,24 +2115,30 @@ microblaze_elf_relax_section (bfd *abfd,
for (isym = isymbuf; isym < isymend; isym++)
{
if (isym->st_shndx == shndx)
- isym->st_value =- calc_fixup (isym->st_value, sec);
+ {
+ isym->st_value -= calc_fixup (isym->st_value, 0, sec);
+ if (isym->st_size)
+ isym->st_size -= calc_fixup (isym->st_value, isym->st_size, sec);
+ }
}
/* Now adjust the global symbols defined in this section. */
isym = isymbuf + symtab_hdr->sh_info;
- isymend = isymbuf + (symtab_hdr->sh_size / sizeof (Elf32_External_Sym));
- for (sym_index = 0; isym < isymend; isym++, sym_index++)
+ symcount = (symtab_hdr->sh_size / sizeof (Elf32_External_Sym)) - symtab_hdr->sh_info;
+ for (sym_index = 0; sym_index < symcount; sym_index++)
{
sym_hash = elf_sym_hashes (abfd)[sym_index];
- if (isym->st_shndx == shndx
- && (sym_hash->root.type == bfd_link_hash_defined
+ if ((sym_hash->root.type == bfd_link_hash_defined
|| sym_hash->root.type == bfd_link_hash_defweak)
&& sym_hash->root.u.def.section == sec)
- {
- sym_hash->root.u.def.value -= calc_fixup (sym_hash->root.u.def.value,
- sec);
- }
- }
+ {
+ sym_hash->root.u.def.value -= calc_fixup (sym_hash->root.u.def.value,
+ 0, sec);
+ if (sym_hash->size)
+ sym_hash->size -= calc_fixup (sym_hash->root.u.def.value,
+ sym_hash->size, sec);
+ }
+ }
/* Physically move the code and change the cooked size. */
dest = sec->relax[0].addr;
diff --git a/gas/testsuite/gas/microblaze/relax_size.elf b/gas/testsuite/gas/microblaze/relax_size.elf
new file mode 100644
index 0000000..cf23ea6
--- /dev/null
+++ b/gas/testsuite/gas/microblaze/relax_size.elf
@@ -0,0 +1,32 @@
+
+Symbol table '.symtab' contains 29 entries:
+ Num: Value Size Type Bind Vis Ndx Name
+ 0: 00000000 0 NOTYPE LOCAL DEFAULT UND
+ 1: 00000050 0 SECTION LOCAL DEFAULT 1
+ 2: 00000058 0 SECTION LOCAL DEFAULT 2
+ 3: 00000000 0 FILE LOCAL DEFAULT ABS relax_size.o
+ 4: 00000050 8 NOTYPE LOCAL DEFAULT 1 func
+ 5: 00000058 0 NOTYPE LOCAL DEFAULT 1 label
+ 6: 00000000 0 FILE LOCAL DEFAULT ABS
+ 7: 00000058 0 NOTYPE GLOBAL DEFAULT 2 _fdata
+ 8: 00000058 0 NOTYPE GLOBAL DEFAULT 1 _etext
+ 9: 00000058 0 NOTYPE GLOBAL DEFAULT 2 _essrw
+ 10: 00000058 0 NOTYPE GLOBAL DEFAULT 1 _heap_end
+ 11: 00000058 0 NOTYPE GLOBAL DEFAULT 1 _heap_start
+ 12: 00000000 0 NOTYPE GLOBAL DEFAULT ABS _ssro_size
+ 13: 00000050 0 NOTYPE GLOBAL DEFAULT 1 _ftext
+ 14: 00000058 0 NOTYPE GLOBAL DEFAULT 2 _essro
+ 15: 00000400 0 NOTYPE GLOBAL DEFAULT ABS _STACK_SIZE
+ 16: 00000000 0 NOTYPE GLOBAL DEFAULT ABS _HEAP_SIZE
+ 17: 00000000 0 NOTYPE GLOBAL DEFAULT ABS _ssrw_size
+ 18: 00000058 0 NOTYPE GLOBAL DEFAULT 2 _stack_end
+ 19: 00000058 0 NOTYPE GLOBAL DEFAULT 2 _edata
+ 20: 00000458 0 NOTYPE GLOBAL DEFAULT 2 _end
+ 21: 00000058 0 NOTYPE GLOBAL DEFAULT 1 _heap
+ 22: 00000058 0 NOTYPE GLOBAL DEFAULT 2 _ssro
+ 23: 00000058 0 NOTYPE GLOBAL DEFAULT 2 _ssrw
+ 24: 00000458 0 NOTYPE GLOBAL DEFAULT 2 _stack
+ 25: 00000050 0 NOTYPE GLOBAL DEFAULT ABS _TEXT_START_ADDR
+ 26: 00000058 0 NOTYPE GLOBAL DEFAULT 2 _frodata
+ 27: 00000058 0 NOTYPE GLOBAL DEFAULT 2 _fbss
+ 28: 00000058 0 NOTYPE GLOBAL DEFAULT 2 _erodata
diff --git a/gas/testsuite/gas/microblaze/relax_size.exp b/gas/testsuite/gas/microblaze/relax_size.exp
new file mode 100644
index 0000000..a733dc8
--- /dev/null
+++ b/gas/testsuite/gas/microblaze/relax_size.exp
@@ -0,0 +1,25 @@
+
+proc ld_run { objects ldflags dest test } {
+ set ld_output [target_link $objects $dest $ldflags]
+}
+
+proc readelf_run { exec flags dest test } {
+ set readelf [find_binutils_prog readelf]
+ verbose -log "$readelf $flags $exec > $dest"
+ catch "exec $readelf $flags $exec > $dest" readelf_output
+}
+
+proc regexp_test { file1 file2 test } {
+ if [regexp_diff $file1 $file2] then { fail $test } else { pass $test }
+}
+
+global srcdir subdir
+if [istarget microblaze*-*-elf] {
+ foreach file [lsort [glob -nocomplain -- $srcdir/$subdir/relax_size*.s]] {
+ set file [file rootname [file tail $file]]
+ gas_run "$file.s" "-o $file.o" ""
+ ld_run "$file.o" "-e 0 -N -relax" "$file.x" "linking $file.x"
+ readelf_run "$file.x" "-s" "$file.elf" "readelf -s $file.x"
+ regexp_test "$file.elf" "$srcdir/$subdir/$file.elf" "matching $file.elf"
+ }
+}
diff --git a/gas/testsuite/gas/microblaze/relax_size.s b/gas/testsuite/gas/microblaze/relax_size.s
new file mode 100644
index 0000000..6b25977
--- /dev/null
+++ b/gas/testsuite/gas/microblaze/relax_size.s
@@ -0,0 +1,7 @@
+ .org 0
+ .section .text
+func:
+ braid label
+ nop
+label:
+ .size func, . - func
diff --git a/gas/testsuite/gas/microblaze/relax_size2.elf b/gas/testsuite/gas/microblaze/relax_size2.elf
new file mode 100644
index 0000000..fbdcc0a
--- /dev/null
+++ b/gas/testsuite/gas/microblaze/relax_size2.elf
@@ -0,0 +1,34 @@
+
+Symbol table '.symtab' contains 31 entries:
+ Num: Value Size Type Bind Vis Ndx Name
+ 0: 00000000 0 NOTYPE LOCAL DEFAULT UND
+ 1: 00000050 0 SECTION LOCAL DEFAULT 1
+ 2: 00000060 0 SECTION LOCAL DEFAULT 2
+ 3: 00000000 0 FILE LOCAL DEFAULT ABS relax_size2.o
+ 4: 00000050 4 NOTYPE LOCAL DEFAULT 1 func
+ 5: 00000054 0 NOTYPE LOCAL DEFAULT 1 label
+ 6: 00000054 8 NOTYPE LOCAL DEFAULT 1 func2
+ 7: 0000005c 0 NOTYPE LOCAL DEFAULT 1 label2
+ 8: 00000000 0 FILE LOCAL DEFAULT ABS
+ 9: 00000060 0 NOTYPE GLOBAL DEFAULT 2 _fdata
+ 10: 0000005c 0 NOTYPE GLOBAL DEFAULT 1 _etext
+ 11: 00000060 0 NOTYPE GLOBAL DEFAULT 2 _essrw
+ 12: 00000060 0 NOTYPE GLOBAL DEFAULT 1 _heap_end
+ 13: 00000060 0 NOTYPE GLOBAL DEFAULT 1 _heap_start
+ 14: 00000000 0 NOTYPE GLOBAL DEFAULT ABS _ssro_size
+ 15: 00000050 0 NOTYPE GLOBAL DEFAULT 1 _ftext
+ 16: 00000060 0 NOTYPE GLOBAL DEFAULT 2 _essro
+ 17: 00000400 0 NOTYPE GLOBAL DEFAULT ABS _STACK_SIZE
+ 18: 00000000 0 NOTYPE GLOBAL DEFAULT ABS _HEAP_SIZE
+ 19: 00000000 0 NOTYPE GLOBAL DEFAULT ABS _ssrw_size
+ 20: 00000060 0 NOTYPE GLOBAL DEFAULT 2 _stack_end
+ 21: 00000060 0 NOTYPE GLOBAL DEFAULT 2 _edata
+ 22: 00000460 0 NOTYPE GLOBAL DEFAULT 2 _end
+ 23: 00000060 0 NOTYPE GLOBAL DEFAULT 1 _heap
+ 24: 00000060 0 NOTYPE GLOBAL DEFAULT 2 _ssro
+ 25: 00000060 0 NOTYPE GLOBAL DEFAULT 2 _ssrw
+ 26: 00000460 0 NOTYPE GLOBAL DEFAULT 2 _stack
+ 27: 00000050 0 NOTYPE GLOBAL DEFAULT ABS _TEXT_START_ADDR
+ 28: 0000005c 0 NOTYPE GLOBAL DEFAULT 2 _frodata
+ 29: 00000060 0 NOTYPE GLOBAL DEFAULT 2 _fbss
+ 30: 0000005c 0 NOTYPE GLOBAL DEFAULT 2 _erodata
diff --git a/gas/testsuite/gas/microblaze/relax_size2.s b/gas/testsuite/gas/microblaze/relax_size2.s
new file mode 100644
index 0000000..dedabfb
--- /dev/null
+++ b/gas/testsuite/gas/microblaze/relax_size2.s
@@ -0,0 +1,11 @@
+ .org 0
+ .section .text
+func:
+ nop
+label:
+ .size func, . - func
+func2:
+ braid label2
+ nop
+label2:
+ .size func2, . - func2
--
1.7.9.5