Another fix for #16525 (object unloading in GHCi)

changed the description

added 81 commits

8278c569...502647f7 - 66 commits from branch ghc:master
5dd18660 - Fix and enable object unloading in GHCi
db468843 - WIP implementing new plan
a468b49d - Implement TODOs
f2f86016 - Fix a few bugs
c7a78f8b - Fix more bugs
9ebd96f4 - Fix link field
57bbd64f - Fix checking THUNK_SELECTOR payload
9d602288 - Free oc stable ptrs in checkUnload_ to allow collecting unloaded ojbs
dc746bbb - Restore 'just purge' behavior
69470dfb - Fix removeOcSymbols calls
0d7b6d22 - Fwd declare removeOcSymbols and freeOcStablePtrs
640472b0 - Update a test pgm
beab9d68 - Update oc status on objectUnload
7c25223d - Mark object code in GC
34d8aa00 - Remove new tests for now (hard to check output)

marked as a Work In Progress from osa1/ghc@db468843

Looking at the segfaults, locally I can reproduce, but only if I build GHC using Hadrian. No segfaults with Make-built one.

Locally the segfault happens here:

Program received signal SIGSEGV, Segmentation fault.
─── Assembly ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────
0x00007f2e59b9de7a __strcmp_sse2_unaligned+10 and    $0xfff,%eax
0x00007f2e59b9de7f __strcmp_sse2_unaligned+15 cmp    $0xfc0,%eax
0x00007f2e59b9de84 __strcmp_sse2_unaligned+20 jg     0x7f2e59b9e102 <__strcmp_sse2_unaligned+658>
0x00007f2e59b9de8a __strcmp_sse2_unaligned+26 movdqu (%rdi),%xmm1 <----------- segfault
0x00007f2e59b9de8e __strcmp_sse2_unaligned+30 movdqu (%rsi),%xmm0
0x00007f2e59b9de92 __strcmp_sse2_unaligned+34 pcmpeqb %xmm1,%xmm0
0x00007f2e59b9de96 __strcmp_sse2_unaligned+38 pminub %xmm1,%xmm0
─── Source ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
26     pxor    %xmm7, %xmm7
27     orl    %esi, %eax
28     andl    $4095, %eax
29     cmpl    $4032, %eax
30     jg    L(cross_page)
31     movdqu    (%rdi), %xmm1 <----------------- segfault
32     movdqu    (%rsi), %xmm0
33     pcmpeqb    %xmm1, %xmm0
34     pminub    %xmm1, %xmm0
35     pxor    %xmm1, %xmm1
36     pcmpeqb    %xmm1, %xmm0
─── Stack ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
[0] from 0x00007f2e59b9de8a in __strcmp_sse2_unaligned+26 at ../sysdeps/x86_64/multiarch/strcmp-sse2-unaligned.S:31
(no arguments)
[1] from 0x0000000003b01222 in compareStr+35 at rts/Hash.c:112
arg key1 = 139837005706570
arg key2 = 139837071986036
[+]
─── Threads ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────
[1] id 22254 from 0x00007f2e59b9de8a in __strcmp_sse2_unaligned+26 at ../sysdeps/x86_64/multiarch/strcmp-sse2-unaligned.S:31
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
__strcmp_sse2_unaligned () at ../sysdeps/x86_64/multiarch/strcmp-sse2-unaligned.S:31
31              movdqu  (%rdi), %xmm1
>>> bt
#0  __strcmp_sse2_unaligned () at ../sysdeps/x86_64/multiarch/strcmp-sse2-unaligned.S:31
#1  0x0000000003b01222 in compareStr (key1=139837006706570, key2=139837071986036) at rts/Hash.c:112
#2  0x0000000003b014fd in lookupHashTable_inlined (table=0x45f7da0, key=139837071986036, f=0x3b01170 <hashStr>, cmp=0x3b011ff <compareStr>) at rts/Hash.c:205
#3  0x0000000003b015ad in lookupStrHashTable (table=0x45f7da0, key=0x7f2e5b008d74 "Test_caf_info") at rts/Hash.c:229
#4  0x0000000003b0798f in ghciInsertSymbolTable (obj_name=0x56614b0 "Test.o", table=0x45f7da0, key=0x7f2e5b008d74 "Test_caf_info", data=0x413eb2c8, weak=0, owner=0x4dd19c0) at rts/Linker.c:261
#5  0x0000000003b2a9eb in ocGetNames_ELF (oc=0x4dd110c0) at rts/linker/Elf.c:942
#6  0x0000000003b097db in loadOc (oc=0x4dd19c0) at rts/Linker.c:1623
#7  0x0000000003b096c4 in loadObj_ (path=0x2d6ae9 "Test.o") at rts/Linker.c:1542
#8  0x0000000003b09730 in loadObj (path=0x2d6ae9 "Test.o") at rts/Linker.c:1560
#9  0x000000000038c7c1 in main ()

