Closed Bug 1307561 Opened 3 years ago Closed 3 years ago

Windows ASan LIB path isn't getting picked up

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: dmajor, Unassigned)

References

Details

Attachments

(1 file)

$ cat $MOZCONFIG
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj/asan
ac_add_options --enable-address-sanitizer
ac_add_options --disable-jemalloc
export CC=clang-cl
export CXX=clang-cl
export LDFLAGS="clang_rt.asan_dynamic-i386.lib clang_rt.asan_dynamic_runtime_thunk-i386.lib"
export LIB="$LIB;D:\\tools\\llvm-active\\lib\\clang\\4.0.0\\lib\\windows"
export HOST_CFLAGS=" "
export HOST_CXXFLAGS=" "
export HOST_LDFLAGS=" "


 0:20.17 Executing: C:/PROGRA~2/MICROS~1.0/VC/bin/AMD64_~2/link.exe -NOLOGO -DLL -OUT:nptestjava.dll -PDB:nptestjava.pdb -SUBSYSTEM:WINDOWS,5.01 -MACHINE:X86 @d:\build\msys\s\central\obj\asan\dom\plugins\test\testplugin\javaplugin\tmpqr17v2.list nptest.res clang_rt.asan_dynamic-i386.lib clang_rt.asan_dynamic_runtime_thunk-i386.lib -LARGEADDRESSAWARE -NXCOMPAT -DYNAMICBASE -SAFESEH -DEBUG -DEBUGTYPE:CV -DEBUG -OPT:REF -DEF:d:/build/msys/s/central/dom/plugins/test/testplugin/nptest.def kernel32.lib user32.lib gdi32.lib winmm.lib wsock32.lib advapi32.lib secur32.lib msimg32.lib imm32.lib
 0:20.17 d:\build\msys\s\central\obj\asan\dom\plugins\test\testplugin\javaplugin\tmpqr17v2.list:
 0:20.17     Unified_cpp_javaplugin0.obj
 0:20.17 
 0:20.17 LINK : fatal error LNK1181: cannot open input file 'clang_rt.asan_dynamic-i386.lib'
 0:20.17
Ehsan suggested that I investigate lib_path here: https://dxr.mozilla.org/mozilla-central/rev/42c95d88aaaa7c2eca1d278399421d437441ac4d/build/moz.configure/windows.configure#314

I added a print statement before the return, and it didn't do anything. I even changed it to a die, and it still didn't do anything. I don't know if I'm doing something wrong.

Glandium, do you have any suggestions for what to do?
Flags: needinfo?(mh+mozilla)
Check objdir/config/autoconf.mk for the value of LIB
Flags: needinfo?(mh+mozilla)
and/or objdir/config.status
> Check objdir/config/autoconf.mk for the value of LIB
> and/or objdir/config.status

LIB doesn't appear in either of those files.
lib_path aborts due to:

def lib_path(target, vc_path, windows_sdk_dir, ucrt_sdk_dir, dia_sdk_dir):
    if not vc_path:
        return

https://dxr.mozilla.org/mozilla-central/rev/42c95d88aaaa7c2eca1d278399421d437441ac4d/build/moz.configure/windows.configure#314
So I tried this horrible hack to unblock myself:

 def vc_path(c_compiler):
     if c_compiler.type != 'msvc':
-        return
+        cl = find_program('cl')
+    else:
+        cl = find_program(c_compiler.compiler)
     # Normally, we'd start from c_compiler.compiler, but for now, it's not the
     # ideal full path to the compiler. At least, we're guaranteed find_program
     # will get us the one we found in toolchain.configure.
-    cl = find_program(c_compiler.compiler)

Now I at least get _something_ reasonable in LIB, but it doesn't contain the value from my mozconfig.

At this point in lib_path:

    lib_env = os.environ.get('LIB')

