[Git][ghc/ghc][wip/ghc-8.10-backports] 2 commits: rts: Refactor foreign export tracking

Ben Gamari gitlab at gitlab.haskell.org
Thu Nov 19 02:48:02 UTC 2020



Ben Gamari pushed to branch wip/ghc-8.10-backports at Glasgow Haskell Compiler / GHC


Commits:
ed57c3a9 by Ben Gamari at 2020-11-18T21:39:41-05:00
rts: Refactor foreign export tracking

This avoids calling `libc` in the initializers which are responsible for
registering foreign exports. We believe this should avoid the corruption
observed in #18548.

See Note [Tracking foreign exports] in rts/ForeignExports.c for an
overview of the new scheme.

(cherry picked from commit c492134912e5270180881b7345ee86dc32756bdd)

- - - - -
65be3832 by Ben Gamari at 2020-11-18T21:39:53-05:00
rts: Refactor unloading of foreign export StablePtrs

Previously we would allocate a linked list cell for each foreign export.
Now we can avoid this by taking advantage of the fact that they are
already broken into groups.

(cherry picked from commit 40dc91069d15bfc1d81f1722b39e06cac8fdddd1)

- - - - -


10 changed files:

- compiler/deSugar/DsForeign.hs
- includes/Rts.h
- + includes/rts/ForeignExports.h
- + rts/ForeignExports.c
- + rts/ForeignExports.h
- rts/Linker.c
- rts/LinkerInternals.h
- rts/RtsStartup.c
- rts/RtsSymbols.c
- rts/rts.cabal.in


Changes:

=====================================
compiler/deSugar/DsForeign.hs
=====================================
@@ -86,15 +86,16 @@ dsForeigns' :: [LForeignDecl GhcTc]
 dsForeigns' []
   = return (NoStubs, nilOL)
 dsForeigns' fos = do
+    mod <- getModule
     fives <- mapM do_ldecl fos
     let
         (hs, cs, idss, bindss) = unzip4 fives
         fe_ids = concat idss
-        fe_init_code = map foreignExportInitialiser fe_ids
+        fe_init_code = foreignExportsInitialiser mod fe_ids
     --
     return (ForeignStubs
              (vcat hs)
-             (vcat cs $$ vcat fe_init_code),
+             (vcat cs $$ fe_init_code),
             foldr (appOL . toOL) nilOL bindss)
   where
    do_ldecl (dL->L loc decl) = putSrcSpanDs loc (do_decl decl)
@@ -667,8 +668,8 @@ mkFExportCBits dflags c_nm maybe_target arg_htys res_hty is_IO_res_ty cc
      ]
 
 
-foreignExportInitialiser :: Id -> SDoc
-foreignExportInitialiser hs_fn =
+foreignExportsInitialiser :: Module -> [Id] -> SDoc
+foreignExportsInitialiser mod hs_fns =
    -- Initialise foreign exports by registering a stable pointer from an
    -- __attribute__((constructor)) function.
    -- The alternative is to do this from stginit functions generated in
@@ -677,14 +678,24 @@ foreignExportInitialiser hs_fn =
    -- all modules that are imported directly or indirectly are actually used by
    -- the program.
    -- (this is bad for big umbrella modules like Graphics.Rendering.OpenGL)
+   --
+   -- See Note [Tracking foreign exports] in rts/ForeignExports.c
    vcat
