* [PATCH]: HC11 linker relax fixes (potential pb on other relax targets)
@ 2003-04-04 21:19 Stephane Carrez
2003-04-08 12:41 ` Alan Modra
0 siblings, 1 reply; 3+ messages in thread
From: Stephane Carrez @ 2003-04-04 21:19 UTC (permalink / raw)
To: binutils
[-- Attachment #1: Type: text/plain, Size: 1771 bytes --]
Hi!
I committed this patch to fix problems in HC11 linker relaxations.
The first 3 points are HC11 specific but the last point is present in
other relaxing targets (I think):
- we must use a PCREL reloc when long branch is converted to a short branch
and we must not compute the branch offset now (the reloc will ensure correct offset)
- a jsr can be changed to bsr if the address is in page0 (0..0x0ff)
- a far symbol must not be relaxed because we will need to use its trampoline
address (and we don't know where it is at this stage).
- when we adjust the symbols after removing bytes, we must take into
account the symbol that marks the end of the section. The test that exists
in the bfd relax targets (and that I copied!) is:
if (isym->st_shndx == sec_shndx
&& isym->st_value > addr
&& isym->st_value < toaddr)
and it is not correct if there is a section that defines a label at its very end.
It will not be adjusted. In general such symbol is defined to compute the
size of some code and it will thus report the size before relax instead of the
final size.
Stephane
2003-04-04 Stephane Carrez <stcarrez@nerim.fr>
* elf32-m68hc11.c (m68hc11_elf_relax_delete_bytes): Also adjust
symbols that mark the end of the section.
(m68hc11_elf_relax_section): Use R_M68HC11_PCREL_8 relocs when
converting to a relative branch so that the offset is computed after
the relaxation; also relocate a jsr into a bsr if possible but don't
relax them if they are to a far symbol as we need to call the
trampoline code.
(elf_m68hc11_howto_table): Set pcrel_offset to true.
2003-04-04 Stephane Carrez <stcarrez@nerim.fr>
* ld-m68hc11/bug-1417.d: Update to take into account jsr->bsr relax.
[-- Attachment #2: relax.diffs --]
[-- Type: text/plain, Size: 7499 bytes --]
Index: bfd/elf32-m68hc11.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-m68hc11.c,v
retrieving revision 1.15
diff -u -p -r1.15 elf32-m68hc11.c
--- bfd/elf32-m68hc11.c 1 Dec 2002 13:24:03 -0000 1.15
+++ bfd/elf32-m68hc11.c 4 Apr 2003 20:54:13 -0000
@@ -1,5 +1,5 @@
/* Motorola 68HC11-specific support for 32-bit ELF
- Copyright 1999, 2000, 2001, 2002 Free Software Foundation, Inc.
+ Copyright 1999, 2000, 2001, 2002, 2003 Free Software Foundation, Inc.
Contributed by Stephane Carrez (stcarrez@nerim.fr)
(Heavily copied from the D10V port by Martin Hunt (hunt@cygnus.com))
@@ -143,7 +143,7 @@ static reloc_howto_type elf_m68hc11_howt
FALSE, /* partial_inplace */
0x00ff, /* src_mask */
0x00ff, /* dst_mask */
- FALSE), /* pcrel_offset */
+ TRUE), /* pcrel_offset */
/* A 16 bit absolute relocation */
HOWTO (R_M68HC11_16, /* type */
@@ -204,7 +204,7 @@ static reloc_howto_type elf_m68hc11_howt
FALSE, /* partial_inplace */
0xffff, /* src_mask */
0xffff, /* dst_mask */
- FALSE), /* pcrel_offset */
+ TRUE), /* pcrel_offset */
/* GNU extension to record C++ vtable hierarchy */
HOWTO (R_M68HC11_GNU_VTINHERIT, /* type */
@@ -247,8 +247,8 @@ static reloc_howto_type elf_m68hc11_howt
bfd_elf_generic_reloc, /* special_function */
"R_M68HC11_24", /* name */
FALSE, /* partial_inplace */
- 0xffff, /* src_mask */
- 0xffff, /* dst_mask */
+ 0xffffff, /* src_mask */
+ 0xffffff, /* dst_mask */
FALSE), /* pcrel_offset */
/* A 16-bit low relocation */
@@ -445,6 +445,9 @@ elf32_m68hc11_gc_sweep_hook (abfd, info,
return TRUE;
}
+\f
+/* 68HC11 Linker Relaxation. */
+
struct m68hc11_direct_relax
{
const char *name;
@@ -694,6 +697,7 @@ m68hc11_elf_relax_section (abfd, sec, li
bfd_vma value;
Elf_Internal_Sym *isym;
asection *sym_sec;
+ int is_far = 0;
/* If this isn't something that can be relaxed, then ignore
this reloc. */
@@ -747,7 +751,7 @@ m68hc11_elf_relax_section (abfd, sec, li
prev_insn_group = 0;
/* Do nothing if this reloc is the last byte in the section. */
- if (irel->r_offset == sec->_cooked_size)
+ if (irel->r_offset + 2 >= sec->_cooked_size)
continue;
/* See if the next instruction is an unconditional pc-relative
@@ -793,6 +797,7 @@ m68hc11_elf_relax_section (abfd, sec, li
{
/* A local symbol. */
isym = isymbuf + ELF32_R_SYM (irel->r_info);
+ is_far = isym->st_other & STO_M68HC12_FAR;
sym_sec = bfd_section_from_elf_index (abfd, isym->st_shndx);
symval = (isym->st_value
+ sym_sec->output_section->vma
@@ -818,6 +823,7 @@ m68hc11_elf_relax_section (abfd, sec, li
continue;
}
+ is_far = h->other & STO_M68HC12_FAR;
isym = 0;
sym_sec = h->root.u.def.section;
symval = (h->root.u.def.value
@@ -891,23 +897,25 @@ m68hc11_elf_relax_section (abfd, sec, li
{
code = 0x20;
bfd_put_8 (abfd, code, contents + prev_insn_branch->r_offset);
- bfd_put_8 (abfd, offset,
+ bfd_put_8 (abfd, 0xff,
contents + prev_insn_branch->r_offset + 1);
+ irel->r_offset = prev_insn_branch->r_offset + 1;
irel->r_info = ELF32_R_INFO (ELF32_R_SYM (irel->r_info),
- R_M68HC11_NONE);
+ R_M68HC11_PCREL_8);
m68hc11_elf_relax_delete_bytes (abfd, sec,
- irel->r_offset, 1);
+ irel->r_offset + 1, 1);
}
else
{
code ^= 0x1;
bfd_put_8 (abfd, code, contents + prev_insn_branch->r_offset);
- bfd_put_8 (abfd, offset,
+ bfd_put_8 (abfd, 0xff,
contents + prev_insn_branch->r_offset + 1);
+ irel->r_offset = prev_insn_branch->r_offset + 1;
irel->r_info = ELF32_R_INFO (ELF32_R_SYM (irel->r_info),
- R_M68HC11_NONE);
+ R_M68HC11_PCREL_8);
m68hc11_elf_relax_delete_bytes (abfd, sec,
- irel->r_offset - 1, 3);
+ irel->r_offset + 1, 3);
}
prev_insn_branch = 0;
*again = TRUE;
@@ -991,14 +999,14 @@ m68hc11_elf_relax_section (abfd, sec, li
/* That will change things, so, we should relax again. */
*again = TRUE;
}
- else if (ELF32_R_TYPE (irel->r_info) == R_M68HC11_16)
+ else if (ELF32_R_TYPE (irel->r_info) == R_M68HC11_16 && !is_far)
{
unsigned char code;
bfd_vma offset;
prev_insn_branch = 0;
code = bfd_get_8 (abfd, contents + irel->r_offset - 1);
- if (code == 0x7e)
+ if (code == 0x7e || code == 0xbd)
{
offset = value - (irel->r_offset
+ sec->output_section->vma
@@ -1021,13 +1029,13 @@ m68hc11_elf_relax_section (abfd, sec, li
free_extsyms = NULL;
/* Shrink the branch. */
- code = 0x20;
+ code = (code == 0x7e) ? 0x20 : 0x8d;
bfd_put_8 (abfd, code,
contents + irel->r_offset - 1);
- bfd_put_8 (abfd, offset,
+ bfd_put_8 (abfd, 0xff,
contents + irel->r_offset);
irel->r_info = ELF32_R_INFO (ELF32_R_SYM (irel->r_info),
- R_M68HC11_NONE);
+ R_M68HC11_PCREL_8);
m68hc11_elf_relax_delete_bytes (abfd, sec,
irel->r_offset + 1, 1);
/* That will change things, so, we should relax again. */
@@ -1220,7 +1228,7 @@ m68hc11_elf_relax_delete_bytes (abfd, se
{
if (isym->st_shndx == sec_shndx
&& isym->st_value > addr
- && isym->st_value < toaddr)
+ && isym->st_value <= toaddr)
isym->st_value -= count;
}
@@ -1236,7 +1244,7 @@ m68hc11_elf_relax_delete_bytes (abfd, se
|| sym_hash->root.type == bfd_link_hash_defweak)
&& sym_hash->root.u.def.section == sec
&& sym_hash->root.u.def.value > addr
- && sym_hash->root.u.def.value < toaddr)
+ && sym_hash->root.u.def.value <= toaddr)
{
sym_hash->root.u.def.value -= count;
}
Index: ld/testsuite/ld-m68hc11/bug-1417.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-m68hc11/bug-1417.d,v
retrieving revision 1.1
diff -u -p -r1.1 bug-1417.d
--- ld/testsuite/ld-m68hc11/bug-1417.d 1 Dec 2002 13:25:05 -0000 1.1
+++ ld/testsuite/ld-m68hc11/bug-1417.d 4 Apr 2003 20:54:14 -0000
@@ -8,8 +8,8 @@
Disassembly of section .text:
0+8000 <_start> tst 0+ <__bss_size>
-0+8003 <_start\+0x3> bne 0+8008 <L1>
-0+8005 <_start\+0x5> jsr 0+800c <foo>
-0+8008 <L1> bset \*0+ <__bss_size> \#\$04
-0+800b <L2> rts
-0+800c <foo> rts
+0+8003 <_start\+0x3> bne 0+8007 <L1>
+0+8005 <_start\+0x5> bsr 0+800b <foo>
+0+8007 <L1> bset \*0+ <__bss_size> \#\$04
+0+800a <L2> rts
+0+800b <foo> rts
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH]: HC11 linker relax fixes (potential pb on other relax targets)
2003-04-04 21:19 [PATCH]: HC11 linker relax fixes (potential pb on other relax targets) Stephane Carrez
@ 2003-04-08 12:41 ` Alan Modra
2003-04-09 22:10 ` Stephane Carrez
0 siblings, 1 reply; 3+ messages in thread
From: Alan Modra @ 2003-04-08 12:41 UTC (permalink / raw)
To: Stephane Carrez; +Cc: binutils
On Fri, Apr 04, 2003 at 11:19:41PM +0200, Stephane Carrez wrote:
> - when we adjust the symbols after removing bytes, we must take into
> account the symbol that marks the end of the section. The test that
> exists
> in the bfd relax targets (and that I copied!) is:
>
> if (isym->st_shndx == sec_shndx
> && isym->st_value > addr
> && isym->st_value < toaddr)
>
> and it is not correct if there is a section that defines a label at its
> very end.
> It will not be adjusted.
I wonder why there is any test of symbol value against "toaddr"? It
seems to me that this test is bogus, and your replacement of "< toaddr"
with "<= toaddr" just fixes one particular symbol value. For instance,
suppose we have
.text
nop
nop
sym1:
sym2 = sym1 + 2
and the relax pass decides to delete one of the nops. Shouldn't sym2
be decremented? Of course, you could argue that
sym3 = .text + 4
might be better left alone, but that just demonstrates the fragility
of adjusting symbols based on their value.
--
Alan Modra
IBM OzLabs - Linux Technology Centre
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH]: HC11 linker relax fixes (potential pb on other relax targets)
2003-04-08 12:41 ` Alan Modra
@ 2003-04-09 22:10 ` Stephane Carrez
0 siblings, 0 replies; 3+ messages in thread
From: Stephane Carrez @ 2003-04-09 22:10 UTC (permalink / raw)
To: Alan Modra; +Cc: Stephane Carrez, binutils
Hi!
Alan Modra wrote:
> On Fri, Apr 04, 2003 at 11:19:41PM +0200, Stephane Carrez wrote:
>
>> - when we adjust the symbols after removing bytes, we must take into
>> account the symbol that marks the end of the section. The test that
>> exists
>> in the bfd relax targets (and that I copied!) is:
>>
>> if (isym->st_shndx == sec_shndx
>> && isym->st_value > addr
>> && isym->st_value < toaddr)
>>
>> and it is not correct if there is a section that defines a label at its
>> very end.
>> It will not be adjusted.
>
>
> I wonder why there is any test of symbol value against "toaddr"? It
> seems to me that this test is bogus, and your replacement of "< toaddr"
> with "<= toaddr" just fixes one particular symbol value. For instance,
> suppose we have
>
> .text
> nop
> nop
> sym1:
> sym2 = sym1 + 2
>
> and the relax pass decides to delete one of the nops. Shouldn't sym2
> be decremented? Of course, you could argue that
>
> sym3 = .text + 4
>
> might be better left alone, but that just demonstrates the fragility
> of adjusting symbols based on their value.
>
I agree. I don't know the rationale for the toaddr test.
At least you give me one to remove it.
Stephane
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2003-04-09 22:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-04-04 21:19 [PATCH]: HC11 linker relax fixes (potential pb on other relax targets) Stephane Carrez
2003-04-08 12:41 ` Alan Modra
2003-04-09 22:10 ` Stephane Carrez
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).