CyberShadow / Digger

A tool to build D and bisect old D versions
Other
57 stars 9 forks source link

optlink fails with 'Error 138: Module or Dictionary corrupt' #92

Open ntrel opened 2 years ago

ntrel commented 2 years ago

I built digger from git f2aeac45252e15b01afedb842b3faddf4c83989a.

Windows 8 Digger v3.0.6 - a D source code building and archaeology tool

$ dmd --version
DMD32 D Compiler v2.098.1-dirty

The last page or so of digger output of ../Digger/digger.exe rebuild is shown below. I found the same optlink error still happens using ./work/build/bin/dmd.exe when trying to link a simple test program.

digger: phobos-incremental-71e4b2b1da6e4c22ca9f3e12af2a957a built OK!
digger: Copy: C:\git\dmd\work\repo\phobos\phobos.lib -> C:\git\dmd\work\temp\phobos-incremental-71e4b2b1da6e4c22ca9f3e12af2a957a\lib\phobos.lib
digger: Saving to cache.
digger: Installing phobos-incremental-71e4b2b1da6e4c22ca9f3e12af2a957a
digger: Copy: C:\git\dmd\work\temp-cache\v3\phobos-incremental-71e4b2b1da6e4c22ca9f3e12af2a957a\lib -> C:\git\dmd\work\build\lib
digger: needInstalled: rdmd-incremental-9559138c4afc9734e928ad2466d748f0
digger: Cache miss.
digger: needBuild: rdmd-incremental-9559138c4afc9734e928ad2466d748f0
digger: Building rdmd-incremental-9559138c4afc9734e928ad2466d748f0
digger: DMC=C:\git\dmd\work\dl\dm857-snn2074-optlink80017\bin
digger: Environment: SystemDrive=C:
digger: Environment: TMPDIR=C:\git\dmd\work\tmp
digger: Environment: HOME=C:\git\dmd\work\home
digger: Environment: SystemRoot=C:\WINDOWS
digger: Environment: PATH=C:\git\dmd\work\dl\dm857-snn2074-optlink80017\bin;C:\WINDOWS\system32;C:\WINDOWS;C:\Program Files\Git\mingw64\bin
digger: Environment: TEMP=C:\git\dmd\work\tmp
digger: Environment: TMP=C:\git\dmd\work\tmp
digger: Environment: DMC=C:\git\dmd\work\dl\dm857-snn2074-optlink80017\bin
digger: Working directory: C:\git\dmd\work\repo\tools
digger: Running: "C:\git\dmd\work\build\bin\dmd.exe" ^"-m32^" rdmd
OPTLINK (R) for Win32  Release 8.00.17
Copyright (C) Digital Mars 1989-2013  All rights reserved.
http://www.digitalmars.com/ctg/optlink.html
rdmd.obj Offset 00000H Record Type 004C
 Error 138: Module or Dictionary corrupt
Error: linker exited with status 1
digger: Saving to cache.
digger: Clearing temporary cache
Fatal error: Command ["C:\\git\\dmd\\work\\build\\bin\\dmd.exe", "-m32", "rdmd"] failed with status 1
CyberShadow commented 2 years ago

Well, that looks like an OPTLINK bug, right? So it should be reported to issues.dlang.org, not here.

If you're just looking for a workaround:

MoonlightSentinel commented 2 years ago

Which dmd revision is used to build rdmd? This looks like an issue fixed by dlang/dmd#13611 - -m32 was changed to emit MsCoff files but still called OPTLINK due to an outdated entry in the sc.ini.

CyberShadow commented 2 years ago

Which dmd revision is used to build rdmd?

The one from the point in time corresponding to the version of rdmd being built (so, latest if Digger is told to build master).

This looks like an issue fixed by dlang/dmd#13611 - -m32 emitted MsCoff files but called OPTLINK due to an outdated entry in the sc.ini.

That looks interesting. Digger generates sc.ini files, so maybe it needs to be updated. Do you have a link to the change in DMD which introduced this? (Also, wasn't this a breaking change?)

MoonlightSentinel commented 2 years ago

See dlang/dmd#13110

CyberShadow commented 2 years ago

Thanks!

Yeah, that's a bit of an oof. A single pull request which

  1. introduces -m32omf
  2. changes the default
  3. changes the meaning of -m32

That's going to be annoying to deal with.

MoonlightSentinel commented 2 years ago

Yeah, that's a bit of an oof. A single pull request which

Adding -m32omf before switching the defaults would've been better (the PR broke druntime / phobos CI)

Digger generates sc.ini files, so maybe it needs to be updated.

Probably. But digger should rather reuse / patch the existing configuration files from dmd (either located in ini or generated by build.d) to avoid such issues in the future.

CyberShadow commented 2 years ago

So, there is a bit of a dilemma.

Digger's equivalent of -m32 could mean one of the following:

  1. Ask DMD to generate 32-bit COFF object files
  2. Ask DMD to generate 32-bit OMF object files
  3. Ask DMD to generate 32-bit object files using whatever the default was at the time

Conceptually, you almost never want 3 when bisecting. If your test command relies on anything at all adjacent or sensitive to the object file format and all consequences stemming from that decision (like which libc to use), then you're likely to get a false result pointing at that pull request which changed the default.

One exception to this is when bisecting over a version range so wide that it spans the point when 32-bit COFF was introduced (or at least became usable given the context) and the point where OMF was (will be) removed. Then, neither 1 nor 2 will work.

Another exception is of course when the regression was indeed caused by the change of the default, but you don't really need Digger to diagnose this.

Supporting all three in Digger (as 32 / 32coff / 32omf) might be impractical, there is already a good amount of complexity for supporting multi-model builds (i.e. 32 + 64).

Probably. But digger should rather reuse / patch the existing configuration files from dmd (either located in ini or generated by build.d) to avoid such issues in the future.

I think I looked into this when I wrote it but found that it was too impractical considering the version range that Digger aims to support. Old sc.ini files had hard-coded paths that expected you to install the compiler in \dm\dmd or such.