[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Tinycc-devel] [PATCH] use RELA relocations properly for R_DATA_PTR

From: Michael Matz
Subject: Re: [Tinycc-devel] [PATCH] use RELA relocations properly for R_DATA_PTR
Date: Sat, 21 Feb 2015 19:18:21 +0100 (CET)
User-agent: Alpine 2.00 (LNX 1167 2008-08-23)

Hello Edmund,

On Tue, 17 Feb 2015, Edmund Grimley Evans wrote:

With TCC the addend is zero! Apparently TCC is putting the offset is
in the data section:

readelf -x .data t1.gcc.o
 0x00000000 00000000 00000000                   ........

readelf -x .data t1.tcc.o
 0x00000000 10000000 00000000                   ........

Now this isn't how RELA relocations are normally described as working,
though I'm not sure that it's wrong.

It's strictly wrong, but harmless, because, as you noticed, all linkers OR in the value at the reloc place. It's one of those things that always wants fixing but never really reach the top of TODO lists :) As it reached yours, all the better.

I haven't found a document that says you have to ignore what value is "in the place" with a RELA relocation,

Actually, the x86-64 psABI is explicit. The formulas for calculating reloc values only contain one 'A' (addend) and specify that the addend is in r_addend of the reloc, so the "addend" at the place must be ignored. But alas, real world is more forgiving :)

Proposed commit message:

Use RELA relocations properly for R_DATA_PTR on x86_64.

libtcc.c: Add greloca, a generalisation of greloc that takes an addend.
tcc.h: Add greloca and put_elf_reloca.
tccelf.c: Add put_elf_reloca, a generalisation of put_elf_reloc.
tccgen.c: On x86_64, use greloca instead of greloc in init_putv.

The patch is fine, please commit.  I have one remark:

-            if (vtop->r & VT_SYM) {
+#ifdef TCC_TARGET_X86_64
+            if (vtop->r & VT_SYM)
+                greloca(sec, vtop->sym, c, R_DATA_PTR, vtop->c.ptr_offset);
+            else
+                *(addr_t *)ptr |= (vtop->c.ptr_offset & bit_mask) << bit_pos;
+            if (vtop->r & VT_SYM)
                 greloc(sec, vtop->sym, c, R_DATA_PTR);
-            }
             *(addr_t *)ptr |= (vtop->c.ptr_offset & bit_mask) << bit_pos;

Strictly speaking the addend should be
  (vtop->c.ptr_offset & bit_mask) << bit_pos;
This should make no difference for VT_SYM vtops, but perhaps an assert would be better (bit_pos == 0). OTOH this routine has other checking problems (e.g. it doesn't give an error if the value doesn't fit e.g. a VT_BYTE place), so adding just one assert for this case seems inconsistent.

Oh, and one other thing: if you could include (smallish) patches inline at the end of mail, it makes quoting parts of them for review easier; of course only if your mailer doesn't mangle patches added inline.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]