RFA: pseudo-relocations for pe targets

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

RFA: pseudo-relocations for pe targets

Kai Tietz
Hello all,

I did some research about the pseudo-relocation implementation in ld (used
by cygwin and mingw AFAICS). For w64 there must be a different structure
to be used, because there are different wide relocation types used (in
common ip-relative 32-bit and 64-bit, and 64-bit absolute relocations). By
the current implementation there are several problems I see.
a) Auto-imported variables do not work in all cases (e.g. structure field
addresses, etc).
b) Each relocation leads to additional code needed.
c) For 64-bit different relocation sizes are possible.
d) If the relocations are placed within custom read-only sections the
pseudo-relocator produces an assertation by attempting to write to
read-only addresses.

So I would suggest to extend the structure used for pseudo-relocations by
two new members that it looks like that
  .long addend
  .long target
  .long fixup_sym
  .long flags

The new fixup_symbol points to the iat entry of the referenced import
element, or to the stub function.
The flags value has at the moment just the following meaning: Bit 0-7:
Relocation size in bytes, the bit 8 indicates that an fixup_sym
dereferencing is necessary (for data elements).

The code in runtimes of the pseudo-relocator would do the following steps:
1) If "addend" is zero and flag:8 is zero continue to the next element
(nothing to be done).
2) Read the existing relocation as value at "target" as ("rel").
3) Substract from "rel" the "addend" and the "fixup_sym".
4) Read the final destination address from "fixup_sym" as "dst".
5) Add to "dst" the "addend" value.
6) Add to "rel" the "dst"
7) Check for overflow
8) Write "rel" back to "target" by using protect() to make sure that no
memory exception appears.

This should enable even pe targets to auto-import even variables in all
cases and speed-up execution, too.

I am interested in your opinion about this and also would like to know, if
there is for the existing 32-bit targets also an interest to have this
change, otherwise I will implement it just for the 64-bit case.

Regards,
 i.A. Kai Tietz

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Christopher Faylor-9
On Mon, Nov 03, 2008 at 03:21:43PM +0100, Kai Tietz wrote:

>The code in runtimes of the pseudo-relocator would do the following steps:
>1) If "addend" is zero and flag:8 is zero continue to the next element
>(nothing to be done).
>2) Read the existing relocation as value at "target" as ("rel").
>3) Substract from "rel" the "addend" and the "fixup_sym".
>4) Read the final destination address from "fixup_sym" as "dst".
>5) Add to "dst" the "addend" value.
>6) Add to "rel" the "dst"
>7) Check for overflow
>8) Write "rel" back to "target" by using protect() to make sure that no
>memory exception appears.
>
>This should enable even pe targets to auto-import even variables in all
>cases and speed-up execution, too.
>
>I am interested in your opinion about this and also would like to know, if
>there is for the existing 32-bit targets also an interest to have this
>change, otherwise I will implement it just for the 64-bit case.

I would be very interested in seeing this added for all PE targets.

cgf
Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Kai Tietz
Christopher Faylor <[hidden email]> wrote
on 03.11.2008 16:23:04:

> On Mon, Nov 03, 2008 at 03:21:43PM +0100, Kai Tietz wrote:
> >The code in runtimes of the pseudo-relocator would do the following
steps:
> >1) If "addend" is zero and flag:8 is zero continue to the next element
> >(nothing to be done).
> >2) Read the existing relocation as value at "target" as ("rel").
> >3) Substract from "rel" the "addend" and the "fixup_sym".
> >4) Read the final destination address from "fixup_sym" as "dst".
> >5) Add to "dst" the "addend" value.
> >6) Add to "rel" the "dst"
> >7) Check for overflow
> >8) Write "rel" back to "target" by using protect() to make sure that no

> >memory exception appears.
> >
> >This should enable even pe targets to auto-import even variables in all

> >cases and speed-up execution, too.
> >
> >I am interested in your opinion about this and also would like to know,
if
> >there is for the existing 32-bit targets also an interest to have this
> >change, otherwise I will implement it just for the 64-bit case.
>
> I would be very interested in seeing this added for all PE targets.
>
> cgf
>

Ok, here is my patch for it. I verified it on x86_64-pc-mingw32, so maybe
somebody could verify it for 32-bit cygwin/mingw, too.
I introduced two new linker parameters named --enable-pseudo-runtime-v1
(for the old implementation) and --enable-pseudo-runtime-v2 for the new
one. The defaults for 32-bit is the old variant (as broken it is, but to
prevent startup failures), and for 64-bit the new version.

ChangeLog

2008-11-02  Kai Tietz  <[hidden email]>

        * emultempl/pep.em (..._before_parse): initialize
pei386_runtime_pseudo_reloc by version 2.
        (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1): New option.
        (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V2): New option.
        (make_import_fixup): Use relocation size to read addend.
        * emultempl/pe.em (..._before_parse): initialize
pei386_runtime_pseudo_reloc by version 1.
        (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1): New option.
        (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V2): New option.
        * pe-dll.c (pe-dll.h): Remove useless include.
        (make_runtime_pseudo_reloc): Change addend to use bfd_vma.
        Handle the two variants of pseudo-relocation.
        (pe_create_import_fixup): Change addend to type bfd_vma.
        Modify for the two pseudo_relocation variants.
        * pe-dll.h (pe_create_import_fixup): Change addend argument type
to bfd_vma.
        * pep-dll.h (pep_create_import_fixup): Likewise.

Is this patch ok for trunk?

Cheers,
Kai



|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

pseudo_reloc_w64.txt (13K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Nick Clifton
Hi Kai,

> Ok, here is my patch for it. I verified it on x86_64-pc-mingw32, so maybe
> somebody could verify it for 32-bit cygwin/mingw, too.

Assuming that it can be verified for 32-bit cygwin/mingw then this patch
is approved.

> 2008-11-02  Kai Tietz  <[hidden email]>
>
>         * emultempl/pep.em (..._before_parse): initialize
> pei386_runtime_pseudo_reloc by version 2.
>         (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1): New option.
>         (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V2): New option.
>         (make_import_fixup): Use relocation size to read addend.
>         * emultempl/pe.em (..._before_parse): initialize
> pei386_runtime_pseudo_reloc by version 1.
>         (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1): New option.
>         (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V2): New option.
>         * pe-dll.c (pe-dll.h): Remove useless include.
>         (make_runtime_pseudo_reloc): Change addend to use bfd_vma.
>         Handle the two variants of pseudo-relocation.
>         (pe_create_import_fixup): Change addend to type bfd_vma.
>         Modify for the two pseudo_relocation variants.
>         * pe-dll.h (pe_create_import_fixup): Change addend argument type
> to bfd_vma.
>         * pep-dll.h (pep_create_import_fixup): Likewise.

One thing - please could you add a line to ld/NEWS mentioning the new
feature ?

Cheers
   Nick


Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Kai Tietz
Hi Nick,

Nick Clifton <[hidden email]> wrote on 06.11.2008 12:55:31:

> Hi Kai,
>
> > Ok, here is my patch for it. I verified it on x86_64-pc-mingw32, so
maybe
> > somebody could verify it for 32-bit cygwin/mingw, too.
>
> Assuming that it can be verified for 32-bit cygwin/mingw then this patch

> is approved.
>
> > 2008-11-02  Kai Tietz  <[hidden email]>
> >
> >         * emultempl/pep.em (..._before_parse): initialize
> > pei386_runtime_pseudo_reloc by version 2.
> >         (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1): New option.
> >         (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V2): New option.
> >         (make_import_fixup): Use relocation size to read addend.
> >         * emultempl/pe.em (..._before_parse): initialize
> > pei386_runtime_pseudo_reloc by version 1.
> >         (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1): New option.
> >         (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V2): New option.
> >         * pe-dll.c (pe-dll.h): Remove useless include.
> >         (make_runtime_pseudo_reloc): Change addend to use bfd_vma.
> >         Handle the two variants of pseudo-relocation.
> >         (pe_create_import_fixup): Change addend to type bfd_vma.
> >         Modify for the two pseudo_relocation variants.
> >         * pe-dll.h (pe_create_import_fixup): Change addend argument
type
> > to bfd_vma.
> >         * pep-dll.h (pep_create_import_fixup): Likewise.
>
> One thing - please could you add a line to ld/NEWS mentioning the new
> feature ?
>
> Cheers
>    Nick

Ok, I'll add a line to ld/NEWS. For version 2.19?

Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Nick Clifton
Hi Kai,

> Ok, I'll add a line to ld/NEWS.

Thanks,

> For version 2.19?

No, let's keep the branch clean.

Cheers
   Nick


Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Kai Tietz-2
In reply to this post by Kai Tietz
Hi,

I missed add for tests the current pseudo-reloc.c implementation of
the version 2.

2008/11/4 Kai Tietz <[hidden email]>:

> Christopher Faylor <[hidden email]> wrote
> on 03.11.2008 16:23:04:
>
>> On Mon, Nov 03, 2008 at 03:21:43PM +0100, Kai Tietz wrote:
>> >The code in runtimes of the pseudo-relocator would do the following
> steps:
>> >1) If "addend" is zero and flag:8 is zero continue to the next element
>> >(nothing to be done).
>> >2) Read the existing relocation as value at "target" as ("rel").
>> >3) Substract from "rel" the "addend" and the "fixup_sym".
>> >4) Read the final destination address from "fixup_sym" as "dst".
>> >5) Add to "dst" the "addend" value.
>> >6) Add to "rel" the "dst"
>> >7) Check for overflow
>> >8) Write "rel" back to "target" by using protect() to make sure that no
>
>> >memory exception appears.
>> >
>> >This should enable even pe targets to auto-import even variables in all
>
>> >cases and speed-up execution, too.
>> >
>> >I am interested in your opinion about this and also would like to know,
> if
>> >there is for the existing 32-bit targets also an interest to have this
>> >change, otherwise I will implement it just for the 64-bit case.
>>
>> I would be very interested in seeing this added for all PE targets.
>>
>> cgf
>>
>
> Ok, here is my patch for it. I verified it on x86_64-pc-mingw32, so maybe
> somebody could verify it for 32-bit cygwin/mingw, too.
> I introduced two new linker parameters named --enable-pseudo-runtime-v1
> (for the old implementation) and --enable-pseudo-runtime-v2 for the new
> one. The defaults for 32-bit is the old variant (as broken it is, but to
> prevent startup failures), and for 64-bit the new version.
>
> ChangeLog
>
> 2008-11-02  Kai Tietz  <[hidden email]>
>
>        * emultempl/pep.em (..._before_parse): initialize
> pei386_runtime_pseudo_reloc by version 2.
>        (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1): New option.
>        (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V2): New option.
>        (make_import_fixup): Use relocation size to read addend.
>        * emultempl/pe.em (..._before_parse): initialize
> pei386_runtime_pseudo_reloc by version 1.
>        (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1): New option.
>        (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V2): New option.
>        * pe-dll.c (pe-dll.h): Remove useless include.
>        (make_runtime_pseudo_reloc): Change addend to use bfd_vma.
>        Handle the two variants of pseudo-relocation.
>        (pe_create_import_fixup): Change addend to type bfd_vma.
>        Modify for the two pseudo_relocation variants.
>        * pe-dll.h (pe_create_import_fixup): Change addend argument type
> to bfd_vma.
>        * pep-dll.h (pep_create_import_fixup): Likewise.
>
> Is this patch ok for trunk?
>

Cheers,
Kai

|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

