[Git][ghc/ghc][wip/sym-type] rts/linker: Clearly define SymType

Ben Gamari (@bgamari) gitlab at gitlab.haskell.org
Fri Dec 8 21:30:28 UTC 2023



Ben Gamari pushed to branch wip/sym-type at Glasgow Haskell Compiler / GHC


Commits:
ea9a0ae4 by Ben Gamari at 2023-12-08T16:30:02-05:00
rts/linker: Clearly define SymType

Previously SymType was both an enumeration of three symbol types *and*
an orthogonal flag (`DUP_DISCARD`, introduced in !9475). This was quite
fragile as it meant that to extract the symbol type one had to careful
mask out the flag. Naturally this wasn't done consistently.

Fix this by renaming the field to `flags` and adding an accessor.

Fixes #24117.

- - - - -


5 changed files:

- rts/Linker.c
- rts/LinkerInternals.h
- rts/linker/Elf.c
- rts/linker/MachO.c
- rts/linker/PEi386.c


Changes:

=====================================
rts/Linker.c
=====================================
@@ -226,7 +226,7 @@ static void ghciRemoveSymbolTable(StrHashTable *table, const SymbolName* key,
 static const char *
 symbolTypeString (SymType type)
 {
-    switch (type & ~SYM_TYPE_DUP_DISCARD) {
+    switch (type) {
         case SYM_TYPE_CODE: return "code";
         case SYM_TYPE_DATA: return "data";
         case SYM_TYPE_INDIRECT_DATA: return "indirect-data";
@@ -262,6 +262,7 @@ int ghciInsertSymbolTable(
    SymbolAddr* data,
    SymStrength strength,
    SymType type,
+   SymDuplicable duplicable,
    ObjectCode *owner)
 {
    RtsSymbolInfo *pinfo = lookupStrHashTable(table, key);
@@ -272,13 +273,14 @@ int ghciInsertSymbolTable(
       pinfo->owner = owner;
       pinfo->strength = strength;
       pinfo->type = type;
+      pinfo->duplicable = duplicable;
       insertStrHashTable(table, key, pinfo);
       return 1;
    }
-   else if (pinfo->type ^ type)
+   else if (pinfo->type != type)
    {
        /* We were asked to discard the symbol on duplicates, do so quietly.  */
-       if (!(type & SYM_TYPE_DUP_DISCARD))
+       if (duplicable != SYM_DISCARD_ON_DUPLICATE)
        {
          debugBelch("Symbol type mismatch.\n");
          debugBelch("Symbol %s was defined by %" PATH_FMT " to be a %s symbol.\n",
@@ -466,7 +468,7 @@ initLinker_ (int retain_cafs)
     for (const RtsSymbolVal *sym = rtsSyms; sym->lbl != NULL; sym++) {
         if (! ghciInsertSymbolTable(WSTR("(GHCi built-in symbols)"),
                                     symhash, sym->lbl, sym->addr,
-                                    sym->strength, sym->type, NULL)) {
+                                    sym->strength, sym->type, SYM_ERROR_ON_DUPLICATE, NULL)) {
             barf("ghciInsertSymbolTable failed");
         }
         IF_DEBUG(linker, debugBelch("initLinker: inserting rts symbol %s, %p\n", sym->lbl, sym->addr));
@@ -476,7 +478,7 @@ initLinker_ (int retain_cafs)
     if (! ghciInsertSymbolTable(WSTR("(GHCi built-in symbols)"), symhash,
                                 MAYBE_LEADING_UNDERSCORE_STR("newCAF"),
                                 retain_cafs ? newRetainedCAF : newGCdCAF,
-                                HS_BOOL_FALSE, SYM_TYPE_CODE, NULL)) {
+                                HS_BOOL_FALSE, SYM_TYPE_CODE, SYM_ERROR_ON_DUPLICATE, NULL)) {
         barf("ghciInsertSymbolTable failed");
     }
 
@@ -864,7 +866,7 @@ HsBool removeLibrarySearchPath(HsPtr dll_path_index)
 HsInt insertSymbol(pathchar* obj_name, SymbolName* key, SymbolAddr* data)
 {
     return ghciInsertSymbolTable(obj_name, symhash, key, data, HS_BOOL_FALSE,
-                                 SYM_TYPE_CODE, NULL);
+                                 SYM_TYPE_CODE, SYM_ERROR_ON_DUPLICATE, NULL);
 }
 
 /* -----------------------------------------------------------------------------
@@ -1696,7 +1698,7 @@ int ocTryLoad (ObjectCode* oc) {
             && !ghciInsertSymbolTable(oc->fileName, symhash, symbol.name,
                                       symbol.addr,
                                       isSymbolWeak(oc, symbol.name),
-                                      symbol.type, oc)) {
+                                      symbol.type, SYM_ERROR_ON_DUPLICATE, oc)) {
             return 0;
         }
     }


=====================================
rts/LinkerInternals.h
=====================================
@@ -55,17 +55,24 @@ typedef struct _Section    Section;
 
 /* What kind of thing a symbol identifies. We need to know this to determine how
  * to process overflowing relocations. See Note [Processing overflowed relocations].
- * This is bitfield however only the option SYM_TYPE_DUP_DISCARD can be combined
- * with the other values. */
+ * Be sure to update the width of RtsSymbolInfo.type if you add variants
+ * to this enumeration.
+ */
 typedef enum _SymType {
     SYM_TYPE_CODE = 1 << 0, /* the symbol is a function and can be relocated via a jump island */
     SYM_TYPE_DATA = 1 << 1, /* the symbol is data */
     SYM_TYPE_INDIRECT_DATA = 1 << 2, /* see Note [_iob_func symbol] */
-    SYM_TYPE_DUP_DISCARD = 1 << 3, /* the symbol is a symbol in a BFD import library
-                                      however if a duplicate is found with a mismatching
-                                      SymType then discard this one.  */
 } SymType;
 
+/* How to handle duplicate symbols. */
+typedef enum {
+    // Throw an error if a duplicate symbol of different SymType is found
+    SYM_ERROR_ON_DUPLICATE = 0,
+    // Discard if a duplicate symbol of different SymType is found. This
+    // is necessary on PE platforms for symbols defined in BFD import
+    // libraries.
+    SYM_DISCARD_ON_DUPLICATE = 1
+} SymDuplicable;
 
 #if defined(OBJFORMAT_ELF)
 #  include "linker/ElfTypes.h"
@@ -438,7 +445,8 @@ typedef struct _RtsSymbolInfo {
     SymbolAddr* value;
     ObjectCode *owner;
     SymStrength strength;
-    SymType type;
+    SymType type: 16;
+    SymDuplicable duplicable: 1;
 } RtsSymbolInfo;
 
 #include "BeginPrivate.h"
@@ -466,6 +474,7 @@ int ghciInsertSymbolTable(
     SymbolAddr* data,
     SymStrength weak,
     SymType type,
+    SymDuplicable duplicable,
     ObjectCode *owner);
 
 /* Lock-free version of lookupSymbol. When 'dependent' is not NULL, adds it as a


=====================================
rts/linker/Elf.c
=====================================
@@ -1083,7 +1083,7 @@ ocGetNames_ELF ( ObjectCode* oc )
                            setWeakSymbol(oc, nm);
                        }
                        if (!ghciInsertSymbolTable(oc->fileName, symhash,
-                                                  nm, symbol->addr, isWeak, sym_type, oc)
+                                                  nm, symbol->addr, isWeak, sym_type, SYM_ERROR_ON_DUPLICATE, oc)
                            ) {
                            goto fail;
                        }


=====================================
rts/linker/MachO.c
=====================================
@@ -1390,14 +1390,15 @@ ocGetNames_MachO(ObjectCode* oc)
                     {
                             IF_DEBUG(linker_verbose, debugBelch("ocGetNames_MachO: inserting %s\n", nm));
                             SymbolAddr* addr = oc->info->macho_symbols[i].addr;
-                            // TODO: Make figure out how to determine this from the object file
-                            SymType sym_type = SYM_TYPE_CODE;
+                            // TODO: Figure out how to determine this from the object file
+                            const SymType sym_type = SYM_TYPE_CODE;
                             ghciInsertSymbolTable( oc->fileName
                                                  , symhash
                                                  , nm
                                                  , addr
                                                  , HS_BOOL_FALSE
                                                  , sym_type
+                                                 , SYM_ERROR_ON_DUPLICATE
                                                  , oc);
 
                             oc->symbols[curSymbol].name = nm;
@@ -1440,7 +1441,7 @@ ocGetNames_MachO(ObjectCode* oc)
 
                 IF_DEBUG(linker_verbose, debugBelch("ocGetNames_MachO: inserting common symbol: %s\n", nm));
                 ghciInsertSymbolTable(oc->fileName, symhash, nm,
-                                       (void*)commonCounter, HS_BOOL_FALSE, sym_type, oc);
+                                       (void*)commonCounter, HS_BOOL_FALSE, sym_type, SYM_ERROR_ON_DUPLICATE, oc);
                 oc->symbols[curSymbol].name = nm;
                 oc->symbols[curSymbol].addr = oc->info->macho_symbols[i].addr;
                 curSymbol++;


=====================================
rts/linker/PEi386.c
=====================================
@@ -299,7 +299,7 @@
    These two issues mean that for GHC we need to take a different approach
    to handling import libraries.  For normal C libraries we have proper
    differentiation between CODE and DATA.   For GHC produced import libraries
-   we do not.   As such the SYM_TYPE_DUP_DISCARD tells the linker that if a
+   we do not.  As such the dup_discard flag tells the linker that if a
    duplicate symbol is found, and we were going to discard it anyway, just do
    so quitely.  This works because the RTS symbols themselves are provided by
    the currently loaded RTS as built-in symbols.
@@ -438,7 +438,7 @@ void initLinker_PEi386(void)
     if (!ghciInsertSymbolTable(WSTR("(GHCi/Ld special symbols)"),
                                symhash, "__image_base__",
                                GetModuleHandleW (NULL), HS_BOOL_TRUE,
-                               SYM_TYPE_CODE, NULL)) {
+                               SYM_TYPE_CODE, SYM_ERROR_ON_DUPLICATE, NULL)) {
         barf("ghciInsertSymbolTable failed");
     }
 
@@ -1814,9 +1814,8 @@ ocGetNames_PEi386 ( ObjectCode* oc )
           sname = strdup (sname);
           addr  = strdup (addr);
           type = has_code_section ? SYM_TYPE_CODE : SYM_TYPE_DATA;
-          type |= SYM_TYPE_DUP_DISCARD;
           if (!ghciInsertSymbolTable(oc->fileName, symhash, sname,
-                                     addr, false, type, oc)) {
+                                     addr, false, type, SYM_DISCARD_ON_DUPLICATE, oc)) {
              releaseOcInfo (oc);
              stgFree (oc->image);
              oc->image = NULL;
@@ -1895,7 +1894,7 @@ ocGetNames_PEi386 ( ObjectCode* oc )
           stgFree(tmp);
           sname = strdup (sname);
           if (!ghciInsertSymbolTable(oc->fileName, symhash, sname,
-                                     addr, false, type, oc))
+                                     addr, false, type, SYM_ERROR_ON_DUPLICATE, oc))
                return false;
 
           break;
@@ -1918,7 +1917,7 @@ ocGetNames_PEi386 ( ObjectCode* oc )
          }
 
          if (! ghciInsertSymbolTable(oc->fileName, symhash, sname, addr,
-                                     isWeak, type, oc))
+                                     isWeak, type, SYM_ERROR_ON_DUPLICATE, oc))
              return false;
       } else {
           /* We're skipping the symbol, but if we ever load this
@@ -2324,7 +2323,9 @@ SymbolAddr *lookupSymbol_PEi386(SymbolName *lbl, ObjectCode *dependent, SymType
         sym = lookupSymbolInDLLs(lbl, dependent);
         return sym; // might be NULL if not found
     } else {
-        if (type) *type = pinfo->type;
+        if (type) {
+            *type = pinfo->type;
+        }
 
         if (pinfo && pinfo->owner && isSymbolImport (pinfo->owner, lbl))
         {



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/ea9a0ae4628cf9d112cf4b32ea95930be7a371f6

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/ea9a0ae4628cf9d112cf4b32ea95930be7a371f6
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/20231208/cc47b3ab/attachment-0001.html>


More information about the ghc-commits mailing list