diff options
author | Ömer Sinan Ağacan <omeragacan@gmail.com> | 2020-06-17 13:03:56 +0300 |
---|---|---|
committer | Ben Gamari <ben@smart-cactus.org> | 2020-09-29 12:39:48 -0400 |
commit | 16f83b5799f976b5ca839103fc83628ccc11b01f (patch) | |
tree | b452e3bb650699db7c44fdf24535e6c07a34a2b1 | |
parent | 3ebc51e667d3b41735f28eee7a63058989765009 (diff) | |
download | haskell-16f83b5799f976b5ca839103fc83628ccc11b01f.tar.gz |
Fix uninitialized field read in Linker.c
Valgrind report of the bug when running the test `linker_unload`:
==29666== Conditional jump or move depends on uninitialised value(s)
==29666== at 0x369C5B4: setOcInitialStatus (Linker.c:1305)
==29666== by 0x369C6C5: mkOc (Linker.c:1347)
==29666== by 0x36C027A: loadArchive_ (LoadArchive.c:522)
==29666== by 0x36C0600: loadArchive (LoadArchive.c:626)
==29666== by 0x2C144CD: ??? (in /home/omer/haskell/ghc_2/testsuite/tests/rts/linker/linker_unload.run/linker_unload)
==29666==
==29666== Conditional jump or move depends on uninitialised value(s)
==29666== at 0x369C5B4: setOcInitialStatus (Linker.c:1305)
==29666== by 0x369C6C5: mkOc (Linker.c:1347)
==29666== by 0x369C9F6: preloadObjectFile (Linker.c:1507)
==29666== by 0x369CA8D: loadObj_ (Linker.c:1536)
==29666== by 0x369CB17: loadObj (Linker.c:1557)
==29666== by 0x3866BC: main (linker_unload.c:33)
The problem is `mkOc` allocates a new `ObjectCode` and calls
`setOcInitialStatus` without initializing the `status` field.
`setOcInitialStatus` reads the field as first thing:
static void setOcInitialStatus(ObjectCode* oc) {
if (oc->status == OBJECT_DONT_RESOLVE)
return;
if (oc->archiveMemberName == NULL) {
oc->status = OBJECT_NEEDED;
} else {
oc->status = OBJECT_LOADED;
}
}
`setOcInitialStatus` is unsed in two places for two different purposes:
in `mkOc` where we don't have the `status` field initialized yet (`mkOc`
is supposed to initialize it), and `loadOc` where we do have `status`
field initialized and we want to update it. Instead of splitting the
function into two functions which are both called just once I inline the
functions in the use sites and remove it.
Fixes #18342
(cherry picked from commit 08c1cb0f30770acbf366423f085f8ef92f7f6a06)
-rw-r--r-- | rts/Linker.c | 36 |
1 files changed, 16 insertions, 20 deletions
diff --git a/rts/Linker.c b/rts/Linker.c index ebd99bd708..bd951e1950 100644 --- a/rts/Linker.c +++ b/rts/Linker.c @@ -1238,23 +1238,6 @@ void freeObjectCode (ObjectCode *oc) stgFree(oc); } -/* ----------------------------------------------------------------------------- -* Sets the initial status of a fresh ObjectCode -*/ -static void setOcInitialStatus(ObjectCode* oc) { - /* If a target has requested the ObjectCode not to be resolved then - honor this requests. Usually this means the ObjectCode has not been - initialized and can't be. */ - if (oc->status == OBJECT_DONT_RESOLVE) - return; - - if (oc->archiveMemberName == NULL) { - oc->status = OBJECT_NEEDED; - } else { - oc->status = OBJECT_LOADED; - } -} - ObjectCode* mkOc( pathchar *path, char *image, int imageSize, bool mapped, char *archiveMemberName, int misalignment ) { @@ -1287,7 +1270,11 @@ mkOc( pathchar *path, char *image, int imageSize, oc->archiveMemberName = NULL; } - setOcInitialStatus( oc ); + if (oc->archiveMemberName == NULL) { + oc->status = OBJECT_NEEDED; + } else { + oc->status = OBJECT_LOADED; + } oc->fileSize = imageSize; oc->symbols = NULL; @@ -1558,8 +1545,17 @@ HsInt loadOc (ObjectCode* oc) # endif #endif - /* loaded, but not resolved yet, ensure the OC is in a consistent state */ - setOcInitialStatus( oc ); + /* Loaded, but not resolved yet, ensure the OC is in a consistent state. + If a target has requested the ObjectCode not to be resolved then honor + this requests. Usually this means the ObjectCode has not been initialized + and can't be. */ + if (oc->status != OBJECT_DONT_RESOLVE) { + if (oc->archiveMemberName == NULL) { + oc->status = OBJECT_NEEDED; + } else { + oc->status = OBJECT_LOADED; + } + } IF_DEBUG(linker, debugBelch("loadOc: done.\n")); return 1; |