Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ntrel opened this issue Feb 22, 2022 · 7 comments
Open

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

ntrel opened this issue Feb 22, 2022 · 7 comments

Comments

@ntrel
Copy link

ntrel commented Feb 22, 2022

I built digger from git f2aeac4.

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
Copy link
Owner

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:

  • You could ask Digger to skip building rdmd with -c build.components.enable.rdmd = false
  • To build all tools via the Makefile (not sure if this would make a difference), you can combine the above with -c build.components.enable.tools = true
  • To use the Microsoft linker instead of OPTLINK, you could use -c build.components.common.model = 32mscoff or = 64.

@MoonlightSentinel
Copy link

MoonlightSentinel commented Feb 22, 2022

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
Copy link
Owner

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
Copy link

See dlang/dmd#13110

@CyberShadow
Copy link
Owner

CyberShadow commented Feb 22, 2022

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
Copy link

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
Copy link
Owner

CyberShadow commented Feb 22, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants