Adjust relocation type secrel for w64 and fix base-file/dlltool functionality

[Kai Tietz]

Hello,

ChangeLog bfd

2008-07-30  Kai Tietz  <kai.tietz@...>

        * coff-x86_64.c (howto_table): Make R_AMD64_SECREL a 64-bit
        wide relocation.
        * cofflink.c (_bfd_coff_generic_relocate_section):  Write out
bfd_vma
        values instead of long typed values.

ChangeLog binutils

2008-07-30  Kai Tietz  <kai.tietz@...>

        * dlltool.c (PAGE_SIZE): Make of type bfd_vma.
        (PAGE_MASK): Likewise.
        (flush_page): Change argument page_addr to bfd_vma and
        local variable needed, too. Emit DIR64 relocation for w64.
        (sfunc): Compare bfd_vma instead of long types.
        (gen_exp_file): Emit .quad size for import symbol. Adjust
base_file feature
        to read and use bfd_vma instead of long typed address values.

Is this patch ok for apply?

Regards,
 i.A. Kai Tietz



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

Index: src/bfd/coff-x86_64.c
===================================================================
--- src.orig/bfd/coff-x86_64.c
+++ src/bfd/coff-x86_64.c
@@ -323,16 +323,17 @@ static reloc_howto_type howto_table[] =
   /* 32-bit longword section relative relocation (11).  */
   HOWTO (R_AMD64_SECREL, /* type */
  0, /* rightshift */
- 2, /* size (0 = byte, 1 = short, 2 = long) */
- 32, /* bitsize */
+ 4, /* size (0 = byte, 1 = short, 2 = long,
+   4 = long long) */
+ 64, /* bitsize */
  FALSE, /* pc_relative */
  0, /* bitpos */
  complain_overflow_bitfield, /* complain_on_overflow */
  coff_amd64_reloc, /* special_function */
- "secrel32", /* name */
+ "secrel64", /* name */
  TRUE, /* partial_inplace */
- 0xffffffff, /* src_mask */
- 0xffffffff, /* dst_mask */
+ 0xffffffffffffffffll, /* src_mask */
+ 0xffffffffffffffffll, /* dst_mask */
  TRUE), /* pcrel_offset */
 #else
   EMPTY_HOWTO (11),
