[Git][ghc/ghc][wip/no-assert] rts/linker: Replace some ASSERTs with CHECK

Ben Gamari gitlab at gitlab.haskell.org
Mon Nov 23 23:13:44 UTC 2020



Ben Gamari pushed to branch wip/no-assert at Glasgow Haskell Compiler / GHC


Commits:
ed9d5a1e by Ben Gamari at 2020-11-23T18:12:15-05:00
rts/linker: Replace some ASSERTs with CHECK

In the past some people have confused ASSERT, which is for checking
internal invariants, which CHECK, which should be used when checking
things that might fail due to bad input (and therefore should be enabled
even in the release compiler). Change some of these cases in the linker
to use CHECK.

- - - - -


5 changed files:

- rts/Linker.c
- rts/linker/Elf.c
- rts/linker/MachO.c
- rts/linker/PEi386.c
- rts/linker/elf_got.c


Changes:

=====================================
rts/Linker.c
=====================================
@@ -49,7 +49,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <stdio.h>
-#include <assert.h>
 #include <fs_rts.h>
 
 #if defined(HAVE_SYS_STAT_H)
@@ -885,12 +884,11 @@ SymbolAddr* lookupDependentSymbol (SymbolName* lbl, ObjectCode *dependent)
         */
         IF_DEBUG(linker, debugBelch("lookupSymbol: looking up %s with dlsym\n",
                                     lbl));
-        ASSERT(lbl[0] == '_');
+        CHECK(lbl[0] == '_');
         return internal_dlsym(lbl + 1);
 
 #       else
-        ASSERT(false);
-        return NULL;
+#       error No OBJFORMAT_* macro set
 #       endif
     } else {
         if (dependent) {
@@ -2112,7 +2110,7 @@ HsInt unloadNativeObj (void *handle)
             n_unloaded_objects += 1;
 
             // dynamic objects have no symbols
-            ASSERT(nc->symbols == NULL);
+            CHECK(nc->symbols == NULL);
             freeOcStablePtrs(nc);
 
             // Remove object code from root set


=====================================
rts/linker/Elf.c
=====================================
@@ -416,7 +416,7 @@ ocVerifyImage_ELF ( ObjectCode* oc )
              "\nSection header table: start %ld, n_entries %d, ent_size %d\n",
              (long)ehdr->e_shoff, shnum, ehdr->e_shentsize  ));
 
-   ASSERT(ehdr->e_shentsize == sizeof(Elf_Shdr));
+   CHECK(ehdr->e_shentsize == sizeof(Elf_Shdr));
 
    shdr = (Elf_Shdr*) (ehdrC + ehdr->e_shoff);
 
@@ -537,7 +537,7 @@ ocVerifyImage_ELF ( ObjectCode* oc )
 #if defined(SHN_XINDEX)
          /* See Note [Many ELF Sections] */
          if (secno == SHN_XINDEX) {
-            ASSERT(shndxTable);
+            CHECK(shndxTable);
             secno = shndxTable[j];
          }
 #endif
@@ -859,7 +859,7 @@ ocGetNames_ELF ( ObjectCode* oc )
                             PROT_READ | PROT_WRITE,
                             MAP_ANON | MAP_PRIVATE,
                             -1, 0);
-          ASSERT(common_mem != NULL);
+          CHECK(common_mem != NULL);
       }
 
       //TODO: we ignore local symbols anyway right? So we can use the
@@ -888,7 +888,7 @@ ocGetNames_ELF ( ObjectCode* oc )
                secno = shndx;
 #if defined(SHN_XINDEX)
                if (shndx == SHN_XINDEX) {
-                  ASSERT(shndxTable);
+                  CHECK(shndxTable);
                   secno = shndxTable[j];
                }
 #endif
@@ -897,11 +897,11 @@ ocGetNames_ELF ( ObjectCode* oc )
 
                if (shndx == SHN_COMMON) {
                    isLocal = false;
-                   ASSERT(common_used < common_size);
-                   ASSERT(common_mem);
+                   CHECK(common_used < common_size);
+                   CHECK(common_mem);
                    symbol->addr = (void*)((uintptr_t)common_mem + common_used);
                    common_used += symbol->elf_sym->st_size;
-                   ASSERT(common_used <= common_size);
+                   CHECK(common_used <= common_size);
 
                    IF_DEBUG(linker,
                             debugBelch("COMMON symbol, size %ld name %s allocated at %p\n",
@@ -930,7 +930,7 @@ ocGetNames_ELF ( ObjectCode* oc )
                           )
                        ) {
                    /* Section 0 is the undefined section, hence > and not >=. */
-                   ASSERT(secno > 0 && secno < shnum);
+                   CHECK(secno > 0 && secno < shnum);
                    /*
                    if (shdr[secno].sh_type == SHT_NOBITS) {
                       debugBelch("   BSS symbol, size %d off %d name %s\n",
@@ -957,7 +957,7 @@ ocGetNames_ELF ( ObjectCode* oc )
                /* And the decision is ... */
 
                if (symbol->addr != NULL) {
-                   ASSERT(nm != NULL);
+                   CHECK(nm != NULL);
                    /* Acquire! */
                    if (!isLocal) {
 
@@ -1040,7 +1040,7 @@ do_Elf_Rel_relocations ( ObjectCode* oc, char* ehdrC,
            break;
        }
    }
-   ASSERT(stab != NULL);
+   CHECK(stab != NULL);
 
    targ  = (Elf_Word*)oc->sections[target_shndx].start;
    IF_DEBUG(linker,debugBelch(
@@ -1246,7 +1246,7 @@ do_Elf_Rel_relocations ( ObjectCode* oc, char* ehdrC,
                result = ((S + A) | T) - P;
                result &= ~1; // Clear thumb indicator bit
 
-               ASSERT(isInt(26, result)); /* X in range */
+               CHECK(isInt(26, result)); /* X in range */
            }
 
            // Update the branch target
@@ -1421,7 +1421,7 @@ do_Elf_Rel_relocations ( ObjectCode* oc, char* ehdrC,
        case COMPAT_R_ARM_GOT_PREL: {
               int32_t A = *pP;
               void* GOT_S = symbol->got_addr;
-              ASSERT(GOT_S);
+              CHECK(GOT_S);
               *(uint32_t *)P = (uint32_t) GOT_S + A - P;
               break;
        }
@@ -1547,21 +1547,21 @@ do_Elf_Rela_relocations ( ObjectCode* oc, char* ehdrC,
          case R_SPARC_WDISP30:
             w1 = *pP & 0xC0000000;
             w2 = (Elf_Word)((value - P) >> 2);
-            ASSERT((w2 & 0xC0000000) == 0);
+            CHECK((w2 & 0xC0000000) == 0);
             w1 |= w2;
             *pP = w1;
             break;
          case R_SPARC_HI22:
             w1 = *pP & 0xFFC00000;
             w2 = (Elf_Word)(value >> 10);
-            ASSERT((w2 & 0xFFC00000) == 0);
+            CHECK((w2 & 0xFFC00000) == 0);
             w1 |= w2;
             *pP = w1;
             break;
          case R_SPARC_LO10:
             w1 = *pP & ~0x3FF;
             w2 = (Elf_Word)(value & 0x3FF);
-            ASSERT((w2 & ~0x3FF) == 0);
+            CHECK((w2 & ~0x3FF) == 0);
             w1 |= w2;
             *pP = w1;
             break;
@@ -1861,12 +1861,12 @@ ocResolve_ELF ( ObjectCode* oc )
                 Elf_Word secno = symbol->elf_sym->st_shndx;
 #if defined(SHN_XINDEX)
                 if (secno == SHN_XINDEX) {
-                    ASSERT(shndxTable);
+                    CHECK(shndxTable);
                     secno = shndxTable[i];
                 }
 #endif
-                ASSERT(symbol->elf_sym->st_name == 0);
-                ASSERT(symbol->elf_sym->st_value == 0);
+                CHECK(symbol->elf_sym->st_name == 0);
+                CHECK(symbol->elf_sym->st_value == 0);
                 symbol->addr = oc->sections[ secno ].start;
             }
         }


=====================================
rts/linker/MachO.c
=====================================
@@ -252,7 +252,6 @@ resolveImports(
                        "%s: unknown symbol `%s'", oc->fileName, symbol->name);
             return 0;
         }
-        ASSERT(addr);
 
         checkProddableBlock(oc,
                             ((void**)(oc->image + sect->offset)) + i,
@@ -847,7 +846,7 @@ relocateSection(ObjectCode* oc, int curSection)
             IF_DEBUG(linker, debugBelch("               : value = %p\n", (void *)symbol->nlist->n_value));
 
             if ((symbol->nlist->n_type & N_TYPE) == N_SECT) {
-                ASSERT(symbol->addr != NULL);
+                CHECK(symbol->addr != NULL);
                 value = (uint64_t) symbol->addr;
                 IF_DEBUG(linker, debugBelch("relocateSection, defined external symbol %s, relocated address %p\n",
                                             nm, (void *)value));
@@ -949,29 +948,29 @@ relocateSection(ObjectCode* oc, int curSection)
         {
             if((int32_t)(value - baseValue) != (int64_t)(value - baseValue))
             {
-                ASSERT(reloc->r_extern);
+                CHECK(reloc->r_extern);
                 value = (uint64_t) &makeSymbolExtra(oc, reloc->r_symbolnum, value)
                                         -> jumpIsland;
             }
-            ASSERT((int32_t)(value - baseValue) == (int64_t)(value - baseValue));
+            CHECK((int32_t)(value - baseValue) == (int64_t)(value - baseValue));
             type = X86_64_RELOC_SIGNED;
         }
 
         switch(type)
         {
             case X86_64_RELOC_UNSIGNED:
-                ASSERT(!reloc->r_pcrel);
+                CHECK(!reloc->r_pcrel);
                 thing += value;
                 break;
             case X86_64_RELOC_SIGNED:
             case X86_64_RELOC_SIGNED_1:
             case X86_64_RELOC_SIGNED_2:
             case X86_64_RELOC_SIGNED_4:
-                ASSERT(reloc->r_pcrel);
+                CHECK(reloc->r_pcrel);
                 thing += value - baseValue;
                 break;
             case X86_64_RELOC_SUBTRACTOR:
-                ASSERT(!reloc->r_pcrel);
+                CHECK(!reloc->r_pcrel);
                 thing -= value;
                 break;
             default:


=====================================
rts/linker/PEi386.c
=====================================
@@ -1594,7 +1594,7 @@ ocGetNames_PEi386 ( ObjectCode* oc )
           barf ("Could not allocate any heap memory from private heap.");
       }
 
-      ASSERT(section.size == 0 || section.info->virtualSize == 0);
+      CHECK(section.size == 0 || section.info->virtualSize == 0);
       sz = section.size;
       if (sz < section.info->virtualSize) sz = section.info->virtualSize;
 
@@ -2032,7 +2032,7 @@ ocRunInit_PEi386 ( ObjectCode *oc )
   getProgEnvv(&envc, &envv);
 
   Section section = *oc->info->init;
-  ASSERT(SECTIONKIND_INIT_ARRAY == section.kind);
+  CHECK(SECTIONKIND_INIT_ARRAY == section.kind);
 
   uint8_t *init_startC = section.start;
   init_t *init_start   = (init_t*)init_startC;


=====================================
rts/linker/elf_got.c
=====================================
@@ -136,10 +136,10 @@ verifyGot(ObjectCode * oc) {
         for(size_t i=0; i < symTab->n_symbols; i++) {
             ElfSymbol * symbol = &symTab->symbols[i];
             if(symbol->got_addr) {
-                ASSERT((void*)(*(void**)symbol->got_addr)
-                       == (void*)symbol->addr);
+                CHECK((void*)(*(void**)symbol->got_addr)
+                      == (void*)symbol->addr);
             }
-            ASSERT(0 == ((uintptr_t)symbol->addr & 0xffff000000000000));
+            CHECK(0 == ((uintptr_t)symbol->addr & 0xffff000000000000));
         }
     }
     return EXIT_SUCCESS;



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

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/ed9d5a1e195f7ce19ea27dec040d8e9bbb18e661
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/20201123/160f9ade/attachment-0001.html>


More information about the ghc-commits mailing list