-    [ text "static void stginit_export_" <> ppr hs_fn
-         <> text "() __attribute__((constructor));"
-    , text "static void stginit_export_" <> ppr hs_fn <> text "()"
-    , braces (text "foreignExportStablePtr"
-       <> parens (text "(StgPtr) &" <> ppr hs_fn <> text "_closure")
-       <> semi)
+    [ text "static struct ForeignExportsList" <+> list_symbol <+> equals
+         <+> braces (text ".exports = " <+> export_list) <> semi
+    , text "static void " <> ctor_symbol <> text "(void)"
+         <+> text " __attribute__((constructor));"
+    , text "static void " <> ctor_symbol <> text "()"
+    , braces (text "registerForeignExports" <> parens (char '&' <> list_symbol) <> semi)
     ]
+  where
+    mod_str = pprModuleName (moduleName mod)
+    ctor_symbol = text "stginit_export_" <> mod_str
+    list_symbol = text "stg_exports_" <> mod_str
+    export_list = braces $ pprWithCommas closure_ptr hs_fns
+
+    closure_ptr :: Id -> SDoc
+    closure_ptr fn = text "(StgPtr) &" <> ppr fn <> text "_closure"
 
 
 mkHObj :: Type -> SDoc


=====================================
includes/Rts.h
=====================================
@@ -209,6 +209,9 @@ void _assertFail(const char *filename, unsigned int linenum)
 #include "rts/storage/GC.h"
 #include "rts/NonMoving.h"
 
+/* Foreign exports */
+#include "rts/ForeignExports.h"
+
 /* Other RTS external APIs */
 #include "rts/Parallel.h"
 #include "rts/Signals.h"


=====================================
includes/rts/ForeignExports.h
=====================================
@@ -0,0 +1,38 @@
+/* -----------------------------------------------------------------------------
+ *
+ * (c) The GHC Team, 1995-2009
+ *
+ * Interface to the RTS's foreign export tracking code.
+ *
+ * Do not #include this file directly: #include "Rts.h" instead.
+ *
+ * To understand the structure of the RTS headers, see the wiki:
+ *   https://gitlab.haskell.org/ghc/ghc/wikis/commentary/source-tree/includes
+ *
+ * ---------------------------------------------------------------------------*/
+
+#pragma once
+
+struct _ObjectCode;
+
+/* N.B. See Note [Tracking foreign exports] in
+ * rts/ForeignExports.c. */
+struct ForeignExportsList {
+      /* a link field for linking these together into lists.
+       */
+    struct ForeignExportsList *next;
+      /* the length of ->exports */
+    int n_entries;
+      /* if the RTS linker loaded the module,
+       * to which ObjectCode these exports belong. */
+    struct _ObjectCode *oc;
+      /* if the RTS linker loaded the module,
+       * this points to an array of length ->n_entries
+       * recording the StablePtr for each export. */
+    StgStablePtr **stable_ptrs;
+      /* the exported closures. of length ->exports. */
+    StgPtr exports[];
+};
+
+void registerForeignExports(struct ForeignExportsList *exports);
+


=====================================
rts/ForeignExports.c
=====================================
@@ -0,0 +1,130 @@
+/* -----------------------------------------------------------------------------
+ *
+ * (c) The GHC Team 2020
+ *
+ * Management of foreign exports.
+ *
+ * ---------------------------------------------------------------------------*/
+
+#include "Rts.h"
+#include "RtsUtils.h"
+#include "ForeignExports.h"
+
+/* protected by linker_mutex after start-up */
+static struct ForeignExportsList *pending = NULL;
+static ObjectCode *loading_obj = NULL;
+
+/*
+ * Note [Tracking foreign exports]
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * Foreign exports are garbage collection roots. That is, things (e.g. CAFs)
+ * depended upon by a module's `foreign export`s need to be kept alive for as
+ * long an module is loaded. To ensure this we create a stable pointer to each
+ * `foreign export`'d closure. This works as follows:
+ *
+ * 1. The compiler  (namely GHC.HsToCore.Foreign.Decl.foreignExports)
+ *    inserts a C-stub into each module containing a `foreign export`. This
+ *    stub contains two things:
+ *
+ *    - A `ForeignExportsList` listing all of the exported closures, and
+ *
+ *    - An initializer which calls `registerForeignExports` with a reference to
+ *      the `ForeignExportsList`.
+ *
+ * 2. When the module's object code is loaded, its initializer is called by the
+ *    linker (this might be the system's dynamic linker or GHC's own static
+ *    linker). `registerForeignExports` then places the module's
+ *    `ForeignExportsList` on `pending` list.
+ *
+ * 3. When loading has finished (e.g. during RTS initialization or at the end
+ *    of `Linker.c:ocTryLoad`) `processForeignExports` is called. Here we
+ *    traverse the `pending` list and create a `StablePtr` for each export
+ *    therein.
+ *
+ * The reason for this two-step process is that we are very restricted in what
+ * we can do in an initializer function. For instance, we cannot necessarily
+ * call `malloc`  since the `libc`'s own initializer may not have run yet.
+ * For instance, doing exactly this resulted in #18548.
+ *
+ * Another consideration here is that the linker needs to know which
+ * `StablePtr`s belong to each `ObjectCode` so it can free them when the module is
+ * unloaded.  For this reason, the linker informs us when it is loading an
+ * object by calling `foreignExportsLoadingObject` and
+ * `foreignExportsFinishedLoadingObject`. We take note of the `ObjectCode*` we
+ * are loading in `loading_obj` such that we can associate the `ForeignExportsList` with
+ * the `ObjectCode` in `processForeignExports`. We then record each of the
+ * StablePtrs we create in the ->stable_ptrs array of ForeignExportsList so
+ * they can be enumerated during unloading.
+ *
+ */
+
+void registerForeignExports(struct ForeignExportsList *exports)
+{
+    ASSERT(exports->next == NULL);
+    ASSERT(exports->oc == NULL);
+    exports->next = pending;
+    exports->oc = loading_obj;
+    pending = exports;
+}
+
+/* -----------------------------------------------------------------------------
+   Create a StablePtr for a foreign export.  This is normally called by
+   a C function with __attribute__((constructor)), which is generated
+   by GHC and linked into the module.
+
+   If the object code is being loaded dynamically, then we remember
+   which StablePtrs were allocated by the constructors and free them
+   again in unloadObj().
+   -------------------------------------------------------------------------- */
+
+void foreignExportsLoadingObject(ObjectCode *oc)
+{
+    ASSERT(loading_obj == NULL);
+    loading_obj = oc;
+}
+
+void foreignExportsFinishedLoadingObject()
+{
+    ASSERT(loading_obj != NULL);
+    loading_obj = NULL;
+    processForeignExports();
+}
+
+/* Caller must own linker_mutex so that we can safely modify
+ * oc->stable_ptrs. */
+void processForeignExports()
+{
+    while (pending) {
+        struct ForeignExportsList *cur = pending;
+        pending = cur->next;
+
+        /* sanity check */
+        ASSERT(cur->stable_ptrs == NULL);
+
+        /* N.B. We only need to populate the ->stable_ptrs
+         * array if the object might later be unloaded.
+         */
+        if (cur->oc != NULL) {
+            cur->stable_ptrs =
+              stgMallocBytes(sizeof(StgStablePtr*) * cur->n_entries,
+                             "foreignExportStablePtr");
+
+            for (int i=0; i < cur->n_entries; i++) {
+                StgStablePtr *sptr = getStablePtr(cur->exports[i]);
+
+                if (cur->oc != NULL) {
+                    cur->stable_ptrs[i] = sptr;
+                }
+            }
+            cur->next = cur->oc->foreign_exports;
+            cur->oc->foreign_exports = cur;
+        } else {
+            /* can't be unloaded, don't bother populating
+             * ->stable_ptrs array. */
+            for (int i=0; i < cur->n_entries; i++) {
+                getStablePtr(cur->exports[i]);
+            }
+        }
+    }
+}


=====================================
rts/ForeignExports.h
=====================================
@@ -0,0 +1,21 @@
+/* -----------------------------------------------------------------------------
+ *
+ * (c) The GHC Team 2020
+ *
+ * Management of foreign exports.
+ *
+ * ---------------------------------------------------------------------------*/
+
+#pragma once
+
+#include "Rts.h"
+#include "LinkerInternals.h"
+
+#include "BeginPrivate.h"
+
+void foreignExportsLoadingObject(ObjectCode *oc);
+void foreignExportsFinishedLoadingObject(void);
+void processForeignExports(void);
+
+#include "EndPrivate.h"
+


=====================================
rts/Linker.c
=====================================
@@ -26,6 +26,7 @@
 #include "RtsSymbols.h"
 #include "RtsSymbolInfo.h"
 #include "Profiling.h"
+#include "ForeignExports.h"
 #include "sm/OSMem.h"
 #include "linker/M32Alloc.h"
 #include "linker/CacheFlush.h"
@@ -951,37 +952,6 @@ SymbolAddr* lookupSymbol( SymbolName* lbl )
     return r;
 }
 
-/* -----------------------------------------------------------------------------
-   Create a StablePtr for a foreign export.  This is normally called by
-   a C function with __attribute__((constructor)), which is generated
-   by GHC and linked into the module.
-
-   If the object code is being loaded dynamically, then we remember
-   which StablePtrs were allocated by the constructors and free them
-   again in unloadObj().
-   -------------------------------------------------------------------------- */
-
-static ObjectCode *loading_obj = NULL;
-
-StgStablePtr foreignExportStablePtr (StgPtr p)
-{
-    ForeignExportStablePtr *fe_sptr;
-    StgStablePtr *sptr;
-
-    sptr = getStablePtr(p);
-
-    if (loading_obj != NULL) {
-        fe_sptr = stgMallocBytes(sizeof(ForeignExportStablePtr),
-                                 "foreignExportStablePtr");
-        fe_sptr->stable_ptr = sptr;
-        fe_sptr->next = loading_obj->stable_ptrs;
-        loading_obj->stable_ptrs = fe_sptr;
-    }
-
-    return sptr;
-}
-
-
 /* -----------------------------------------------------------------------------
  * Debugging aid: look in GHCi's object symbol tables for symbols
  * within DELTA bytes of the specified address, and show their names.
@@ -1252,14 +1222,18 @@ static void freeOcStablePtrs (ObjectCode *oc)
 {
     // Release any StablePtrs that were created when this
     // object module was initialized.
-    ForeignExportStablePtr *fe_ptr, *next;
+    struct ForeignExportsList *exports, *next;
 
-    for (fe_ptr = oc->stable_ptrs; fe_ptr != NULL; fe_ptr = next) {
-        next = fe_ptr->next;
-        freeStablePtr(fe_ptr->stable_ptr);
-        stgFree(fe_ptr);
+    for (exports = oc->foreign_exports; exports != NULL; exports = next) {
+        next = exports->next;
+        for (int i = 0; i < exports->n_entries; i++) {
+            freeStablePtr(exports->stable_ptrs[i]);
+        }
+        stgFree(exports->stable_ptrs);
+        exports->stable_ptrs = NULL;
+        exports->next = NULL;
     }
-    oc->stable_ptrs = NULL;
+    oc->foreign_exports = NULL;
 }
 
 static void
@@ -1416,7 +1390,7 @@ mkOc( pathchar *path, char *image, int imageSize,
    oc->n_segments        = 0;
    oc->segments          = NULL;
    oc->proddables        = NULL;
-   oc->stable_ptrs       = NULL;
+   oc->foreign_exports   = NULL;
 #if defined(NEED_SYMBOL_EXTRAS)
    oc->symbol_extras     = NULL;
 #endif
@@ -1777,7 +1751,8 @@ int ocTryLoad (ObjectCode* oc) {
 
     IF_DEBUG(linker, debugBelch("ocTryLoad: ocRunInit start\n"));
 
-    loading_obj = oc; // tells foreignExportStablePtr what to do
+    // See Note [Tracking foreign exports] in ForeignExports.c
+    foreignExportsLoadingObject(oc);
 #if defined(OBJFORMAT_ELF)
     r = ocRunInit_ELF ( oc );
 #elif defined(OBJFORMAT_PEi386)
@@ -1787,7 +1762,7 @@ int ocTryLoad (ObjectCode* oc) {
 #else
     barf("ocTryLoad: initializers not implemented on this platform");
 #endif
-    loading_obj = NULL;
+    foreignExportsFinishedLoadingObject();
 
     if (!r) { return r; }
 


=====================================
rts/LinkerInternals.h
=====================================
@@ -135,17 +135,6 @@ typedef struct _Segment {
     int n_sections;
 } Segment;
 
-/*
- * We must keep track of the StablePtrs that are created for foreign
- * exports by constructor functions when the module is loaded, so that
- * we can free them again when the module is unloaded.  If we don't do
- * this, then the StablePtr will keep the module alive indefinitely.
- */
-typedef struct ForeignExportStablePtr_ {
-    StgStablePtr stable_ptr;
-    struct ForeignExportStablePtr_ *next;
-} ForeignExportStablePtr;
-
 #if defined(powerpc_HOST_ARCH) || defined(x86_64_HOST_ARCH)
 #define NEED_SYMBOL_EXTRAS 1
 #endif
