This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] bfd/arc: Allow non-instruction relocations within .text sections
- From: Andrew Burgess <andrew dot burgess at embecosm dot com>
- To: Cupertino Miranda <Cupertino dot Miranda at synopsys dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>, "Claudiu dot Zissulescu at synopsys dot com" <Claudiu dot Zissulescu at synopsys dot com>
- Date: Thu, 7 Jan 2016 14:44:06 +0000
- Subject: Re: [PATCH] bfd/arc: Allow non-instruction relocations within .text sections
- Authentication-results: sourceware.org; auth=none
- References: <09dbe4a0879459cdc16f9f2aa68626563df5ee85 dot 1452087829 dot git dot andrew dot burgess at embecosm dot com> <EB86EB452ADE4B44B294F7149B8A27770218836E at DE02WEMBXB dot internal dot synopsys dot com>
* Cupertino Miranda <Cupertino.Miranda@synopsys.com> [2016-01-06 15:56:24 +0000]:
> Hi Andrew,
>
> We are currently finishing cleaning up a patch to integrate support for Linux (PIC and TLS).
> I am glad to say that we have realized this mistake and would intend to address it soon after the upcoming patch.
> Nevertheless, your implementation does it a little different from how we would like to address it.
>
> More precisely we do not define a new field in the RELOC_HOWTO but implement it through the FORMULA field using the ME preprocessor macro.
>
> ARC_RELOC_HOWTO(ARC_PC32, 50, \
> 2, \
> 32, \
> replace_word32, \
> signed, \
> ( ME ( ( S + A ) - P ) ))
>
> ME macro instead of deleted gets replaced by:
> #define ME(RELOC) (RELOC)
>
> And the remaining part does pretty much the same as your code, but resorting to the FORMULA to check if ME is present.
>
> static bfd_vma
> middle_endian_convert (bfd_vma insn, bfd_boolean do_it)
> {
> if (do_it)
> {
> insn =
> ((insn & 0xffff0000) >> 16) |
> ((insn & 0xffff) << 16);
> }
> return insn;
> }
>
> #define IS_ME(FORMULA) (strstr(#FORMULA, "ME") != NULL)
>
> #define ARC_RELOC_HOWTO(TYPE, VALUE, SIZE, BITSIZE, RELOC_FUNCTION, OVERFLOW, FORMULA) \
> case R_##TYPE: \
> { \
> bfd_vma bitsize ATTRIBUTE_UNUSED = BITSIZE; \
> insn = middle_endian_convert (insn, IS_ME(FORMULA)); \
> relocation = FORMULA ; \
> insn = middle_endian_convert (insn, IS_ME(FORMULA)); \
> } \
> break;
>
> Our view is to keep relocation definitions to be limited to this
> FORMULA concept, instead of adding new fields to the relocation
> table.
The patch below is a rewrite inline with the description above. The
only change is that I have extended the IS_ME macro to consider
whether the bfd is big or little endian, and I believe that this makes
a difference as to whether the byte shuffling is required.
Is this good with you?
Does my list of formula modifications match the list of changes you
have? I built my list based on how relocations were handled before
the big rewrite, but that doesn't mean it's correct, just that I've
not seen any test regressions either in binutils or GCC.
Thanks,
Andrew
---
On a little endian arc, a 4-byte instruction ABCD, where A is the most
significant byte, and D is the least significant byte would be stored in
memory (low to high) as BADC. That is, each 2-byte chunk is stored in
little endian order in memory.
Currently, when a relocation is applied to such a 4-byte instruction, we
make use of arc_bfd_get_32 and arc_bfd_put_32, which perform byte
manipulation to correct for this in memory byte ordering.
This byte ordering correction is applied to all relocations within
executable sections (for little endian arc) when really the correction
should only be applied to instruction relocations; it is the instruction
fetch mechanism that loads 4-byte instructions 2-bytes at a time.
It is possible to place data into an executable section, for example it
might be more efficient to place small jump tables inline within the
code rather than placing them into a data section.
The problem then, is that these two aspects, the byte order correction,
and inline data, conflict. Placing a 4-byte label relocation inline in
the code results in the byte-order correction being applied to it,
which, when the label is loaded using a standard arc load instruction,
returns a corrupted address.
Before the recent arc rewrite, placing data inline into the executable
sections did work. This was thanks to using a different code path to
patch instruction related relocations, to the code that patched data
related relocations. The instruction related relocations therefore
received the byte order correction, while the data related relocations
didn't.
After the recent rewrite this feature was lost, though I believe this
was by accident, rather than design. This commit brings this feature
back, though the implementation is different, in order to fit with the
new arc design.
The formula field, in those relocations that should have the byte
ordering fix applied, is extended to include a call to a new macro ME,
this macro is then used within the bfd library to trigger the
application of the byte ordering fix when appropriate. This design is
discussed here:
https://sourceware.org/ml/binutils/2016-01/msg00021.html
bfd/ChangeLog:
* elf32-arc.c (arc_bfd_get_32): Becomes an alias for bfd_get_32.
(arc_bfd_put_32): Becomes an alias for bfd_put_32.
(get_middle_endian_relocation): Delete.
(middle_endian_convert): New function.
(ME): Redefine, now does nothing.
(IS_ME): New define.
(arc_do_relocation): Extend the attached 'ARC_RELOC_HOWTO'
definition to call middle_endian_convert. Add a new local
variable and make use of this throughout.
include/ChangeLog:
* elf/arc-reloc.def: Add a call to ME within the formula for each
relocation that requires middle-endian correction.
gas/ChangeLog:
* testsuite/gas/arc/inline-data-1.d: New file.
* testsuite/gas/arc/inline-data-1.s: New file.
---
bfd/ChangeLog | 13 ++++++++
bfd/elf32-arc.c | 59 ++++++++++++++---------------------
gas/ChangeLog | 5 +++
gas/testsuite/gas/arc/inline-data-1.d | 7 +++++
gas/testsuite/gas/arc/inline-data-1.s | 4 +++
include/ChangeLog | 6 ++++
include/elf/arc-reloc.def | 54 ++++++++++++++++----------------
7 files changed, 85 insertions(+), 63 deletions(-)
create mode 100644 gas/testsuite/gas/arc/inline-data-1.d
create mode 100644 gas/testsuite/gas/arc/inline-data-1.s
diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 063cd14..1f92602 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,16 @@
+2016-01-05 Cupertino Miranda <Cupertino.Miranda@synopsys.com>
+ Andrew Burgess <andrew.burgess@embecosm.com>
+
+ * elf32-arc.c (arc_bfd_get_32): Becomes an alias for bfd_get_32.
+ (arc_bfd_put_32): Becomes an alias for bfd_put_32.
+ (get_middle_endian_relocation): Delete.
+ (middle_endian_convert): New function.
+ (ME): Redefine, now does nothing.
+ (IS_ME): New define.
+ (arc_do_relocation): Extend the attached 'ARC_RELOC_HOWTO'
+ definition to call middle_endian_convert. Add a new local
+ variable and make use of this throughout.
+
2016-01-01 Alan Modra <amodra@gmail.com>
Update year range in copyright notice of all files.
diff --git a/bfd/elf32-arc.c b/bfd/elf32-arc.c
index e9e299c..bf505fc 100644
--- a/bfd/elf32-arc.c
+++ b/bfd/elf32-arc.c
@@ -140,32 +140,10 @@ is_reloc_for_PLT (reloc_howto_type * howto)
#define arc_bfd_get_8(A,B,C) bfd_get_8(A,B)
#define arc_bfd_get_16(A,B,C) bfd_get_16(A,B)
+#define arc_bfd_get_32(A,B,C) bfd_get_32(A,B)
#define arc_bfd_put_8(A,B,C,D) bfd_put_8(A,B,C)
#define arc_bfd_put_16(A,B,C,D) bfd_put_16(A,B,C)
-
-static long
-arc_bfd_get_32 (bfd * abfd, void *loc, asection * input_section)
-{
- long insn = bfd_get_32 (abfd, loc);
-
- if (!bfd_big_endian (abfd)
- && input_section
- && (input_section->flags & SEC_CODE))
- insn = ((0x0000fffff & insn) << 16) | ((0xffff0000 & insn) >> 16);
-
- return insn;
-}
-
-static void
-arc_bfd_put_32 (bfd * abfd, long insn, void *loc, asection * input_section)
-{
- if (!bfd_big_endian (abfd)
- && input_section
- && (input_section->flags & SEC_CODE))
- insn = ((0x0000fffff & insn) << 16) | ((0xffff0000 & insn) >> 16);
-
- bfd_put_32 (abfd, insn, loc);
-}
+#define arc_bfd_put_32(A,B,C,D) bfd_put_32(A,B,C)
static bfd_reloc_status_type
arc_elf_reloc (bfd *abfd ATTRIBUTE_UNUSED,
@@ -473,16 +451,22 @@ debug_arc_reloc (struct arc_relocation_data reloc_data)
fprintf (stderr, " input section is NULL\n");
}
-static ATTRIBUTE_UNUSED bfd_vma
-get_middle_endian_relocation (bfd_vma reloc)
+static bfd_vma
+middle_endian_convert (bfd_vma insn, bfd_boolean do_it)
{
- bfd_vma ret =
- ((reloc & 0xffff0000) >> 16) |
- ((reloc & 0xffff) << 16);
- return ret;
+ if (do_it)
+ {
+ insn =
+ ((insn & 0xffff0000) >> 16) |
+ ((insn & 0xffff) << 16);
+ }
+ return insn;
}
-#define ME(RELOC) (get_middle_endian_reloction(RELOC))
+#define ME(reloc) (reloc)
+
+#define IS_ME(FORMULA,BFD) ((strstr(#FORMULA, "ME") != NULL) && \
+ !bfd_big_endian (BFD))
#define S (reloc_data.sym_value \
+ reloc_data.sym_section->output_offset \
@@ -518,7 +502,9 @@ get_middle_endian_relocation (bfd_vma reloc)
{ \
bfd_vma bitsize ATTRIBUTE_UNUSED = BITSIZE; \
relocation = FORMULA ; \
+ insn = middle_endian_convert (insn, IS_ME (FORMULA, abfd)); \
insn = RELOC_FUNCTION (insn, relocation); \
+ insn = middle_endian_convert (insn, IS_ME (FORMULA, abfd)); \
} \
break;
@@ -528,6 +514,7 @@ arc_do_relocation (bfd_byte * contents, struct arc_relocation_data reloc_data)
bfd_vma relocation = 0;
bfd_vma insn;
bfd_vma orig_insn ATTRIBUTE_UNUSED;
+ bfd * abfd = reloc_data.input_section->owner;
if (reloc_data.should_relocate == FALSE)
return bfd_reloc_notsupported;
@@ -535,13 +522,13 @@ arc_do_relocation (bfd_byte * contents, struct arc_relocation_data reloc_data)
switch (reloc_data.howto->size)
{
case 2:
- insn = arc_bfd_get_32 (reloc_data.input_section->owner,
+ insn = arc_bfd_get_32 (abfd,
contents + reloc_data.reloc_offset,
reloc_data.input_section);
break;
case 1:
case 0:
- insn = arc_bfd_get_16 (reloc_data.input_section->owner,
+ insn = arc_bfd_get_16 (abfd,
contents + reloc_data.reloc_offset,
reloc_data.input_section);
break;
@@ -569,7 +556,7 @@ arc_do_relocation (bfd_byte * contents, struct arc_relocation_data reloc_data)
flag = bfd_check_overflow (reloc_data.howto->complain_on_overflow,
reloc_data.howto->bitsize,
reloc_data.howto->rightshift,
- bfd_arch_bits_per_address (reloc_data.input_section->owner),
+ bfd_arch_bits_per_address (abfd),
relocation);
#undef DEBUG_ARC_RELOC
@@ -594,13 +581,13 @@ arc_do_relocation (bfd_byte * contents, struct arc_relocation_data reloc_data)
switch (reloc_data.howto->size)
{
case 2:
- arc_bfd_put_32 (reloc_data.input_section->owner, insn,
+ arc_bfd_put_32 (abfd, insn,
contents + reloc_data.reloc_offset,
reloc_data.input_section);
break;
case 1:
case 0:
- arc_bfd_put_16 (reloc_data.input_section->owner, insn,
+ arc_bfd_put_16 (abfd, insn,
contents + reloc_data.reloc_offset,
reloc_data.input_section);
break;
diff --git a/gas/ChangeLog b/gas/ChangeLog
index 64eeded..400f67a 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,8 @@
+2016-01-06 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ * testsuite/gas/arc/inline-data-1.d: New file.
+ * testsuite/gas/arc/inline-data-1.s: New file.
+
2016-01-01 Alan Modra <amodra@gmail.com>
Update year range in copyright notice of all files.
diff --git a/gas/testsuite/gas/arc/inline-data-1.d b/gas/testsuite/gas/arc/inline-data-1.d
new file mode 100644
index 0000000..ce5c272
--- /dev/null
+++ b/gas/testsuite/gas/arc/inline-data-1.d
@@ -0,0 +1,7 @@
+#as: -mcpu=arc700
+#objdump: -sj .text
+
+.*: +file format .*arc.*
+
+Contents of section .text:
+ [0-9a-f]+ ddccbbaa ffee .*
diff --git a/gas/testsuite/gas/arc/inline-data-1.s b/gas/testsuite/gas/arc/inline-data-1.s
new file mode 100644
index 0000000..e63bf06
--- /dev/null
+++ b/gas/testsuite/gas/arc/inline-data-1.s
@@ -0,0 +1,4 @@
+ .text
+
+ .word 0xaabbccdd
+ .short 0xeeff
diff --git a/include/ChangeLog b/include/ChangeLog
index 70e19b7..9f2a5d6 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,3 +1,9 @@
+2016-01-06 Andrew Burgess <andrew.burgess@embecosm.com>
+ Cupertino Miranda <Cupertino.Miranda@synopsys.com>
+
+ * elf/arc-reloc.def: Add a call to ME within the formula for each
+ relocation that requires middle-endian correction.
+
2016-01-01 Alan Modra <amodra@gmail.com>
Update year range in copyright notice of all files.
diff --git a/include/elf/arc-reloc.def b/include/elf/arc-reloc.def
index 893291f..cb43a21 100644
--- a/include/elf/arc-reloc.def
+++ b/include/elf/arc-reloc.def
@@ -80,56 +80,56 @@ ARC_RELOC_HOWTO(ARC_S21H_PCREL, 14, \
20, \
replace_disp21h, \
signed, \
- ( ( ( S + A ) - P ) >> 1 ))
+ ( ME ( ( ( S + A ) - P ) >> 1 )))
ARC_RELOC_HOWTO(ARC_S21W_PCREL, 15, \
2, \
19, \
replace_disp21w, \
signed, \
- ( ( ( S + A ) - P ) >> 2 ))
+ ( ME ( ( ( S + A ) - P ) >> 2 )))
ARC_RELOC_HOWTO(ARC_S25H_PCREL, 16, \
2, \
24, \
replace_disp25h, \
signed, \
- ( ( ( S + A ) - P ) >> 1 ))
+ ( ME ( ( ( S + A ) - P ) >> 1 )))
ARC_RELOC_HOWTO(ARC_S25W_PCREL, 17, \
2, \
23, \
replace_disp25w, \
signed, \
- ( ( ( S + A ) - P ) >> 2 ))
+ ( ME ( ( ( S + A ) - P ) >> 2 )))
ARC_RELOC_HOWTO(ARC_SDA32, 18, \
2, \
32, \
replace_word32, \
signed, \
- ( ( S + A ) - _SDA_BASE_ ))
+ ( ME ( ( S + A ) - _SDA_BASE_ )))
ARC_RELOC_HOWTO(ARC_SDA_LDST, 19, \
2, \
9, \
replace_disp9ls, \
signed, \
- ( ( S + A ) - _SDA_BASE_ ))
+ ( ME ( ( S + A ) - _SDA_BASE_ )))
ARC_RELOC_HOWTO(ARC_SDA_LDST1, 20, \
2, \
9, \
replace_disp9ls, \
signed, \
- ( ( ( S + A ) - _SDA_BASE_ ) >> 1 ))
+ ( ME ( ( ( S + A ) - _SDA_BASE_ ) >> 1 )))
ARC_RELOC_HOWTO(ARC_SDA_LDST2, 21, \
2, \
9, \
replace_disp9ls, \
signed, \
- ( ( ( S + A ) - _SDA_BASE_ ) >> 2 ))
+ ( ME ( ( ( S + A ) - _SDA_BASE_ ) >> 2 )))
ARC_RELOC_HOWTO(ARC_SDA16_LD, 22, \
1, \
@@ -171,42 +171,42 @@ ARC_RELOC_HOWTO(ARC_32_ME, 27, \
32, \
replace_limm, \
signed, \
- ( S + A ))
+ ( ME ( S + A )))
ARC_RELOC_HOWTO(ARC_32_ME_S, 105, \
2, \
32, \
replace_limms, \
signed, \
- ( S + A ))
+ ( ME ( S + A )))
ARC_RELOC_HOWTO(ARC_N32_ME, 28, \
2, \
32, \
replace_word32, \
bitfield, \
- ( S - A ))
+ ( ME ( S - A )))
ARC_RELOC_HOWTO(ARC_SECTOFF_ME, 29, \
2, \
32, \
replace_word32, \
bitfield, \
- ( ( S - SECTSTART ) + A ))
+ ( ME ( ( S - SECTSTART ) + A )))
ARC_RELOC_HOWTO(ARC_SDA32_ME, 30, \
2, \
32, \
replace_limm, \
signed, \
- ( ( S + A ) - _SDA_BASE_ ))
+ ( ME ( ( S + A ) - _SDA_BASE_ )))
ARC_RELOC_HOWTO(ARC_W_ME, 31, \
2, \
32, \
replace_word32, \
bitfield, \
- ( S + A ))
+ ( ME ( S + A )))
ARC_RELOC_HOWTO(AC_SECTOFF_U8, 35, \
2, \
@@ -255,14 +255,14 @@ ARC_RELOC_HOWTO(ARC_SECTOFF_ME_1, 41, \
32, \
replace_word32, \
bitfield, \
- ( ( ( S - SECTSTART ) + A ) >> 1 ))
+ ( ME ( ( ( S - SECTSTART ) + A ) >> 1 )))
ARC_RELOC_HOWTO(ARC_SECTOFF_ME_2, 42, \
2, \
32, \
replace_word32, \
bitfield, \
- ( ( ( S - SECTSTART ) + A ) >> 2 ))
+ ( ME ( ( ( S - SECTSTART ) + A ) >> 2 )))
ARC_RELOC_HOWTO(ARC_SECTOFF_1, 43, \
2, \
@@ -297,7 +297,7 @@ ARC_RELOC_HOWTO(ARC_PC32, 50, \
32, \
replace_word32, \
signed, \
- ( ( S + A ) - P ))
+ ( ME ( ( S + A ) - P )))
ARC_RELOC_HOWTO(ARC_GOT32, 59, \
2, \
@@ -311,14 +311,14 @@ ARC_RELOC_HOWTO(ARC_GOTPC32, 51, \
32, \
replace_word32, \
signed, \
- ( ( ( GOT + G ) + A ) - P ))
+ ( ME ( ( ( GOT + G ) + A ) - P )))
ARC_RELOC_HOWTO(ARC_PLT32, 52, \
2, \
32, \
replace_word32, \
signed, \
- ( ( L + A ) - P ))
+ ( ME ( ( L + A ) - P )))
ARC_RELOC_HOWTO(ARC_COPY, 53, \
2, \
@@ -339,42 +339,42 @@ ARC_RELOC_HOWTO(ARC_JMP_SLOT, 55, \
32, \
replace_word32, \
signed, \
- S)
+ ( ME ( S )))
ARC_RELOC_HOWTO(ARC_RELATIVE, 56, \
2, \
32, \
replace_word32, \
signed, \
- ( B + A ))
+ ( ME ( B + A )))
ARC_RELOC_HOWTO(ARC_GOTOFF, 57, \
2, \
32, \
replace_word32, \
signed, \
- ( ( S + A ) - GOT ))
+ ( ME ( ( S + A ) - GOT )))
ARC_RELOC_HOWTO(ARC_GOTPC, 58, \
2, \
32, \
replace_word32, \
signed, \
- ( ( GOT + A ) - P ))
+ ( ME ( ( GOT + A ) - P )))
ARC_RELOC_HOWTO(ARC_S21W_PCREL_PLT, 60, \
2, \
19, \
replace_disp21w, \
signed, \
- ( ( ( L + A ) - P ) >> 2 ))
+ ( ME ( ( ( L + A ) - P ) >> 2 )))
ARC_RELOC_HOWTO(ARC_S25H_PCREL_PLT, 61, \
2, \
24, \
replace_disp25h, \
signed, \
- ( ( ( L + A ) - P ) >> 1 ))
+ ( ME ( ( ( L + A ) - P ) >> 1 )))
ARC_RELOC_HOWTO(ARC_TLS_DTPMOD, 66, \
2, \
@@ -451,12 +451,12 @@ ARC_RELOC_HOWTO(ARC_S25W_PCREL_PLT, 76, \
23, \
replace_disp25w, \
signed, \
- ( ( ( L + A ) - P ) >> 2 ))
+ ( ME ( ( ( L + A ) - P ) >> 2 )))
ARC_RELOC_HOWTO(ARC_S21H_PCREL_PLT, 77, \
2, \
20, \
replace_disp21h, \
signed, \
- ( ( ( L + A ) - P ) >> 1 ))
+ ( ME ( ( ( L + A ) - P ) >> 1 )))
--
2.6.4