Opened 6 months ago

Closed 5 months ago

#8698 closed bug (fixed)

.ctors handling does not work on Windows 64-bit ghci

Reported by: awson Owned by: ezyang
Priority: normal Milestone: 7.8.1
Component: Compiler Version: 7.8.1-rc1
Keywords: Cc: ezyang
Operating System: Windows Architecture: x86_64 (amd64)
Type of failure: GHCi crash Difficulty: Easy (less than 1 hour)
Test Case: Blocked By:
Blocking: Related Tickets:

Description

When fixing #7134 I've found .ctors section handling does not work on 64-bit Windows ghci.

Patch https://ghc.haskell.org/trac/ghc/attachment/ticket/7134/ghc-w64fix.patch now disables .ctor handling for 64-bit Windows ghci.

Attachments (2)

cbits.o (3.2 KB) - added by awson 6 months ago.
cbits.o from integer-gmp. I think it is the culprit.
0001-Fix-8698-by-properly-handling-long-section-names-and.patch (5.3 KB) - added by ezyang 5 months ago.
Proposed fix

Download all attachments as: .zip

Change History (21)

comment:1 Changed 6 months ago by awson

https://ghc.haskell.org/trac/ghc/attachment/ticket/7134/ghc-w64fix.patch is updated to mark relevant places with the number of this ticket (#8698).

comment:2 Changed 6 months ago by ezyang

  • Cc ezyang added
  • Owner set to ezyang

Can you upload the object file in question?

Changed 6 months ago by awson

cbits.o from integer-gmp. I think it is the culprit.

comment:3 Changed 6 months ago by awson

Done.

comment:4 Changed 6 months ago by awson

Also, it might be necessary to add RTS_WIN64_ONLY(SymI_HasProto(FreeEnvironmentStringsW)) to RTS_MINGW_ONLY_SYMBOLS definition.

comment:5 Changed 6 months ago by ezyang

OK, I can't figure out what the problem is just by eyeballing the code, so I'll need to reproduce this on my machine. Can you give detailed instructions for reproduction? What patches do I need to apply?

comment:6 Changed 6 months ago by thoughtpolice

You can set yourself up a 64bit Windows MSYS environment following this if you don't have one already:

https://ghc.haskell.org/trac/ghc/wiki/Building/Preparation/Windows/MSYS2

FWIW, me and Herbert also saw this failure. It can be triggered by a vanilla perf build, i.e. just clone and:

$ ./boot; ./configure
$ make

should trigger it while building stage2 libraries.

Note that for some exceptionally strange reason, me and Herbert could not produce these results with ./validate - you must do a regular perf build.

I have not merged Kyra's patch for #7134 yet - so right now the build should still fail, no patches necessary.

comment:7 Changed 6 months ago by thoughtpolice

I'll also mention that I do have a internet-facing Win64 build machine where you could reproduce this error from too, if you want (but a local VM would probably be easier.)

comment:8 Changed 5 months ago by ezyang

NB: #7134 has since been merged, so that patch needs to be taken out.

Last edited 5 months ago by ezyang (previous) (diff)

comment:9 Changed 5 months ago by ezyang

This patch seems to be enough to do that:

diff --git a/rts/Linker.c b/rts/Linker.c
index b9c8fd0..34f6803 100644
--- a/rts/Linker.c
+++ b/rts/Linker.c
@@ -211,9 +211,7 @@ static int ocAllocateSymbolExtras_ELF ( ObjectCode* oc );
 static int ocVerifyImage_PEi386 ( ObjectCode* oc );
 static int ocGetNames_PEi386    ( ObjectCode* oc );
 static int ocResolve_PEi386     ( ObjectCode* oc );
-#if !defined(x86_64_HOST_ARCH)
 static int ocRunInit_PEi386     ( ObjectCode* oc );
-#endif
 static void *lookupSymbolInDLLs ( unsigned char *lbl );
 static void zapTrailingAtSign   ( unsigned char *sym );
 static char *allocateImageAndTrampolines (
@@ -2875,10 +2873,8 @@ resolveObjs( void )
 #if defined(OBJFORMAT_ELF)
             r = ocRunInit_ELF ( oc );
 #elif defined(OBJFORMAT_PEi386)
-#if !defined(x86_64_HOST_ARCH)
             /* It does not work on x86_64 yet. #8698. */
             r = ocRunInit_PEi386 ( oc );
-#endif
 #elif defined(OBJFORMAT_MACHO)
             r = ocRunInit_MachO ( oc );
 #else
@@ -4235,14 +4231,6 @@ ocResolve_PEi386 ( ObjectCode* oc )
           continue;
       }
 
-#if defined(x86_64_HOST_ARCH)
-      /* It does not work on x86_64 yet. #8698. */
-      if (0 == strcmp(".ctors", (char*)secname)) {
-          stgFree(secname);
-          continue;
-      }
-#endif
-
       stgFree(secname);
 
       if ( sectab_i->Characteristics & MYIMAGE_SCN_LNK_NRELOC_OVFL ) {
@@ -4415,7 +4403,6 @@ ocResolve_PEi386 ( ObjectCode* oc )
 }
 
 /* It does not work on x86_64 yet. #8698. */
-#if !defined(x86_64_HOST_ARCH)
 static int
 ocRunInit_PEi386 ( ObjectCode *oc )
 {
@@ -4458,7 +4445,6 @@ ocRunInit_PEi386 ( ObjectCode *oc )
     freeProgEnvv(envc, envv);
     return 1;
 }
-#endif
 
 #endif /* defined(OBJFORMAT_PEi386) */
 

Last edited 5 months ago by ezyang (previous) (diff)

comment:10 Changed 5 months ago by ezyang

I've identified the bug: our section name comparison function only handles short names, but the relocation here is a long name. Cooking up a fix.

comment:11 Changed 5 months ago by ezyang

  • Difficulty changed from Unknown to Easy (less than 1 hour)

comment:12 Changed 5 months ago by Edward Z. Yang <ezyang@…>

In 40ce20357fb6266471a53cec7de0a810a3070f36/ghc:

Fix #8698 by properly handling long section names and reenabling .ctors handling

Our old function for searching for sections could only deal
with section names that were eight bytes or shorter; this
patch adds support for long section names.

Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>

comment:13 Changed 5 months ago by ezyang

  • Status changed from new to merge

comment:14 Changed 5 months ago by awson

At 4323:
errorBelch("%" PATH_FMT ": can't find section named: ", oc->fileName);
named as what?

comment:15 Changed 5 months ago by ezyang

It's printed using printName the next line. There might be a better way of doing this.

comment:16 Changed 5 months ago by awson

Ah, sorry, I've overlooked this. printName uses debugBelch though.

comment:17 Changed 5 months ago by ezyang

Oh, that is not going to work properly then. Hm, I guess we could parametrize printName with which belch function to use?

comment:18 Changed 5 months ago by thoughtpolice

  • Milestone set to 7.8.1

comment:19 Changed 5 months ago by thoughtpolice

  • Resolution set to fixed
  • Status changed from merge to closed

Merged.

Note: See TracTickets for help on using tickets.