Index: src/bfd/cofflink.c
===================================================================
--- src.orig/bfd/cofflink.c
+++ src/bfd/cofflink.c
@@ -2985,16 +2985,16 @@ _bfd_coff_generic_relocate_section (bfd
  absolute.  We output the address here to a file.
  This file is then read by dlltool when generating the
  reloc section.  Note that the base file is not
- portable between systems.  We write out a long here,
- and dlltool reads in a long.  */
-      long addr = (rel->r_vaddr
+ portable between systems.  We write out a bfd_vma here,
+ and dlltool reads in a bfd_vma.  */
+      bfd_vma addr = (rel->r_vaddr
    - input_section->vma
    + input_section->output_offset
    + input_section->output_section->vma);
       if (coff_data (output_bfd)->pe)
  addr -= pe_data(output_bfd)->pe_opthdr.ImageBase;
-      if (fwrite (&addr, 1, sizeof (long), (FILE *) info->base_file)
-  != sizeof (long))
+      if (fwrite (&addr, 1, sizeof (bfd_vma), (FILE *) info->base_file)
+  != sizeof (bfd_vma))
  {
   bfd_set_error (bfd_error_system_call);
   return FALSE;
Index: src/binutils/dlltool.c
===================================================================
--- src.orig/binutils/dlltool.c
+++ src/binutils/dlltool.c
@@ -241,8 +241,8 @@
 
 #define show_allnames 0
 
-#define PAGE_SIZE 4096
-#define PAGE_MASK (-PAGE_SIZE)
+#define PAGE_SIZE ((bfd_vma) 4096)
+#define PAGE_MASK ((bfd_vma) (-4096))
 #include "sysdep.h"
 #include "bfd.h"
 #include "libiberty.h"
@@ -712,7 +712,7 @@ static void scan_open_obj_file (bfd *);
 static void scan_obj_file (const char *);
 static void dump_def_info (FILE *);
 static int sfunc (const void *, const void *);
-static void flush_page (FILE *, long *, int, int);
+static void flush_page (FILE *, bfd_vma *, bfd_vma, int);
 static void gen_def_file (void);
 static void generate_idata_ofile (FILE *);
 static void assemble_file (const char *, const char *);
@@ -1584,18 +1584,18 @@ dump_def_info (FILE *f)
 static int
 sfunc (const void *a, const void *b)
 {
-  return *(const long *) a - *(const long *) b;
+  return (int) (*(const bfd_vma *) a - *(const bfd_vma *) b);
 }
 
 static void
-flush_page (FILE *f, long *need, int page_addr, int on_page)
+flush_page (FILE *f, bfd_vma *need, bfd_vma page_addr, int on_page)
 {
   int i;
 
   /* Flush this page.  */
   fprintf (f, "\t%s\t0x%08x\t%s Starting RVA for chunk\n",
    ASM_LONG,
-   page_addr,
+   (int) page_addr,
    ASM_C);
   fprintf (f, "\t%s\t0x%x\t%s Size of block\n",
    ASM_LONG,
@@ -1604,12 +1604,20 @@ flush_page (FILE *f, long *need, int pag
 
   for (i = 0; i < on_page; i++)
     {
-      unsigned long needed = need[i];
+      bfd_vma needed = need[i];
 
       if (needed)
- needed = ((needed - page_addr) | 0x3000) & 0xffff;
+        {
+#ifndef DLLTOOL_MX86_64
+  /* Relocation via HIGHLOW.  */
+          needed = ((needed - page_addr) | 0x3000) & 0xffff;
+#else
+  /* Relocation via DIR64.  */
+  needed = ((needed - page_addr) | 0xa000) & 0xffff;
+#endif
+ }
 
-      fprintf (f, "\t%s\t0x%lx\n", ASM_SHORT, needed);
+      fprintf (f, "\t%s\t0x%lx\n", ASM_SHORT, (long) needed);
     }
 
   /* And padding */
@@ -1970,19 +1978,23 @@ gen_exp_file (void)
     if (create_compat_implib)
       fprintf (f, "__imp_%s:\n", exp->name);
     fprintf (f, "_imp__%s:\n", exp->name);
+#ifdef DLLTOOL_MX86_64
+    fprintf (f, "\t.quad\t%s\n", exp->name);
+#else
     fprintf (f, "\t%s\t%s\n", ASM_LONG, exp->name);
+#endif
   }
     }
 
   /* Dump the reloc section if a base file is provided.  */
   if (base_file)
     {
-      int addr;
-      long need[PAGE_SIZE];
-      long page_addr;
+      bfd_vma addr;
+      bfd_vma need[PAGE_SIZE];
+      bfd_vma page_addr;
       int numbytes;
       int num_entries;
-      long *copy;
+      bfd_vma *copy;
       int j;
       int on_page;
       fprintf (f, "\t.section\t.init\n");
@@ -1993,7 +2005,7 @@ gen_exp_file (void)
       fseek (base_file, 0, SEEK_SET);
       copy = xmalloc (numbytes);
       fread (copy, 1, numbytes, base_file);
-      num_entries = numbytes / sizeof (long);
+      num_entries = numbytes / sizeof (bfd_vma);
 
 
       fprintf (f, "\t.section\t.reloc\n");
@@ -2001,8 +2013,8 @@ gen_exp_file (void)
  {
   int src;
   int dst = 0;
-  int last = -1;
-  qsort (copy, num_entries, sizeof (long), sfunc);
+  bfd_vma last = (bfd_vma) -1;
+  qsort (copy, num_entries, sizeof (bfd_vma), sfunc);
   /* Delete duplicates */
   for (src = 0; src < num_entries; src++)
     {
=

[NightStrike]

On 7/30/08, Kai Tietz <Kai.Tietz@...> wrote:

> Hello,
>
> ChangeLog bfd
>
> 2008-07-30  Kai Tietz  <kai.tietz@...>
>
>        * coff-x86_64.c (howto_table): Make R_AMD64_SECREL a 64-bit
>        wide relocation.
>        * cofflink.c (_bfd_coff_generic_relocate_section):  Write out
> bfd_vma
>        values instead of long typed values.
>
> ChangeLog binutils
>
> 2008-07-30  Kai Tietz  <kai.tietz@...>
>
>        * dlltool.c (PAGE_SIZE): Make of type bfd_vma.
>        (PAGE_MASK): Likewise.
>        (flush_page): Change argument page_addr to bfd_vma and
>        local variable needed, too. Emit DIR64 relocation for w64.
>        (sfunc): Compare bfd_vma instead of long types.
>        (gen_exp_file): Emit .quad size for import symbol. Adjust
> base_file feature
>        to read and use bfd_vma instead of long typed address values.
>
> Is this patch ok for apply?
>
> Regards,
>  i.A. Kai Tietz
>
>
>
> |  (\_/)  This is Bunny. Copy and paste Bunny
> | (='.'=) into your signature to help him gain
> | (")_(") world domination.
>
>

Was this applied yet?

[Nick Clifton]

Hi NightStrike, Hi Kai

> Was this applied yet?

No.  It has not been approved yet either.

Kai - what problem does this patch solve ?  How was it tested ?  Do you
have a test case that can be added to the linker or binutils testsuites ?

One thing that concerns me is the changing of the R_AMD64_SECREL reloc.
  Is this an ABI defined reloc ?

Cheers
   Nick

[Kai Tietz]

Hi Nick,

Nick Clifton <nickc@...> wrote on 26.08.2008 16:48:09:

> Kai - what problem does this patch solve ?  How was it tested ?  Do you
> have a test case that can be added to the linker or binutils testsuites
?
>
> One thing that concerns me is the changing of the R_AMD64_SECREL reloc.
>   Is this an ABI defined reloc ?

The patch was tested for x86_64-pc-mingw32. The secrel issue is, that PE+
has to use DIR64 for relocation and there is not suitable 32-bit ImageBase
relocation present. The R_(I386|AMD64)_SECREL isn't a real coff
relocation. It is mainly used for dwarf2. The problem fixed by this patch
are the base relocations for w64 target for secrel (the second patch I
sent for gas supporting .secrel64 is part necessary, too). dlltool emits
at the moment HIGHLOW relocations, which have to be altered into DIR64
relocations for w64 and the size of the vma's have to be changed to
bfd_vma instead of 'long' stored in the base-file.

Some test in the testsuite needs to be reworked completly for w64 in gas
and ld. But first I wanted to get some response about the patch before
adjusting them.

The most interesting question here is, if it wouldn't be better to alter
the dwarf2 code for mingw in general. The idea was - as I spoke to Aaron
about it - to use instead of .secrel for dwarf2 .rva an use the global
__image_base to relocate the dwarf2 code. So we could get rid of the
necessarity of base relocations in debug section at all and the
implementation would be more near to the unix one.


Cheers,
Kai

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

[Nick Clifton]

Hi Kai,

> The patch was tested for x86_64-pc-mingw32. The secrel issue is, that PE+
> has to use DIR64 for relocation and there is not suitable 32-bit ImageBase
> relocation present. The R_(I386|AMD64)_SECREL isn't a real coff
> relocation. It is mainly used for dwarf2. The problem fixed by this patch
> are the base relocations for w64 target for secrel (the second patch I
> sent for gas supporting .secrel64 is part necessary, too). dlltool emits
> at the moment HIGHLOW relocations, which have to be altered into DIR64
> relocations for w64 and the size of the vma's have to be changed to
> bfd_vma instead of 'long' stored in the base-file.
>
> Some test in the testsuite needs to be reworked completly for w64 in gas
> and ld. But first I wanted to get some response about the patch before
> adjusting them.

Ok - well the patch is approved, so please go ahead and apply it (and
adjust the testsuite entries).

> The most interesting question here is, if it wouldn't be better to alter
> the dwarf2 code for mingw in general. The idea was - as I spoke to Aaron
> about it - to use instead of .secrel for dwarf2 .rva an use the global
> __image_base to relocate the dwarf2 code. So we could get rid of the
> necessarity of base relocations in debug section at all and the
> implementation would be more near to the unix one.

Hmm, whilst it might be neater and simpler to be more like the unix
implementation, would it actually solve any outstanding problems with
the current system ?

Cheers
   Nick

[Kai Tietz]

Hi Nick,

Nick Clifton <nickc@...> wrote on 02.09.2008 08:44:08:

> Hi Kai,
>
> > The patch was tested for x86_64-pc-mingw32. The secrel issue is, that
PE+
> > has to use DIR64 for relocation and there is not suitable 32-bit
ImageBase
> > relocation present. The R_(I386|AMD64)_SECREL isn't a real coff
> > relocation. It is mainly used for dwarf2. The problem fixed by this
patch
> > are the base relocations for w64 target for secrel (the second patch I

> > sent for gas supporting .secrel64 is part necessary, too). dlltool
emits
> > at the moment HIGHLOW relocations, which have to be altered into DIR64

> > relocations for w64 and the size of the vma's have to be changed to
> > bfd_vma instead of 'long' stored in the base-file.
> >
> > Some test in the testsuite needs to be reworked completly for w64 in
gas
> > and ld. But first I wanted to get some response about the patch before

> > adjusting them.
>
> Ok - well the patch is approved, so please go ahead and apply it (and
> adjust the testsuite entries).

Ok, I will prepare an new patch for this with testsuite adjustments and
includubg the gas part, too.
As long as gcc uses 32-bit relocations I won't modify the 32-bit secrel to
be 64-bit.

> > The most interesting question here is, if it wouldn't be better to
alter
> > the dwarf2 code for mingw in general. The idea was - as I spoke to
Aaron
> > about it - to use instead of .secrel for dwarf2 .rva an use the global

> > __image_base to relocate the dwarf2 code. So we could get rid of the
> > necessarity of base relocations in debug section at all and the
> > implementation would be more near to the unix one.
>
> Hmm, whilst it might be neater and simpler to be more like the unix
> implementation, would it actually solve any outstanding problems with
> the current system ?

The interesting point about this approach is, that we could get rid of the
need to have relocations in debug-section and dwarf2 information at all
and that 32-bit dwarf2 can be supported for w64, too. Otherwise w64 has to
use 64-bit dwarf2, which consumes much more space.

Cheers,
Kai

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

[Nick Clifton]

Hi Kai,

> The interesting point about this approach is, that we could get rid of the
> need to have relocations in debug-section and dwarf2 information at all
> and that 32-bit dwarf2 can be supported for w64, too.

Hmm, well my feeling is "if it ain't broke, don't fix it".  The current
method works right ?  Even if it is not the best solution, I would
prefer to keep it as introducing a new system is very likely to
introduce new bugs.

> Otherwise w64 has to
> use 64-bit dwarf2, which consumes much more space.

Surely the space restrictions on 64-bit systems are much less serious ?
  I.e. I would assume that any 64-bit system would have a large amount
of memory - otherwise why use 64-bits ?

Cheers
   Nick