* [Patch, microblaze, gas, bfd] PR/14736 Correct adjustment of global symbols after relax
@ 2012-11-22 12:05 David Holsgrove
2012-11-29 2:02 ` Michael Eager
0 siblings, 1 reply; 4+ messages in thread
From: David Holsgrove @ 2012-11-22 12:05 UTC (permalink / raw)
To: binutils
Cc: John Williams, Michael Eager, Vinod Kathail, Tom Shui,
Vidhumouli Hunsigida, Nagaraju Mekala, Edgar E. Iglesias
[-- Attachment #1: Type: text/plain, Size: 794 bytes --]
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
[-- Attachment #2: 0002-bfd-bfd-elf32-microblaze.c-Correct-adjustment-of-glo.patch --]
[-- Type: application/octet-stream, Size: 7175 bytes --]
From b3645961a3714e92706f75ab6031e6f5e66e91df 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 | 46 ++++++++++++++++++++++-----
gas/testsuite/gas/microblaze/relax_size.elf | 32 +++++++++++++++++++
gas/testsuite/gas/microblaze/relax_size.exp | 22 +++++++++++++
gas/testsuite/gas/microblaze/relax_size.s | 7 ++++
4 files changed, 99 insertions(+), 8 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
diff --git a/bfd/elf32-microblaze.c b/bfd/elf32-microblaze.c
index be2de13..71daa3c 100644
--- a/bfd/elf32-microblaze.c
+++ b/bfd/elf32-microblaze.c
@@ -1273,6 +1273,27 @@ microblaze_elf_merge_private_bfd_data (bfd * ibfd, bfd * obfd)
/* Calculate fixup value for reference. */
static int
+calc_size_fixup (bfd_vma start, bfd_vma size, asection *sec)
+{
+ bfd_vma end = start + size;
+ int i, fixup = 0;
+
+ if (sec == NULL || sec->relax == NULL)
+ return 0;
+
+ /* Look for addr in relax table, total fixup value. */
+ for (i = 0; i < sec->relax_count; i++)
+ {
+ if (end < sec->relax[i].addr)
+ break;
+ if (start > sec->relax[i].addr)
+ continue;
+ fixup += sec->relax[i].size;
+ }
+ return fixup;
+}
+
+static int
calc_fixup (bfd_vma addr, asection *sec)
{
int i, fixup = 0;
@@ -1784,23 +1805,32 @@ microblaze_elf_relax_section (bfd *abfd,
isymend = isymbuf + symtab_hdr->sh_info;
for (isym = isymbuf; isym < isymend; isym++)
{
- if (isym->st_shndx == shndx)
- isym->st_value =- calc_fixup (isym->st_value, sec);
+ if (isym->st_shndx == shndx) {
+ int count, count_size;
+
+ count = calc_fixup (isym->st_value, sec);
+ count_size = calc_size_fixup (isym->st_value, isym->st_size, sec);
+ isym->st_value -= count;
+ isym->st_size -= count_size;
+ }
}
/* 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);
+ int count, count_size;
+
+ count = calc_fixup (sym_hash->root.u.def.value, sec);
+ count_size = calc_size_fixup (sym_hash->root.u.def.value, sym_hash->size, sec);
+ sym_hash->root.u.def.value -= count;
+ sym_hash->size -= count_size;
}
}
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..16ef447
--- /dev/null
+++ b/gas/testsuite/gas/microblaze/relax_size.exp
@@ -0,0 +1,22 @@
+
+proc ld_test { objects ldflags dest test } {
+ set ld_output [target_link $objects $dest $ldflags]
+}
+
+proc readelf_test { 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] {
+ gas_test "relax_size.s" {-o relax_size.o} {} {assembling relax_size}
+ ld_test {relax_size.o } {-e 0 -N -relax } {relax_size.x} {linking relax_size.x}
+ readelf_test {relax_size.x} {-s} {relax_size.elf} {readelf -s relax_size.x }
+ regexp_test {relax_size.elf} "$srcdir/$subdir/relax_size.elf" {matching readelf}
+}
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
--
1.7.9.5
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch, microblaze, gas, bfd] PR/14736 Correct adjustment of global symbols after relax
2012-11-22 12:05 [Patch, microblaze, gas, bfd] PR/14736 Correct adjustment of global symbols after relax David Holsgrove
@ 2012-11-29 2:02 ` Michael Eager
2012-12-13 13:26 ` David Holsgrove
0 siblings, 1 reply; 4+ messages in thread
From: Michael Eager @ 2012-11-29 2:02 UTC (permalink / raw)
To: David Holsgrove
Cc: binutils, John Williams, Vinod Kathail, Tom Shui,
Vidhumouli Hunsigida, Nagaraju Mekala, Edgar E. Iglesias
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?
+ 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).
--
Michael Eager eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [Patch, microblaze, gas, bfd] PR/14736 Correct adjustment of global symbols after relax
2012-11-29 2:02 ` Michael Eager
@ 2012-12-13 13:26 ` David Holsgrove
2012-12-18 16:04 ` Michael Eager
0 siblings, 1 reply; 4+ messages in thread
From: David Holsgrove @ 2012-12-13 13:26 UTC (permalink / raw)
To: Michael Eager
Cc: binutils, John Williams, Vinod Kathail, Tom Shui,
Vidhumouli Hunsigida, Nagaraju Mekala, Edgar E. Iglesias
[-- Attachment #1: Type: text/plain, Size: 1959 bytes --]
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
>
>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-bfd-bfd-elf32-microblaze.c-Correct-adjustment-of-glo.patch --]
[-- Type: text/x-patch; name="0001-bfd-bfd-elf32-microblaze.c-Correct-adjustment-of-glo.patch", Size: 13955 bytes --]
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch, microblaze, gas, bfd] PR/14736 Correct adjustment of global symbols after relax
2012-12-13 13:26 ` David Holsgrove
@ 2012-12-18 16:04 ` Michael Eager
0 siblings, 0 replies; 4+ messages in thread
From: Michael Eager @ 2012-12-18 16:04 UTC (permalink / raw)
To: David Holsgrove
Cc: binutils, John Williams, Vinod Kathail, Tom Shui,
Vidhumouli Hunsigida, Nagaraju Mekala, Edgar E. Iglesias
On 12/13/2012 05:26 AM, David Holsgrove wrote:
> 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.
Committed.
Please submit a revised ChangeLog if it changes when you submit a revision.
Also, make sure the ChangeLog reflects all new/changed files.
This is the revised change log:
2012-12-18 Edgar E. Iglesias <edgar.iglesias@gmail.com>
PR ld/14736
* elf32-microblaze.c (calc_fixup): Add end range.
2012-12-18 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
* gas/microblaze/relax_size2.s: Likewise
* gas/microblaze/relax_size2.elf: Likewise
--
Michael Eager eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-12-18 16:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-22 12:05 [Patch, microblaze, gas, bfd] PR/14736 Correct adjustment of global symbols after relax David Holsgrove
2012-11-29 2:02 ` Michael Eager
2012-12-13 13:26 ` David Holsgrove
2012-12-18 16:04 ` Michael Eager
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).