@@ -240,7 +229,8 @@ typedef struct _ObjectCode {
     char* bssBegin;
     char* bssEnd;
 
-    ForeignExportStablePtr *stable_ptrs;
+    /* a list of all ForeignExportsLists owned by this object */
+    struct ForeignExportsList *foreign_exports;
 
     /* Holds the list of symbols in the .o file which
        require extra information.*/


=====================================
rts/RtsStartup.c
=====================================
@@ -20,6 +20,7 @@
 #include "STM.h"        /* initSTM */
 #include "RtsSignals.h"
 #include "Weak.h"
+#include "ForeignExports.h"     /* processForeignExports */
 #include "Ticky.h"
 #include "StgRun.h"
 #include "Prelude.h"            /* fixupRTStoPreludeRefs */
@@ -287,7 +288,13 @@ hs_init_ghc(int *argc, char **argv[], RtsConfig rts_config)
     getStablePtr((StgPtr)runHandlersPtr_closure);
 #endif
 
-    // Initialize the top-level handler system
+    /*
+     * process any foreign exports which were registered while loading the
+     * image
+     * */
+    processForeignExports();
+
+    /* initialize the top-level handler system */
     initTopHandler();
 
     /* initialise the shared Typeable store */


=====================================
rts/RtsSymbols.c
=====================================
@@ -643,7 +643,7 @@
       SymI_HasProto(freeFullProgArgv)                                   \
       SymI_HasProto(getProcessElapsedTime)                              \
       SymI_HasProto(getStablePtr)                                       \
-      SymI_HasProto(foreignExportStablePtr)                             \
+      SymI_HasProto(registerForeignExports)                             \
       SymI_HasProto(hs_init)                                            \
       SymI_HasProto(hs_init_with_rtsopts)                               \
       SymI_HasProto(hs_init_ghc)                                        \


=====================================
rts/rts.cabal.in
=====================================
@@ -131,6 +131,7 @@ library
                       rts/EventLogWriter.h
                       rts/FileLock.h
                       rts/Flags.h
+                      rts/ForeignExports.h
                       rts/GetTime.h
                       rts/Globals.h
                       rts/Hpc.h
@@ -382,6 +383,7 @@ library
                ClosureFlags.c
                Disassembler.c
                FileLock.c
+               ForeignExports.c
                Globals.c
                Hash.c
                Heap.c



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/410b43a29576c40f4a65c9558f080a97c194636c...65be3832f3aa48bbde896ee846c18fcba1f16b42

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/410b43a29576c40f4a65c9558f080a97c194636c...65be3832f3aa48bbde896ee846c18fcba1f16b42
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20201118/9fd62c99/attachment-0001.html>


More information about the ghc-commits mailing list