Address some todos and fixmes

9 files changed, 32 insertions, 31 deletions

diff --git a/rts/Linker.c b/rts/Linker.c
index 3d56989677..4eea5463c9 100644
--- a/rts/Linker.c
+++ b/rts/Linker.c
@@ -1424,6 +1424,7 @@ preloadObjectFile (pathchar *path)
 
    IF_DEBUG(linker, debugBelch("loadObj: preloaded image at %p\n", (void *) image));
 
+   /* FIXME (AP): =mapped= parameter unconditionally set to true */
    oc = mkOc(path, image, fileSize, true, NULL, misalignment);
 
 #if defined(OBJFORMAT_MACHO)
diff --git a/rts/LinkerInternals.h b/rts/LinkerInternals.h
index cd36fd55d2..a773b3e6a9 100644
--- a/rts/LinkerInternals.h
+++ b/rts/LinkerInternals.h
@@ -118,8 +118,6 @@ typedef struct _Segment {
     int n_sections;
 } Segment;
 
-/* todo (AP): add freeSegments */
-
 /*
  * We must keep track of the StablePtrs that are created for foreign
  * exports by constructor functions when the module is loaded, so that
@@ -194,10 +192,7 @@ typedef struct _ObjectCode {
        after allocation, so that we can use realloc */
     int        misalignment;
 
-    /* The section-kind entries for this object module.  Linked
-       list. */
-    /* fixme (AP): doesn't look like a linked list. On MachO it's an array, and
-     * generally Section struct doesn't have pointers to next. */
+    /* The section-kind entries for this object module. An array. */
     int n_sections;
     Section* sections;
 
diff --git a/rts/linker/MachO.c b/rts/linker/MachO.c
index 772ae21274..7e1af76b6b 100644
--- a/rts/linker/MachO.c
+++ b/rts/linker/MachO.c
@@ -147,10 +147,12 @@ ocInit_MachO(ObjectCode * oc)
             oc->info->macho_symbols[i].name  = oc->info->names
                                              + oc->info->nlist[i].n_un.n_strx;
             oc->info->macho_symbols[i].nlist = &oc->info->nlist[i];
-             /* we don't have an address for this symbol yet; this will be
-              * populated during ocGetNames_MachO. hence addr = NULL
+             /* We don't have an address for this symbol yet; this
+              * will be populated during ocGetNames_MachO. Hence init
+              * with NULL
               */
             oc->info->macho_symbols[i].addr  = NULL;
+            oc->info->macho_symbols[i].got_addr = NULL;
         }
     }
 }
@@ -686,7 +688,7 @@ static int
 relocateSection(ObjectCode* oc, int curSection)
 {
     Section * sect = &oc->sections[curSection];
-    MachOSection * msect = sect->info->macho_section; // for convenience access
+    MachOSection * msect = sect->info->macho_section; // for access convenience
     MachORelocationInfo * relocs = sect->info->relocation_info;
     MachOSymbol * symbols = oc->info->macho_symbols;
 
@@ -730,33 +732,29 @@ relocateSection(ObjectCode* oc, int curSection)
         switch(reloc->r_length)
         {
             case 0:
-                checkProddableBlock(oc,thingPtr,1);
                 thing = *(uint8_t*)thingPtr;
                 relocLenBytes = 1;
                 break;
             case 1:
-                checkProddableBlock(oc,thingPtr,2);
                 thing = *(uint16_t*)thingPtr;
                 relocLenBytes = 2;
                 break;
             case 2:
-                checkProddableBlock(oc,thingPtr,4);
                 thing = *(uint32_t*)thingPtr;
                 relocLenBytes = 4;
                 break;
             case 3:
-                checkProddableBlock(oc,thingPtr,8);
                 thing = *(uint64_t*)thingPtr;
                 relocLenBytes = 8;
                 break;
             default:
                 barf("Unknown size.");
         }
+        checkProddableBlock(oc,thingPtr,relocLenBytes);
 
         /*
-         * With SIGNED_N the relocation is not at the
-         * end of instruction and baseValue needs to be
-         * adjusted accordingly.
+         * With SIGNED_N the relocation is not at the end of the
+         * instruction and baseValue needs to be adjusted accordingly.
          */
         switch (type) {
             case X86_64_RELOC_SIGNED_1:
@@ -787,7 +785,6 @@ relocateSection(ObjectCode* oc, int curSection)
             IF_DEBUG(linker, debugBelch("relocateSection: making jump island for %s, extern = %d, X86_64_RELOC_GOT\n",
                                         nm, reloc->r_extern));
 
-            ASSERT(reloc->r_extern); // fixme: redundant?
             if (reloc->r_extern == 0) {
                     errorBelch("\nrelocateSection: global offset table relocation for symbol with r_extern == 0\n");
             }
@@ -800,8 +797,8 @@ relocateSection(ObjectCode* oc, int curSection)
 
                     addr = lookupSymbol_(nm);
                     IF_DEBUG(linker, debugBelch("relocateSection: looked up %s, "
-                                                "external X86_64_RELOC_GOT or X86_64_RELOC_GOT_LOAD\n", nm));
-                    IF_DEBUG(linker, debugBelch("               : addr = %p\n", addr));
+                                                "external X86_64_RELOC_GOT or X86_64_RELOC_GOT_LOAD\n"
+                                                "               : addr = %p\n", nm, addr));
 
                     if (addr == NULL) {
                             errorBelch("\nlookupSymbol failed in relocateSection (RELOC_GOT)\n"
@@ -819,6 +816,7 @@ relocateSection(ObjectCode* oc, int curSection)
                     if ((symbol->nlist->n_type & N_TYPE) == N_SECT) {
                         ASSERT(symbol->addr != NULL);
                         addr = symbol->addr;
+
                         IF_DEBUG(linker, debugBelch("relocateSection: calculated relocation of "
                                                     "non-external X86_64_RELOC_GOT or X86_64_RELOC_GOT_LOAD\n"));
                         IF_DEBUG(linker, debugBelch("               : addr = %p\n", addr));
@@ -829,7 +827,7 @@ relocateSection(ObjectCode* oc, int curSection)
             }
 
             // creates a jump island for every relocation entry for a symbol
-            // fixme: use got_addr to store the loc. of a jump island for a symbol
+            // TODO (AP): use got_addr to store the loc. of a jump island to reuse later
             value = (uint64_t) &makeSymbolExtra(oc, reloc->r_symbolnum, (unsigned long)addr)->addr;
 
             type = X86_64_RELOC_SIGNED;
@@ -875,7 +873,7 @@ relocateSection(ObjectCode* oc, int curSection)
              * For instance, in a signed case:
              * thing = <displ. to to section r_symbolnum *in the image*> (1)
              *       + <offset within r_symbolnum section>
-             * (1) needs to be updated due to different section layout in memory.
+             * (1) needs to be updated due to different section placement in memory.
              */
 
             CHECKM(reloc->r_symbolnum > 0,
@@ -979,6 +977,13 @@ relocateSection(ObjectCode* oc, int curSection)
         }
 
         IF_DEBUG(linker, debugBelch("relocateSection: thing = %p\n", (void *) thing));
+        if (0 == reloc->r_extern) {
+            if (reloc->r_pcrel) {
+                checkProddableBlock(oc, (void *)((char *)thing + baseValue), relocLenBytes);
+            } else {
+                checkProddableBlock(oc, (void *)thing, relocLenBytes);
+            }
+        }
 
         switch(reloc->r_length)
         {
diff --git a/testsuite/tests/rts/linker/Makefile b/testsuite/tests/rts/linker/Makefile
index c5d80faa3f..838a83a2c7 100644
--- a/testsuite/tests/rts/linker/Makefile
+++ b/testsuite/tests/rts/linker/Makefile
@@ -2,7 +2,7 @@ TOP=../../..
 include $(TOP)/mk/boilerplate.mk
 include $(TOP)/mk/test.mk
 
-unsigned_reloc_macho_64:
-	as -o unsigned_reloc_macho_64.o unsigned_reloc_macho_64.s
-	'$(TEST_HC)' $(TEST_HC_OPTS_NO_RTSOPTS) -v0 --make -no-rtsopts-suggestions -no-hs-main -o runner -debug runner.c
-	./runner unsigned_reloc_macho_64.o getAnswer
+unsigned_reloc_macho_x64:
+	as -o unsigned_reloc_macho_x64.o unsigned_reloc_macho_x64.s
+	'$(TEST_HC)' $(TEST_HC_OPTS_NO_RTSOPTS) -v0 --make -no-rtsopts-suggestions -no-hs-main -o runner runner.c
+	./runner unsigned_reloc_macho_x64.o getAnswer
diff --git a/testsuite/tests/rts/linker/all.T b/testsuite/tests/rts/linker/all.T
index d1b3c22b27..66c2156660 100644
--- a/testsuite/tests/rts/linker/all.T
+++ b/testsuite/tests/rts/linker/all.T
@@ -1,9 +1,9 @@
 # -*-: mode: python -*-
 
-test('unsigned_reloc_macho_64',
+test('unsigned_reloc_macho_x64',
      [
-         extra_files(['runner.c', 'unsigned_reloc_macho_64.s']),
+         extra_files(['runner.c', 'unsigned_reloc_macho_x64.s']),
          unless(opsys('darwin'), skip),
          unless(arch('x86_64'), skip)
      ],
-     run_command, ['$MAKE -s --no-print-directory unsigned_reloc_macho_64'])
+     run_command, ['$MAKE -s --no-print-directory unsigned_reloc_macho_x64'])
diff --git a/testsuite/tests/rts/linker/runner b/testsuite/tests/rts/linker/runner
deleted file mode 100755
index 7771d4ab57..0000000000
--- a/testsuite/tests/rts/linker/runner
+++ /dev/null
diff --git a/testsuite/tests/rts/linker/unsigned_reloc_macho_64.stdout b/testsuite/tests/rts/linker/unsigned_reloc_macho_64.stdout
deleted file mode 100644
index 8251451f5f..0000000000
--- a/testsuite/tests/rts/linker/unsigned_reloc_macho_64.stdout
+++ /dev/null
@@ -1,2 +0,0 @@
-Linking: path = unsigned_reloc_macho_64.o, symname = _getAnswer
-42
\ No newline at end of file
diff --git a/testsuite/tests/rts/linker/unsigned_reloc_macho_64.s b/testsuite/tests/rts/linker/unsigned_reloc_macho_x64.s
index 733c9863bd..a4c35028d8 100644
--- a/testsuite/tests/rts/linker/unsigned_reloc_macho_64.s
+++ b/testsuite/tests/rts/linker/unsigned_reloc_macho_x64.s
@@ -6,6 +6,6 @@ _getAnswer:
         movq    (%rdx), %rax
         ret
 
-        .data
+        .data                   # assembler generates an unsigned reloc for L2
 L1:     .quad 42
 L2:     .quad L1
diff --git a/testsuite/tests/rts/linker/unsigned_reloc_macho_x64.stdout b/testsuite/tests/rts/linker/unsigned_reloc_macho_x64.stdout
new file mode 100644
index 0000000000..326c0d48b8
--- /dev/null
+++ b/testsuite/tests/rts/linker/unsigned_reloc_macho_x64.stdout
@@ -0,0 +1,2 @@
+Linking: path = unsigned_reloc_macho_x64.o, symname = _getAnswer
+42
\ No newline at end of file