This is because in comapreStr the first key points to free-ed memory.

Here's a smaller program that segfaults the same way:

#include "ghcconfig.h"
#include "Rts.h"

#define OBJPATH "Test.o"

typedef int testfun(int);

extern void loadPackages(void);

int main (int argc, char *argv[])
{
    RtsConfig conf = defaultRtsConfig;
    conf.rts_opts_enabled = RtsOptsAll;
    hs_init_ghc(&argc, &argv, conf);

    initLinker_(0);

    loadPackages();

    int r = loadObj(OBJPATH);
    if (!r) {
        errorBelch("loadObj(%s) failed", OBJPATH);
        exit(1);
    }
    r = resolveObjs();
    if (!r) {
        errorBelch("resolveObjs failed");
        exit(1);
    }
    testfun *f = lookupSymbol("f");
    if (!f) {
        errorBelch("lookupSymbol failed");
        exit(1);
    }
    r = f(3);
    if (r != 4) {
        errorBelch("call failed; %d", r);
        exit(1);
    }
    // check that we can purge first, then unload
    purgeObj(OBJPATH);
    performMajorGC();
    unloadObj(OBJPATH);
    performMajorGC();
    fflush(stdout);

    loadObj(OBJPATH);

    hs_exit();
    exit(0);
}

It seems like we're not removing entries in symhash after unloading the object code, and the keys of the hash table live in struct ObjectCodes, so when we free an ObjectCode without removing keys to it from symhash the next time we use symhash we get pointers to freed memory.

symhash gets broken after the second performMajorGC() line above.

During checkUnload

added 1 commit

ab65fe27 - Fix use-after-free

Compare with previous version

I fixed a use-after-free which exists in master branch too. Let's see if this will validate now.

Hmm, false alarm, that's not really use-after-free, we have a guard in ocDeinit_ELF for it. I'll revert the commit.

I've added a bunch of sanity checks to the linker code and running the program under valgrind shows that the hash table (symhash) gets corrupted after freeing libHSbase-4.14.0.0.a. I suspect this bug may exist in GHC HEAD too, though I haven't confirmed this yet. I'll try fix that bug, but this also shows another bug: we're not asked by the program to unload libHSbase... so we shouldn't be unloading it even if it's unreachable.

I don't know what's causing the corruption but the second bug should be easy to fix.

The two bugs in my previous comment are closely related: becuase we unload an object that is not asked for unloading explicitly, that is, we don't call unloadObj_ on it, we don't remove its symbols from symhash before freeing it.

Fix is easy (fixing either of the bugs fixes the segfault), though it reveals another segfault ...

added 21 commits

ab65fe27...cf01477f - 3 commits from branch ghc:master
284dec6c - Fix and enable object unloading in GHCi
07b21856 - WIP implementing new plan
9eb6683c - Implement TODOs
aa891c3a - Fix a few bugs
4293bc5f - Fix more bugs
a009fd15 - Fix link field
fe31edd8 - Fix checking THUNK_SELECTOR payload
e213e555 - Free oc stable ptrs in checkUnload_ to allow collecting unloaded ojbs
8c99a189 - Restore 'just purge' behavior
9339fe8a - Fix removeOcSymbols calls
a11ab62b - Fwd declare removeOcSymbols and freeOcStablePtrs
45a96b7a - Update a test pgm
1a78f9ae - Update oc status on objectUnload
fe789190 - Mark object code in GC
16437517 - Remove new tests for now (hard to check output)
0b26d582 - Fix unloading
6c1b749b - Track archives in loaded_objects
f4232cf4 - Fix warns

Compare with previous version

Pushed a few commits -- this should be ready now (hopefully).

If I had a malloc/free implementation that I could LD_PRELOAD while running theses tests that zeroes freed memory that'd easily save me 3-4 hours. Maybe I should implement it myself.

We should also prioritize and fix #18342 (closed) -- it'll cause problems sooner or later.

Submitted a fix: !3538 (closed)

There's a bug in test T16525b: apparently compiler_debugged() does not mean the compiler is linked with debug runtime.

added 1 commit

de050c15 - Fix and enable object unloading in GHCi

Compare with previous version

changed the description

NoFib Results

--------------------------------------------------------------------------------
        Program           Size    Allocs    Instrs     Reads    Writes
