[Git][ghc/ghc][wip/T22834] 3 commits: nativeGen: Explicitly set flags of text sections on Windows

Ben Gamari (@bgamari) gitlab at gitlab.haskell.org
Wed Feb 15 02:52:02 UTC 2023



Ben Gamari pushed to branch wip/T22834 at Glasgow Haskell Compiler / GHC


Commits:
df3edc69 by Ben Gamari at 2023-02-08T11:57:44-05:00
nativeGen: Explicitly set flags of text sections on Windows

The binutils documentation (for COFF) claims,

> If no flags are specified, the default flags depend upon the section
> name. If the section name is not recognized, the default will be for the
> section to be loaded and writable.

We previously assumed that this would do the right thing for split
sections (e.g. a section named `.text$foo` would be correctly inferred
to be a text section). However, we have observed that this is not the
case (at least under the clang toolchain used on Windows): when
split-sections is enabled, text sections are treated by the assembler as
data (matching the "default" behavior specified by the documentation).

Avoid this by setting section flags explicitly. This should fix split
sections on Windows.

Fixes #22834.

- - - - -
3d16a52e by Ben Gamari at 2023-02-08T11:59:40-05:00
nativeGen: Set explicit section types on all platforms

- - - - -
24f97b1f by GHC GitLab CI at 2023-02-14T21:15:18-05:00
linker/PEi386: Don't sign-extend symbol section number

- - - - -


6 changed files:

- .gitlab-ci.yml
- .gitlab/gen_ci.hs
- .gitlab/jobs.yaml
- compiler/GHC/CmmToAsm/Ppr.hs
- rts/linker/PEi386.c
- rts/linker/PEi386.h


Changes:

=====================================
.gitlab-ci.yml
=====================================
@@ -502,7 +502,7 @@ doc-tarball:
       optional: true
     - job: nightly-x86_64-windows-validate
       optional: true
-    - job: release-x86_64-windows-release+no_split_sections
+    - job: release-x86_64-windows-release
       optional: true
 
   tags:
@@ -526,7 +526,7 @@ doc-tarball:
         || mv "ghc-x86_64-linux-deb10-release.tar.xz" "$LINUX_BINDIST" \
         || true
       mv "ghc-x86_64-windows-validate.tar.xz" "$WINDOWS_BINDIST" \
-        || mv "ghc-x86_64-windows-release+no_split_sections.tar.xz" "$WINDOWS_BINDIST" \
+        || mv "ghc-x86_64-windows-release.tar.xz" "$WINDOWS_BINDIST" \
         || true
       if [ ! -f "$LINUX_BINDIST" ]; then
         echo "Error: $LINUX_BINDIST does not exist. Did the Debian 9 job fail?"


=====================================
.gitlab/gen_ci.hs
=====================================
@@ -871,8 +871,8 @@ job_groups =
      -- This job is only for generating head.hackage docs
      , hackage_doc_job (disableValidate (standardBuildsWithConfig Amd64 (Linux Fedora33) releaseConfig))
      , disableValidate (standardBuildsWithConfig Amd64 (Linux Fedora33) dwarf)
-     , fastCI (standardBuildsWithConfig Amd64 Windows (splitSectionsBroken vanilla))
-     , disableValidate (standardBuildsWithConfig Amd64 Windows (splitSectionsBroken nativeInt))
+     , fastCI (standardBuildsWithConfig Amd64 Windows vanilla)
+     , disableValidate (standardBuildsWithConfig Amd64 Windows nativeInt)
      , standardBuilds Amd64 Darwin
      , allowFailureGroup (addValidateRule FreeBSDLabel (validateBuilds Amd64 FreeBSD13 vanilla))
      , standardBuilds AArch64 Darwin


=====================================
.gitlab/jobs.yaml
=====================================
@@ -3156,7 +3156,7 @@
       "XZ_OPT": "-9"
     }
   },
