Commit 67336a67 authored by Ömer Sinan Ağacan's avatar Ömer Sinan Ağacan

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
parent eb8115a8
Pipeline #21102 passed with stages
in 326 minutes and 51 seconds
......@@ -1297,23 +1297,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 ) {
......@@ -1346,7 +1329,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->n_symbols = 0;
......@@ -1638,8 +1625,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;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment