summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTamar Christina <tamar@zhox.com>2018-12-04 00:29:08 +0000
committerBen Gamari <ben@smart-cactus.org>2018-12-06 12:49:25 -0500
commitc64918c1fc86d8feb15bf53b9db11ad81095b8b9 (patch)
tree36d1b3100d1aa1e4b0f427372377353ac9b3de08
parentbf074e3e92c138947d0da3979838e5588f6f292c (diff)
downloadhaskell-c64918c1fc86d8feb15bf53b9db11ad81095b8b9.tar.gz
linker: store entire link map and use it.
Summary: This fixes a corner case in which we have seen the symbol multiple times in different static libraries, but due to a depencency we end up loading the symbol from a library other than the first one. Previously the runtime linker would only track symbols from the first library and did not store the full link map. In this case it was unable to find the address for the symbols in the second library during delay loading. This change stores the address of all symbols seen so a full link map is generated, such that when we make a different decision later than what was expected we're able to still correctly load the library. Test Plan: ./validate, new testcase T15894 Reviewers: angerman, bgamari, erikd, simonmar Reviewed By: bgamari Subscribers: rwbarton, carter GHC Trac Issues: #15894 Differential Revision: https://phabricator.haskell.org/D5353 (cherry picked from commit a8b7cef4d45a5003bf7584e06912f0f632116c71)
-rw-r--r--rts/Linker.c23
-rw-r--r--rts/LinkerInternals.h10
-rw-r--r--rts/linker/Elf.c5
-rw-r--r--rts/linker/MachO.c8
-rw-r--r--rts/linker/PEi386.c11
-rw-r--r--testsuite/tests/rts/T15894/Makefile8
-rw-r--r--testsuite/tests/rts/T15894/T15894.stdout1
-rw-r--r--testsuite/tests/rts/T15894/all.T3
-rw-r--r--testsuite/tests/rts/T15894/copysign.c11
-rw-r--r--testsuite/tests/rts/T15894/main.hs5
10 files changed, 64 insertions, 21 deletions
diff --git a/rts/Linker.c b/rts/Linker.c
index aa6ec7fe7a..b7e83858b0 100644
--- a/rts/Linker.c
+++ b/rts/Linker.c
@@ -961,7 +961,7 @@ void ghci_enquire(SymbolAddr* addr)
for (oc = objects; oc; oc = oc->next) {
for (i = 0; i < oc->n_symbols; i++) {
- sym = oc->symbols[i];
+ sym = oc->symbols[i].name;
if (sym == NULL) continue;
a = NULL;
if (a == NULL) {
@@ -1103,8 +1103,8 @@ static void removeOcSymbols (ObjectCode *oc)
// Remove all the mappings for the symbols within this object..
int i;
for (i = 0; i < oc->n_symbols; i++) {
- if (oc->symbols[i] != NULL) {
- ghciRemoveSymbolTable(symhash, oc->symbols[i], oc);
+ if (oc->symbols[i].name != NULL) {
+ ghciRemoveSymbolTable(symhash, oc->symbols[i].name, oc);
}
}
@@ -1565,18 +1565,21 @@ int ocTryLoad (ObjectCode* oc) {
are to be loaded by this call.
This call is intended to have no side-effects when a non-duplicate
- symbol is re-inserted.
+ symbol is re-inserted. A symbol is only a duplicate if the object file
+ it is defined in has had it's relocations resolved. A resolved object
+ file means the symbols inside it are required.
- We set the Address to NULL since that is not used to distinguish
- symbols. Duplicate symbols are distinguished by name and oc.
+ The symbol address is not used to distinguish symbols. Duplicate symbols
+ are distinguished by name, oc and attributes (weak symbols etc).
*/
int x;
- SymbolName* symbol;
+ Symbol_t symbol;
for (x = 0; x < oc->n_symbols; x++) {
symbol = oc->symbols[x];
- if ( symbol
- && !ghciInsertSymbolTable(oc->fileName, symhash, symbol, NULL,
- isSymbolWeak(oc, symbol), oc)) {
+ if ( symbol.name
+ && !ghciInsertSymbolTable(oc->fileName, symhash, symbol.name,
+ symbol.addr,
+ isSymbolWeak(oc, symbol.name), oc)) {
return 0;
}
}
diff --git a/rts/LinkerInternals.h b/rts/LinkerInternals.h
index 21602f15aa..b48fc75966 100644
--- a/rts/LinkerInternals.h
+++ b/rts/LinkerInternals.h
@@ -20,6 +20,14 @@
typedef void SymbolAddr;
typedef char SymbolName;
+/* Hold extended information about a symbol in case we need to resolve it at a
+ late stage. */
+typedef struct _Symbol
+{
+ SymbolName *name;
+ SymbolAddr *addr;
+} Symbol_t;
+
/* Indication of section kinds for loaded objects. Needed by
the GC for deciding whether or not a pointer on the stack
is a code pointer.
@@ -144,7 +152,7 @@ typedef struct _ObjectCode {
this object into the global symbol hash table. This is so that
we know which parts of the latter mapping to nuke when this
object is removed from the system. */
- char** symbols;
+ Symbol_t *symbols;
int n_symbols;
/* ptr to mem containing the object file image */
diff --git a/rts/linker/Elf.c b/rts/linker/Elf.c
index f2fd88f750..ede0482c6b 100644
--- a/rts/linker/Elf.c
+++ b/rts/linker/Elf.c
@@ -816,7 +816,7 @@ ocGetNames_ELF ( ObjectCode* oc )
oc->n_symbols += symTab->n_symbols;
}
- oc->symbols = stgCallocBytes(oc->n_symbols, sizeof(SymbolName*),
+ oc->symbols = stgCallocBytes(oc->n_symbols, sizeof(Symbol_t),
"ocGetNames_ELF(oc->symbols)");
// Note calloc: if we fail partway through initializing symbols, we need
// to undo the additions to the symbol table so far. We know which ones
@@ -927,7 +927,8 @@ ocGetNames_ELF ( ObjectCode* oc )
) {
goto fail;
}
- oc->symbols[curSymbol++] = nm;
+ oc->symbols[curSymbol++].name = nm;
+ oc->symbols[curSymbol].addr = symbol->addr;
}
} else {
/* Skip. */
diff --git a/rts/linker/MachO.c b/rts/linker/MachO.c
index 053a45de10..b6a14d142c 100644
--- a/rts/linker/MachO.c
+++ b/rts/linker/MachO.c
@@ -1608,7 +1608,7 @@ ocGetNames_MachO(ObjectCode* oc)
*/
IF_DEBUG(linker, debugBelch("ocGetNames_MachO: %d external symbols\n",
oc->n_symbols));
- oc->symbols = stgMallocBytes(oc->n_symbols * sizeof(SymbolName*),
+ oc->symbols = stgMallocBytes(oc->n_symbols * sizeof(Symbol_t),
"ocGetNames_MachO(oc->symbols)");
if (oc->info->symCmd) {
@@ -1639,7 +1639,8 @@ ocGetNames_MachO(ObjectCode* oc)
, HS_BOOL_FALSE
, oc);
- oc->symbols[curSymbol] = nm;
+ oc->symbols[curSymbol].name = nm;
+ oc->symbols[curSymbol].addr = addr;
curSymbol++;
}
}
@@ -1676,7 +1677,8 @@ ocGetNames_MachO(ObjectCode* oc)
IF_DEBUG(linker, debugBelch("ocGetNames_MachO: inserting common symbol: %s\n", nm));
ghciInsertSymbolTable(oc->fileName, symhash, nm,
(void*)commonCounter, HS_BOOL_FALSE, oc);
- oc->symbols[curSymbol] = nm;
+ oc->symbols[curSymbol].name = nm;
+ oc->symbols[curSymbol].addr = oc->info->macho_symbols[i].addr;
curSymbol++;
commonCounter += sz;
diff --git a/rts/linker/PEi386.c b/rts/linker/PEi386.c
index 49aa16d3e3..bcf857eafc 100644
--- a/rts/linker/PEi386.c
+++ b/rts/linker/PEi386.c
@@ -1467,7 +1467,7 @@ ocGetNames_PEi386 ( ObjectCode* oc )
/* Copy exported symbols into the ObjectCode. */
oc->n_symbols = info->numberOfSymbols;
- oc->symbols = stgCallocBytes(sizeof(SymbolName*), oc->n_symbols,
+ oc->symbols = stgCallocBytes(sizeof(Symbol_t), oc->n_symbols,
"ocGetNames_PEi386(oc->symbols)");
/* Work out the size of the global BSS section */
@@ -1623,8 +1623,8 @@ ocGetNames_PEi386 ( ObjectCode* oc )
/* debugBelch("addSymbol %p `%s' Weak:%lld \n", addr, sname, isWeak); */
IF_DEBUG(linker, debugBelch("addSymbol %p `%s'\n", addr,sname));
ASSERT(i < (uint32_t)oc->n_symbols);
- /* cstring_from_COFF_symbol_name always succeeds. */
- oc->symbols[i] = (SymbolName*)sname;
+ oc->symbols[i].name = sname;
+ oc->symbols[i].addr = addr;
if (isWeak) {
setWeakSymbol(oc, sname);
}
@@ -1637,7 +1637,8 @@ ocGetNames_PEi386 ( ObjectCode* oc )
} else {
/* We're skipping the symbol, but if we ever load this
object file we'll want to skip it then too. */
- oc->symbols[i] = NULL;
+ oc->symbols[i].name = NULL;
+ oc->symbols[i].addr = NULL;
# if 0
debugBelch(
@@ -2202,7 +2203,7 @@ resolveSymbolAddr_PEi386 (pathchar* buffer, int size,
"resolveSymbolAddr");
int blanks = 0;
for (int i = 0; i < obj->n_symbols; i++) {
- SymbolName* sym = obj->symbols[i];
+ SymbolName* sym = obj->symbols[i].name;
if (sym == NULL)
{
blanks++;
diff --git a/testsuite/tests/rts/T15894/Makefile b/testsuite/tests/rts/T15894/Makefile
new file mode 100644
index 0000000000..f8499f60d9
--- /dev/null
+++ b/testsuite/tests/rts/T15894/Makefile
@@ -0,0 +1,8 @@
+TOP=../../..
+include $(TOP)/mk/boilerplate.mk
+include $(TOP)/mk/test.mk
+
+T15894:
+ '$(TEST_CC)' -O2 -fno-builtin -c copysign.c -o copysign.o
+ '$(AR)' rsc libcopysign.a copysign.o
+ echo main | LIBRARY_PATH="$(PWD)" '$(TEST_HC)' $(TEST_HC_OPTS_INTERACTIVE) main.hs -lcopysign
diff --git a/testsuite/tests/rts/T15894/T15894.stdout b/testsuite/tests/rts/T15894/T15894.stdout
new file mode 100644
index 0000000000..fa65621f1e
--- /dev/null
+++ b/testsuite/tests/rts/T15894/T15894.stdout
@@ -0,0 +1 @@
+72.25
diff --git a/testsuite/tests/rts/T15894/all.T b/testsuite/tests/rts/T15894/all.T
new file mode 100644
index 0000000000..07733669a4
--- /dev/null
+++ b/testsuite/tests/rts/T15894/all.T
@@ -0,0 +1,3 @@
+test('T15894',
+ [extra_files(['copysign.c', 'main.hs']), when(ghc_dynamic(), skip)],
+ run_command, ['$MAKE -s --no-print-directory T15894'])
diff --git a/testsuite/tests/rts/T15894/copysign.c b/testsuite/tests/rts/T15894/copysign.c
new file mode 100644
index 0000000000..19c5568e5b
--- /dev/null
+++ b/testsuite/tests/rts/T15894/copysign.c
@@ -0,0 +1,11 @@
+double
+copysign (double x, double y)
+{
+ return x * y;
+}
+
+double
+with_copysign (double x)
+{
+ return copysign (x, x);
+}
diff --git a/testsuite/tests/rts/T15894/main.hs b/testsuite/tests/rts/T15894/main.hs
new file mode 100644
index 0000000000..f47fe85de3
--- /dev/null
+++ b/testsuite/tests/rts/T15894/main.hs
@@ -0,0 +1,5 @@
+module Main where
+
+foreign import ccall "with_copysign" c_with_copysign :: Double -> Double
+
+main = print $ c_with_copysign 8.5