-  "release-x86_64-windows-int_native-release+no_split_sections": {
+  "release-x86_64-windows-int_native-release": {
     "after_script": [
       "bash .gitlab/ci.sh save_cache",
       "bash .gitlab/ci.sh clean"
@@ -3165,7 +3165,7 @@
     "artifacts": {
       "expire_in": "1 year",
       "paths": [
-        "ghc-x86_64-windows-int_native-release+no_split_sections.tar.xz",
+        "ghc-x86_64-windows-int_native-release.tar.xz",
         "junit.xml"
       ],
       "reports": {
@@ -3203,8 +3203,8 @@
     ],
     "variables": {
       "BIGNUM_BACKEND": "native",
-      "BIN_DIST_NAME": "ghc-x86_64-windows-int_native-release+no_split_sections",
-      "BUILD_FLAVOUR": "release+no_split_sections",
+      "BIN_DIST_NAME": "ghc-x86_64-windows-int_native-release",
+      "BUILD_FLAVOUR": "release",
       "CABAL_INSTALL_VERSION": "3.8.1.0",
       "CONFIGURE_ARGS": "",
       "GHC_VERSION": "9.4.3",
@@ -3212,11 +3212,11 @@
       "IGNORE_PERF_FAILURES": "all",
       "LANG": "en_US.UTF-8",
       "MSYSTEM": "CLANG64",
-      "TEST_ENV": "x86_64-windows-int_native-release+no_split_sections",
+      "TEST_ENV": "x86_64-windows-int_native-release",
       "XZ_OPT": "-9"
     }
   },
-  "release-x86_64-windows-release+no_split_sections": {
+  "release-x86_64-windows-release": {
     "after_script": [
       "bash .gitlab/ci.sh save_cache",
       "bash .gitlab/ci.sh clean"
@@ -3225,7 +3225,7 @@
     "artifacts": {
       "expire_in": "1 year",
       "paths": [
-        "ghc-x86_64-windows-release+no_split_sections.tar.xz",
+        "ghc-x86_64-windows-release.tar.xz",
         "junit.xml"
       ],
       "reports": {
@@ -3263,8 +3263,8 @@
     ],
     "variables": {
       "BIGNUM_BACKEND": "gmp",
-      "BIN_DIST_NAME": "ghc-x86_64-windows-release+no_split_sections",
-      "BUILD_FLAVOUR": "release+no_split_sections",
+      "BIN_DIST_NAME": "ghc-x86_64-windows-release",
+      "BUILD_FLAVOUR": "release",
       "CABAL_INSTALL_VERSION": "3.8.1.0",
       "CONFIGURE_ARGS": "",
       "GHC_VERSION": "9.4.3",
@@ -3272,7 +3272,7 @@
       "IGNORE_PERF_FAILURES": "all",
       "LANG": "en_US.UTF-8",
       "MSYSTEM": "CLANG64",
-      "TEST_ENV": "x86_64-windows-release+no_split_sections",
+      "TEST_ENV": "x86_64-windows-release",
       "XZ_OPT": "-9"
     }
   },


=====================================
compiler/GHC/CmmToAsm/Ppr.hs
=====================================
@@ -245,6 +245,10 @@ pprGNUSectionHeader config t suffix =
       OtherSection _ ->
         panic "PprBase.pprGNUSectionHeader: unknown section type"
     flags = case t of
+      Text
+        | OSMinGW32 <- platformOS platform
+                    -> text ",\"xr\""
+        | otherwise -> text ",\"ax\"," <> sectionType platform "progbits"
       CString
         | OSMinGW32 <- platformOS platform
                     -> empty


=====================================
rts/linker/PEi386.c
=====================================
@@ -697,8 +697,16 @@ size_t getSymbolSize ( COFF_HEADER_INFO *info )
     }
 }
 
+// Constants which may be returned by getSymSectionNumber.
+// See https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#section-number-values
+#define PE_SECTION_UNDEFINED ((uint32_t) 0)
+#define PE_SECTION_ABSOLUTE  ((uint32_t) -1)
+#define PE_SECTION_DEBUG     ((uint32_t) -2)
+
+// Returns either PE_SECTION_{UNDEFINED,ABSOLUTE,DEBUG} or the (one-based)
+// section number of the given symbol.
 __attribute__ ((always_inline)) inline
-int32_t getSymSectionNumber ( COFF_HEADER_INFO *info, COFF_symbol* sym )
+uint32_t getSymSectionNumber ( COFF_HEADER_INFO *info, COFF_symbol* sym )
 {
     ASSERT(info);
     ASSERT(sym);
@@ -707,7 +715,13 @@ int32_t getSymSectionNumber ( COFF_HEADER_INFO *info, COFF_symbol* sym )
         case COFF_ANON_BIG_OBJ:
             return sym->ex.SectionNumber;
         default:
-            return sym->og.SectionNumber;
+            // Take care to only sign-extend reserved values; see #22941.
+            switch (sym->og.SectionNumber) {
+                case IMAGE_SYM_UNDEFINED: return PE_SECTION_UNDEFINED;
+                case IMAGE_SYM_ABSOLUTE : return PE_SECTION_ABSOLUTE;
+                case IMAGE_SYM_DEBUG: return PE_SECTION_DEBUG;
+                default: return (uint16_t) sym->og.SectionNumber;
+            }
     }
 }
 
@@ -1652,7 +1666,7 @@ ocGetNames_PEi386 ( ObjectCode* oc )
    StgWord globalBssSize = 0;
    for (unsigned int i=0; i < info->numberOfSymbols; i++) {
       COFF_symbol* sym = &oc->info->symbols[i];
-      if (getSymSectionNumber (info, sym) == IMAGE_SYM_UNDEFINED
+      if (getSymSectionNumber (info, sym) == PE_SECTION_UNDEFINED
            && getSymValue (info, sym) > 0
            && getSymStorageClass (info, sym) != IMAGE_SYM_CLASS_SECTION) {
            globalBssSize += getSymValue (info, sym);
@@ -1685,21 +1699,39 @@ ocGetNames_PEi386 ( ObjectCode* oc )
    for (unsigned int i = 0; i < (uint32_t)oc->n_symbols; i++) {
       COFF_symbol* sym = &oc->info->symbols[i];
 
-      int32_t secNumber = getSymSectionNumber (info, sym);
       uint32_t symValue = getSymValue (info, sym);
       uint8_t symStorageClass = getSymStorageClass (info, sym);
-
       SymbolAddr *addr = NULL;
       bool isWeak = false;
       SymbolName *sname = get_sym_name (getSymShortName (info, sym), oc);
-      Section *section = secNumber > 0 ? &oc->sections[secNumber-1] : NULL;
+
+      uint32_t secNumber = getSymSectionNumber (info, sym);
+      Section *section;
+      switch (secNumber) {
+        case PE_SECTION_UNDEFINED:
+          IF_DEBUG(linker, debugBelch("symbol %s is UNDEFINED, skipping...\n", sname));
+          i += getSymNumberOfAuxSymbols (info, sym);
+          continue;
+        case PE_SECTION_ABSOLUTE:
+          IF_DEBUG(linker, debugBelch("symbol %s is ABSOLUTE, skipping...\n", sname));
+          i += getSymNumberOfAuxSymbols (info, sym);
+          continue;
+        case PE_SECTION_DEBUG:
+          IF_DEBUG(linker, debugBelch("symbol %s is DEBUG, skipping...\n", sname));
+          i += getSymNumberOfAuxSymbols (info, sym);
+          continue;
+        default:
+          CHECK(secNumber < (uint32_t) oc->n_sections);
+          section = &oc->sections[secNumber-1];
+      }
 
       SymType type;
       switch (getSymType(oc->info->ch_info, sym)) {
       case 0x00: type = SYM_TYPE_DATA; break;
       case 0x20: type = SYM_TYPE_CODE; break;
       default:
-          debugBelch("Invalid symbol type: 0x%x\n", getSymType(oc->info->ch_info, sym));
+          debugBelch("Symbol %s has invalid type 0x%x\n",
+                     sname, getSymType(oc->info->ch_info, sym));
           return 1;
       }
 
@@ -1730,8 +1762,18 @@ ocGetNames_PEi386 ( ObjectCode* oc )
           CHECK(symValue == 0);
           COFF_symbol_aux_weak_external *aux = (COFF_symbol_aux_weak_external *) (sym+1);
           COFF_symbol* targetSym = &oc->info->symbols[aux->TagIndex];
-          int32_t targetSecNumber = getSymSectionNumber (info, targetSym);
-          Section *targetSection = targetSecNumber > 0 ? &oc->sections[targetSecNumber-1] : NULL;
+
+          uint32_t targetSecNumber = getSymSectionNumber (info, targetSym);
+          Section *targetSection;
+          switch (targetSecNumber) {
+            case PE_SECTION_UNDEFINED:
+            case PE_SECTION_ABSOLUTE:
+            case PE_SECTION_DEBUG:
+              targetSection = NULL;
+              break;
+            default:
+              targetSection = &oc->sections[targetSecNumber-1];
+          }
           addr = (SymbolAddr*) ((size_t) targetSection->start + getSymValue(info, targetSym));
       }
       else if (  secNumber == IMAGE_SYM_UNDEFINED && symValue > 0) {
@@ -1976,7 +2018,19 @@ ocResolve_PEi386 ( ObjectCode* oc )
                   debugBelch("'\n" ));
 
          if (getSymStorageClass (info, sym) == IMAGE_SYM_CLASS_STATIC) {
-            Section section = oc->sections[getSymSectionNumber (info, sym)-1];
+            uint32_t sect_n = getSymSectionNumber (info, sym);
+            switch (sect_n) {
+                case PE_SECTION_UNDEFINED:
+                case PE_SECTION_ABSOLUTE:
+                case PE_SECTION_DEBUG:
+                    errorBelch(" | %" PATH_FMT ": symbol `%s' has invalid section number %02x",
+                               oc->fileName, symbol, sect_n);
+                    return false;
+                default:
+                    break;
+            }
+            CHECK(sect_n < (uint32_t) oc->n_sections);
+            Section section = oc->sections[sect_n - 1];
             S = ((size_t)(section.start))
               + ((size_t)(getSymValue (info, sym)));
          } else {


=====================================
rts/linker/PEi386.h
=====================================
@@ -143,7 +143,7 @@ struct _Alignments {
 COFF_OBJ_TYPE getObjectType ( char* image, pathchar* fileName );
 COFF_HEADER_INFO* getHeaderInfo ( ObjectCode* oc );
 size_t getSymbolSize ( COFF_HEADER_INFO *info );
-int32_t getSymSectionNumber ( COFF_HEADER_INFO *info, COFF_symbol* sym );
+uint32_t getSymSectionNumber ( COFF_HEADER_INFO *info, COFF_symbol* sym );
 uint32_t getSymValue ( COFF_HEADER_INFO *info, COFF_symbol* sym );
 uint8_t getSymStorageClass ( COFF_HEADER_INFO *info, COFF_symbol* sym );
 uint8_t getSymNumberOfAuxSymbols ( COFF_HEADER_INFO *info, COFF_symbol* sym );



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/855f4c9b3eda9333fb7006692fec1c7d87745c09...24f97b1fc34a9f02f1f608a0a93ec269db1813d1

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/855f4c9b3eda9333fb7006692fec1c7d87745c09...24f97b1fc34a9f02f1f608a0a93ec269db1813d1
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/20230214/a0498622/attachment-0001.html>


More information about the ghc-commits mailing list