lib_env doesn't have the value from my mozconfig.
Ah, this is an issue similar to bug 1298740, which has a fix in bug 1305145. We should probably generally stop using os.environ in python configure code.
... or propagate the mozconfig environment into os.environ in python configure.
(In reply to Mike Hommey [:glandium] from comment #8)
> ... or propagate the mozconfig environment into os.environ in python
> configure.

This will probably match the expectations of the current code most closely. I'll post a patch in bug 1298740.
I am not encountering this problem on today's m-c. Did something change in the relevant parts of the build system recently?
I tried your mozconfig, and still have the same issue with https://hg.mozilla.org/mozilla-central/rev/10a2b6ebcd44.
(In reply to Ting-Yu Chou [:ting] from comment #11)
> I tried your mozconfig, and still have the same issue with
> https://hg.mozilla.org/mozilla-central/rev/10a2b6ebcd44.

Apply the hack in comment 6 and do a |./mach clobber| and a |./mach build| will have the LIB from mozconfig.
I guess we can use the environment variable VCDIR that mozilla-build/start-shell.bat set and get rid of vc_path in windows.configure.
None of VCDIR, VSPATH, VSWINPATH, and VCINSTALLDIR can be used for both local and buildbot build.
Comment on attachment 8807427 [details]
Bug 1307561 - Get the path of MSVC even though the compiler is mismatched to set LIB properly.

https://reviewboard.mozilla.org/r/90570/#review91158

I don't see what that has to do with asan libraries not being found. I can see this helping to find a Windows SDK when MSVC is also installed along clang-cl, but a) I don't think MSVC is required for clang-cl builds anyways b) nothing should be changing for pure MSVC builds c) it probably also breaks mingw builds.

So, what exactly are you trying to fix here?
Attachment #8807427 - Flags: review?(mh+mozilla) → review-
(In reply to Mike Hommey [:glandium] from comment #16)
> clang-cl, but a) I don't think MSVC is required for clang-cl builds anyways

No, it is not. But lib_path returns if not vc_path:

https://dxr.mozilla.org/mozilla-central/rev/86f702229e32c6119d092e86431afee576f033a1/build/moz.configure/windows.configure#316-317

, and vc_path returns if the compiler type is not MSVC:

https://dxr.mozilla.org/mozilla-central/rev/86f702229e32c6119d092e86431afee576f033a1/build/moz.configure/windows.configure#248-249

Which when the compiler type is not MSVC, set_config('LIB', lib_path):

https://dxr.mozilla.org/mozilla-central/rev/86f702229e32c6119d092e86431afee576f033a1/build/moz.configure/windows.configure#359

won't have the LIB from mozconfig. So even though we have LIB in mozconfig, is is not taken and linker will complain the libraries in LDFLAGS are not found.

> b) nothing should be changing for pure MSVC builds c) it probably also
> breaks mingw builds.

find_program() should find us the cl from PATH, so nothing is changed for pure MSVC builds. For the other compiler types: clang-cl, clang, gcc, the patch will make sure the LIB or INCLUDE from mozconfig is picked.

Or do you think it is lib_path and include_path should be fixed, so they do not depend on vc_*?
Flags: needinfo?(mh+mozilla)
(In reply to Ting-Yu Chou [:ting] from comment #17)
> (In reply to Mike Hommey [:glandium] from comment #16)
> > clang-cl, but a) I don't think MSVC is required for clang-cl builds anyways
> 
> No, it is not. But lib_path returns if not vc_path:
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> 86f702229e32c6119d092e86431afee576f033a1/build/moz.configure/windows.
> configure#316-317
> 
> , and vc_path returns if the compiler type is not MSVC:
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> 86f702229e32c6119d092e86431afee576f033a1/build/moz.configure/windows.
> configure#248-249
> 
> Which when the compiler type is not MSVC, set_config('LIB', lib_path):
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> 86f702229e32c6119d092e86431afee576f033a1/build/moz.configure/windows.
> configure#359
> 
> won't have the LIB from mozconfig. So even though we have LIB in mozconfig,
> is is not taken and linker will complain the libraries in LDFLAGS are not
> found.
> 
> > b) nothing should be changing for pure MSVC builds c) it probably also
> > breaks mingw builds.
> 
> find_program() should find us the cl from PATH

This is not guaranteed to be true forever, and would create confusion when that changes. Hypothetically, c_compiler.compiler might not be "cl" either, although that's a stretch.

>, so nothing is changed for
> pure MSVC builds. For the other compiler types: clang-cl, clang, gcc, the
> patch will make sure the LIB or INCLUDE from mozconfig is picked.
> 
> Or do you think it is lib_path and include_path should be fixed, so they do
> not depend on vc_*?

They should still depend on vc_*. lib_path and include_path, however, should take the value from the environment when vc_path is not found.

However, as things are, I'd expect it to actually already work, because when include_path and lib_path return None, set_config does nothing, so LIB and INCLUDE are *not* defined in autoconf.mk, and whatever is in the environment and/or mozconfig should take precedence.

BUT, it depends how you're setting it in your mozconfig. I'd expect "mk_add_options 'export LIB=...'" to work.
Flags: needinfo?(mh+mozilla)
You're right, "mk_add_options 'export LIB=...'" works.

It's still vague to me when to use "export XXX" and "mk_add_options 'export XXX=...'", but I think we can close this now. :dmajor, what do you think?
Flags: needinfo?(dmajor)
Ok.
Flags: needinfo?(dmajor)
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.