--------------------------------------------------------------------------------
             CS          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
            CSD          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
             FS          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
              S          -0.1%      0.0%     +2.1%     +2.2%     +2.6%
             VS          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
            VSD          -0.2%      0.0%     +0.0%     +0.0%     +1.8%
            VSM          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
           anna          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
           ansi          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
           atom          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         awards          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         banner          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
     bernouilli          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
   binary-trees          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
          boyer          -0.1%      0.0%     +0.2%     +0.2%     +0.1%
         boyer2          -0.1%      0.0%     +0.1%     +0.1%     +0.1%
           bspt          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
      cacheprof          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
       calendar          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
       cichelli          -0.1%      0.0%     +0.3%     +0.4%     +0.3%
        circsim          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
       clausify          -0.1%      0.0%     +0.0%     +0.1%     +0.0%
  comp_lab_zift          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
       compress          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
      compress2          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
    constraints          -0.1%      0.0%     +0.0%     +0.1%     +0.0%
   cryptarithm1          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
   cryptarithm2          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
            cse          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
   digits-of-e1          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
   digits-of-e2          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         dom-lt          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
          eliza          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
          event          -0.1%      0.0%     +0.1%     +0.1%     +0.1%
    exact-reals          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         exp3_8          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         expert          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
 fannkuch-redux          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
          fasta          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
            fem          -0.1%      0.0%     +0.0%     +0.1%     +0.0%
            fft          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
           fft2          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
       fibheaps          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
           fish          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
          fluid          -0.1%      0.0%     +0.1%     +0.2%     +0.1%
         fulsom          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         gamteb          -0.1%      0.0%     +0.0%     +0.1%     +0.0%
            gcd          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
    gen_regexps          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         genfft          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
             gg          -0.1%      0.0%     +0.2%     +0.3%     +0.2%
           grep          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         hidden          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
            hpg          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
            ida          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
          infer          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
        integer          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
      integrate          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
   k-nucleotide          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
          kahan          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
        knights          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         lambda          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
     last-piece          -0.1%      0.0%     +0.0%     +0.1%     +0.0%
           lcss          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
           life          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
           lift          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         linear          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
      listcompr          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
       listcopy          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
       maillist          -0.1%      0.0%     +0.0%     +0.1%     +0.0%
         mandel          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
        mandel2          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
           mate          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
        minimax          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
        mkhprog          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
     multiplier          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         n-body          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
       nucleic2          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
           para          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
      paraffins          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         parser          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
        parstof          -0.1%      0.0%     +0.3%     +0.4%     +0.4%
            pic          -0.1%      0.0%     +0.2%     +0.2%     +0.2%
       pidigits          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
          power          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         pretty          -0.1%      0.0%     +1.0%     +1.3%     +1.5%
         primes          -0.1%      0.0%     +0.0%     +0.1%     +0.1%
      primetest          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         prolog          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         puzzle          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         queens          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
        reptile          -0.1%      0.0%     +0.0%     +0.1%     +0.0%
reverse-complem          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
        rewrite          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
           rfib          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
            rsa          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
            scc          -0.1%      0.0%     +1.1%     +1.4%     +1.7%
          sched          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
            scs          -0.1%      0.0%     +0.1%     +0.1%     +0.0%
         simple          -0.1%      0.0%     +0.6%     +0.7%     +0.6%
          solid          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
        sorting          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
  spectral-norm          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         sphere          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
         symalg          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
            tak          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
      transform          -0.1%      0.0%     +0.1%     +0.1%     +0.0%
       treejoin          -0.1%      0.0%     +0.1%     +0.2%     +0.1%
      typecheck          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
        veritas          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
           wang          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
      wave4main          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
   wheel-sieve1          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
   wheel-sieve2          -0.1%      0.0%     +0.0%     +0.1%     +0.0%
           x2n1          -0.1%      0.0%     +0.0%     +0.0%     +0.0%
--------------------------------------------------------------------------------
            Min          -0.2%      0.0%     +0.0%     +0.0%     +0.0%
            Max          -0.1%      0.0%     +2.1%     +2.2%     +2.6%
 Geometric Mean          -0.1%     -0.0%     +0.1%     +0.1%     +0.1%

added 1 commit

cd71cf36 - Fix and enable object unloading in GHCi

Compare with previous version

Another fix for #16525 (object unloading in GHCi)

Closed by Ben Gamari 4 years ago (Aug 9, 2020 8:06pm UTC) 4 years ago

Activity

Another fix for #16525 (object unloading in GHCi)

Merge request reports

Closed by Ben Gamari 4 years ago (Aug 9, 2020 8:06pm UTC) 4 years ago

Activity