pseudo-reloc.c (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Kai Tietz
In reply to this post by Nick Clifton
Hi Nick,

Nick Clifton <[hidden email]> wrote on 06.11.2008 14:21:27:

> Hi Kai,
>
> > Ok, I'll add a line to ld/NEWS.
>
> Thanks,
>
> > For version 2.19?
>
> No, let's keep the branch clean.
>
> Cheers
>    Nick
>
>

I add some lines to ld/NEWS and corrected a typo of mine in pe-dll.c.

2008-11-11  Kai Tietz  <[hidden email]>

        * emultempl/pep.em (..._before_parse): initialize
pei386_runtime_pseudo_reloc by version 2.
        (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1): New option.
        (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V2): New option.
        (make_import_fixup): Use relocation size to read addend.
        * emultempl/pe.em (..._before_parse): initialize
pei386_runtime_pseudo_reloc by version 1.
        (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1): New option.
        (OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V2): New option.
        * pe-dll.c (pe-dll.h): Remove useless include.
        (make_runtime_pseudo_reloc): Change addend to use bfd_vma.
        Handle the two variants of pseudo-relocation.
        (pe_create_import_fixup): Change addend to type bfd_vma.
        Modify for the two pseudo_relocation variants.
        * pe-dll.h (pe_create_import_fixup): Change addend argument type
to bfd_vma.
        * pep-dll.h (pep_create_import_fixup): Likewise.
        * NEWS: Add comment.



It would be very kind, if somebody could verify this patch on cygwin for
no side-effects.

Cheers,
Kai



|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

pseudo_reloc_w64.txt (13K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Brian Dessent
Kai Tietz wrote:

> It would be very kind, if somebody could verify this patch on cygwin for
> no side-effects.

I don't see how you can propose changing the size of the data structure
that holds the list of pseudo-relocs without touching the runtime part
of the code in Cygwin that actually does the relocating
(winsup/cygwin/lib/pseudo-reloc.c).  And similarly with the code in the
32 bit MinGW runtime.  This adds a versioning problem in that now the
version of the linker and the version of libcygwin.a have to both agree
otherwise you link an executable that contains a v2 list but whose
runtime support expects to read a v1 list, or vice versa.  There
currently exists no such close coupling, and creating one is going to
break all kinds of things and surprise many people.

It seems to me that the only possible way that you could pull off such a
change in any sane way would be to have the runtime relocator be able to
detect dynamically at runtime whether the linker that was used created a
v1 or a v2 list, and act appropriately.  Then you leave v1 the default
until that version of libcygwin.a had been reasonably widely distributed
(several releases), and then you can make v2 the default.  But I don't
see how you can just switch to v2 like this.

Brian
Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Kai Tietz
Hi Brian,

Brian Dessent <[hidden email]> wrote on 11.11.2008 14:59:37:

> Kai Tietz wrote:
>
> > It would be very kind, if somebody could verify this patch on cygwin
for

> > no side-effects.
>
> I don't see how you can propose changing the size of the data structure
> that holds the list of pseudo-relocs without touching the runtime part
> of the code in Cygwin that actually does the relocating
> (winsup/cygwin/lib/pseudo-reloc.c).  And similarly with the code in the
> 32 bit MinGW runtime.  This adds a versioning problem in that now the
> version of the linker and the version of libcygwin.a have to both agree
> otherwise you link an executable that contains a v2 list but whose
> runtime support expects to read a v1 list, or vice versa.  There
> currently exists no such close coupling, and creating one is going to
> break all kinds of things and surprise many people.
>
> It seems to me that the only possible way that you could pull off such a
> change in any sane way would be to have the runtime relocator be able to
> detect dynamically at runtime whether the linker that was used created a
> v1 or a v2 list, and act appropriately.  Then you leave v1 the default
> until that version of libcygwin.a had been reasonably widely distributed
> (several releases), and then you can make v2 the default.  But I don't
> see how you can just switch to v2 like this.
>
> Brian
>

as you may saw in prior comments (or by source code), this is exactly the
reason why I introduced version 1 and version 2. And as I told before for
32-bit targets version 1 (the old pseudo-relocation fixup structure) is
written by default. So there should be no change in runtime for now (and
the new pseudo-reloc.c file for version 2 is for sure not needed. I
prepare at the moment additionally a version of the pseudo-reloc.c file,
which is able to choose the proper relocation type by itself, but there
should be at the moment no need to change this for 32-bit!

Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Brian Dessent
Kai Tietz wrote:

> as you may saw in prior comments (or by source code), this is exactly the
> reason why I introduced version 1 and version 2. And as I told before for
> 32-bit targets version 1 (the old pseudo-relocation fixup structure) is

Oh right.  I saw the first hunk which sets

-  link_info.pei386_runtime_pseudo_reloc = -1;
+  link_info.pei386_runtime_pseudo_reloc = 2; /* Use by default version
2.  */

...and neglected that this was in pep.em not pe.em.

> prepare at the moment additionally a version of the pseudo-reloc.c file,
> which is able to choose the proper relocation type by itself, but there
> should be at the moment no need to change this for 32-bit!

I still am not clear how you would write a runtime relocator that can
detect whether it's been passed a v1 or a v2 list.  Or is that what you
were implying with the part of the v2 spec about addend == flags == 0
being interpreted as a no-op?  In other words, you're saying that the v2
list would distinguish itself by having as the first entry an element
with all zeros (as that would never naturally occur with a v1 list since
a pseudo-reloc is only required when addend != 0)?

Brian
Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Kai Tietz
Brian Dessent <[hidden email]> wrote on 11.11.2008 15:56:09:

> Kai Tietz wrote:
>
> > as you may saw in prior comments (or by source code), this is exactly
the
> > reason why I introduced version 1 and version 2. And as I told before
for
> > 32-bit targets version 1 (the old pseudo-relocation fixup structure)
is

>
> Oh right.  I saw the first hunk which sets
>
> -  link_info.pei386_runtime_pseudo_reloc = -1;
> +  link_info.pei386_runtime_pseudo_reloc = 2; /* Use by default version
> 2.  */
>
> ...and neglected that this was in pep.em not pe.em.
>
> > prepare at the moment additionally a version of the pseudo-reloc.c
file,
> > which is able to choose the proper relocation type by itself, but
there

> > should be at the moment no need to change this for 32-bit!
>
> I still am not clear how you would write a runtime relocator that can
> detect whether it's been passed a v1 or a v2 list.  Or is that what you
> were implying with the part of the v2 spec about addend == flags == 0
> being interpreted as a no-op?  In other words, you're saying that the v2
> list would distinguish itself by having as the first entry an element
> with all zeros (as that would never naturally occur with a v1 list since
> a pseudo-reloc is only required when addend != 0)?
>
> Brian
>

Well, I thought about the noop version, but deceided not todo so. The
problem is that __image_base__+0 is not necessarily writeable, but old
relocator tries to add then zero there and may raise a PF. So I want to
implement it by checking that the size for the entries modulo
sizeof(pseudo_reloc_v1) is zero for v2 and check that the flags (the
reloc-size) field has just values of 8,16,32,64. Otherwise version one is
assumed.

If you have an better idea, please let me know. I think this check is a
hack, but should work.

Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Pedro Alves-10
On Tuesday 11 November 2008 15:12:29, Kai Tietz wrote:

> Well, I thought about the noop version, but deceided not todo so. The
> problem is that __image_base__+0 is not necessarily writeable, but old
> relocator tries to add then zero there and may raise a PF. So I want to
> implement it by checking that the size for the entries modulo
> sizeof(pseudo_reloc_v1) is zero for v2 and check that the flags (the
> reloc-size) field has just values of 8,16,32,64. Otherwise version one is
> assumed.
>
> If you have an better idea, please let me know. I think this check is a
> hack, but should work.

Why not just have two lists?

 extern char __RUNTIME_PSEUDO_RELOC_LIST___;
 extern char __RUNTIME_PSEUDO_RELOC_LIST__END__;

 extern char __RUNTIME_PSEUDO_RELOC_LIST_V2__;
 extern char __RUNTIME_PSEUDO_RELOC_LIST_V2_END__;

?

This way, it could also be possible to mix v2 relocs with v1 relocs.

In the new v2 case, you could add some reloc-type marker, for
future extension, so that when a v3 (or vNNN) is added, an old v2
relocator can detect it, and bail out.

--
Pedro Alves
Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Brian Dessent
In reply to this post by Kai Tietz
Kai Tietz wrote:

> Well, I thought about the noop version, but deceided not todo so. The
> problem is that __image_base__+0 is not necessarily writeable, but old
> relocator tries to add then zero there and may raise a PF. So I want to

The case of the v1 runtime with a v2 list of relocs is just never going
to work anyway, so it's not worth trying to make it work.  Even if you
avoided the fault with it trying to fixup target=0, it's just going to
fault at the next step when it treats the 'fixup_sym/flags' fields as
what it thinks are the 'addend/target' fields of the next entry.

> implement it by checking that the size for the entries modulo
> sizeof(pseudo_reloc_v1) is zero for v2 and check that the flags (the
> reloc-size) field has just values of 8,16,32,64. Otherwise version one is
> assumed.

Checking the size doesn't really seem like all that great of an idea
given that sizeof(v2) is four words and sizeof(v1) is two words, so all
you can really tell here is that it's a v1 list if there happens to be
an odd number of entries.  In all other cases you can't tell anything
and so you're down to this flags heuristic.  Just assuming that you'll
never have a target that's a power of 2 seems really weak, especially if
in the future you need to extend the list of valid flags.

I think the "first entry of v2 is a zeroed no-op" is a much better way
to go.

Or, use different symbols for the labels of the v2 list, e.g.
__RUNTIME_PSEUDO_RELOC_LIST_V2__ and
__RUNTIME_PSEUDO_RELOC_LIST_V2_END__.  Then you can put a copy of both
the v1 and the v2 runtime routines in libcygwin.a/libmingw.a, and the
linker will pick the correct one to include in the link based on the
presence of either __RUNTIME_PSEUDO_RELOC_LIST__ or
__RUNTIME_PSEUDO_RELOC_LIST_V2__.

Brian
Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Kai Tietz
Brian Dessent <[hidden email]> wrote on 11.11.2008 16:47:29:

> Kai Tietz wrote:
>
> > Well, I thought about the noop version, but deceided not todo so. The
> > problem is that __image_base__+0 is not necessarily writeable, but old
> > relocator tries to add then zero there and may raise a PF. So I want
to
>
> The case of the v1 runtime with a v2 list of relocs is just never going
> to work anyway, so it's not worth trying to make it work.  Even if you
> avoided the fault with it trying to fixup target=0, it's just going to
> fault at the next step when it treats the 'fixup_sym/flags' fields as
> what it thinks are the 'addend/target' fields of the next entry.

That's right.

> > implement it by checking that the size for the entries modulo
> > sizeof(pseudo_reloc_v1) is zero for v2 and check that the flags (the
> > reloc-size) field has just values of 8,16,32,64. Otherwise version one
is
> > assumed.

Sorry here I had a type, of course I meant sizeof(v2).

> Checking the size doesn't really seem like all that great of an idea
> given that sizeof(v2) is four words and sizeof(v1) is two words, so all
sizeof(v2) has three words, but well it is weak.

> you can really tell here is that it's a v1 list if there happens to be
> an odd number of entries.  In all other cases you can't tell anything
> and so you're down to this flags heuristic.  Just assuming that you'll
> never have a target that's a power of 2 seems really weak, especially if
> in the future you need to extend the list of valid flags.
>
> I think the "first entry of v2 is a zeroed no-op" is a much better way
> to go.
>
> Or, use different symbols for the labels of the v2 list, e.g.
> __RUNTIME_PSEUDO_RELOC_LIST_V2__ and
> __RUNTIME_PSEUDO_RELOC_LIST_V2_END__.  Then you can put a copy of both
> the v1 and the v2 runtime routines in libcygwin.a/libmingw.a, and the
> linker will pick the correct one to include in the link based on the
> presence of either __RUNTIME_PSEUDO_RELOC_LIST__ or
> __RUNTIME_PSEUDO_RELOC_LIST_V2__.
>
> Brian
>

Hmm, well. But this means that someone using v2 ld and the startup code of
old version, silently gets started (if we reuse this list, it will at
least crash), and afterwards he gets strange errors (or even worse, just
in special cases).

Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Christopher Faylor-9
In reply to this post by Brian Dessent
On Tue, Nov 11, 2008 at 07:47:29AM -0800, Brian Dessent wrote:

>Kai Tietz wrote:
>
>> Well, I thought about the noop version, but deceided not todo so. The
>> problem is that __image_base__+0 is not necessarily writeable, but old
>> relocator tries to add then zero there and may raise a PF. So I want to
>
>The case of the v1 runtime with a v2 list of relocs is just never going
>to work anyway, so it's not worth trying to make it work.  Even if you
>avoided the fault with it trying to fixup target=0, it's just going to
>fault at the next step when it treats the 'fixup_sym/flags' fields as
>what it thinks are the 'addend/target' fields of the next entry.
>
>> implement it by checking that the size for the entries modulo
>> sizeof(pseudo_reloc_v1) is zero for v2 and check that the flags (the
>> reloc-size) field has just values of 8,16,32,64. Otherwise version one is
>> assumed.
>
>Checking the size doesn't really seem like all that great of an idea
>given that sizeof(v2) is four words and sizeof(v1) is two words, so all
>you can really tell here is that it's a v1 list if there happens to be
>an odd number of entries.  In all other cases you can't tell anything
>and so you're down to this flags heuristic.  Just assuming that you'll
>never have a target that's a power of 2 seems really weak, especially if
>in the future you need to extend the list of valid flags.
>
>I think the "first entry of v2 is a zeroed no-op" is a much better way
>to go.

Or first entry zeroed, second entry version number.

cgf
Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Kai Tietz
> On Tue, Nov 11, 2008 at 07:47:29AM -0800, Brian Dessent wrote:
> >Kai Tietz wrote:
> >
> >> Well, I thought about the noop version, but deceided not todo so. The
> >> problem is that __image_base__+0 is not necessarily writeable, but
old
> >> relocator tries to add then zero there and may raise a PF. So I want
to

> >
> >The case of the v1 runtime with a v2 list of relocs is just never going
> >to work anyway, so it's not worth trying to make it work.  Even if you
> >avoided the fault with it trying to fixup target=0, it's just going to
> >fault at the next step when it treats the 'fixup_sym/flags' fields as
> >what it thinks are the 'addend/target' fields of the next entry.
> >
> >> implement it by checking that the size for the entries modulo
> >> sizeof(pseudo_reloc_v1) is zero for v2 and check that the flags (the
> >> reloc-size) field has just values of 8,16,32,64. Otherwise version
one is
> >> assumed.
> >
> >Checking the size doesn't really seem like all that great of an idea
> >given that sizeof(v2) is four words and sizeof(v1) is two words, so all
> >you can really tell here is that it's a v1 list if there happens to be
> >an odd number of entries.  In all other cases you can't tell anything
> >and so you're down to this flags heuristic.  Just assuming that you'll
> >never have a target that's a power of 2 seems really weak, especially
if
> >in the future you need to extend the list of valid flags.
> >
> >I think the "first entry of v2 is a zeroed no-op" is a much better way
> >to go.
>
> Or first entry zeroed, second entry version number.
>
> cgf
>

About this I thought too. I'll modify the patch accordingly to this and
re-post it here (plus a pseudo-reloc.c variant for test).

Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Kai Tietz-2
2008/11/11 Kai Tietz <[hidden email]>:
>
> About this I thought too. I'll modify the patch accordingly to this and
> re-post it here (plus a pseudo-reloc.c variant for test).
>

Here comes the changed version plus the pseudo-reloc.c file.

Is the patch ok for apply on trunk?

Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

psreloc_v2.txt (17K) Download Attachment
pseudo-reloc.c (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Kai Tietz-2
2008/11/11 Kai Tietz <[hidden email]>:
> 2008/11/11 Kai Tietz <[hidden email]>:
>>
>> About this I thought too. I'll modify the patch accordingly to this and
>> re-post it here (plus a pseudo-reloc.c variant for test).
>>
>
> Here comes the changed version plus the pseudo-reloc.c file.
>
> Is the patch ok for apply on trunk?

I noticed that I sent an older pseudo-reloc.c file, so I attached the
current one.

I test it on w64 by the following test files:
a.c: (compiled by gcc -mdll -o a.dll a.c)
#include <stdio.h>

typedef struct abc {
  int a;
  int b;
  int c;
} abc;

__declspec(dllexport) abc my_abc = { 1,2,3 };
__declspec(dllexport) int my_a = 2;
__declspec(dllexport) void show_a(void)
{
  printf ("In dll: %d %p, %d %p\n", my_a, &my_a, my_abc.b, &my_abc.b);
}
b.c: (compiled by gcc -o b.exe b.c a.dll -Wl,--enable-auto-import)
#include <stdio.h>

typedef struct abc {
  int a;
  int b;
  int c;
} abc;

extern abc my_abc;

extern int my_a;
extern void show_a(void);

int main()
{
  show_a();
  printf ("In Exe: %d %p, %d %p\n", my_a, &my_a, my_abc.b, &my_abc.b);
  return 1;
}
Result on execution on w64:
$ ./b.exe
In dll: 2 000000006608200C, 2 0000000066082004
In Exe: 2 000000006608200C, 2 0000000066082004

Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

pseudo-reloc.c (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFA: pseudo-relocations for pe targets

Kai Tietz
> 2008/11/11 Kai Tietz <[hidden email]>:
> > 2008/11/11 Kai Tietz <[hidden email]>:
> >>
> >> About this I thought too. I'll modify the patch accordingly to this
and

> >> re-post it here (plus a pseudo-reloc.c variant for test).
> >>
> >
> > Here comes the changed version plus the pseudo-reloc.c file.
> >
> > Is the patch ok for apply on trunk?
>
> I noticed that I sent an older pseudo-reloc.c file, so I attached the
> current one.
>
> I test it on w64 by the following test files:
> a.c: (compiled by gcc -mdll -o a.dll a.c)
> #include <stdio.h>
>
> typedef struct abc {
>   int a;
>   int b;
>   int c;
> } abc;
>
> __declspec(dllexport) abc my_abc = { 1,2,3 };
> __declspec(dllexport) int my_a = 2;
> __declspec(dllexport) void show_a(void)
> {
>   printf ("In dll: %d %p, %d %p\n", my_a, &my_a, my_abc.b, &my_abc.b);
> }
> b.c: (compiled by gcc -o b.exe b.c a.dll -Wl,--enable-auto-import)
> #include <stdio.h>
>
> typedef struct abc {
>   int a;
>   int b;
>   int c;
> } abc;
>
> extern abc my_abc;
>
> extern int my_a;
> extern void show_a(void);
>
> int main()
> {
>   show_a();
>   printf ("In Exe: %d %p, %d %p\n", my_a, &my_a, my_abc.b, &my_abc.b);
>   return 1;
> }
> Result on execution on w64:
> $ ./b.exe
> In dll: 2 000000006608200C, 2 0000000066082004
> In Exe: 2 000000006608200C, 2 0000000066082004

Can I assume that the patch for binutils/ld is ok?

Cheers,
Kai
12