Closed Bug 1037058 Opened 7 years ago Closed 7 years ago

Build system support for ASAN on Windows

Categories

(Firefox Build System :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: ehsan.akhgari, Unassigned)

References

Details

I tried everything that I could think of in order to make it possible for us to build ASAN on Windows without changing the build system but this is proving next to impossible.  So I'd like to discuss my plan here.

ASAN support on Windows is implemented using two different runtime libraries, one that is supposed to be linked into the main exe which provides the shared ASAN runtime across the address space, and another one that is supposed to be linked into each DLL loaded by that exe which works by forwarding the calls into the ASAN runtime to the runtime library that lives in the exe by calling GetProcAddress to get access to some ASAN entry points exported from the main exe.

What this means for us is that we need to be able to pass the right runtime library when linking either an exe or DLL.  Here's my plan on how to do that:

1. Make WIN32_EXE_LDFLAGS be specific to target programs and not host programs.  I checked and currently all host programs only use the values for that flag in config.mk, so we should be able to add a HOST_WIN32_EXE_LDFLAGS to config.mk which maps to the same value as WIN32_EXE_LDFLAGS, and then extend WIN32_EXE_LDFLAGS to pass in the exe-specific ASAN runtime library when linking executables.

2. Add a WIN32_DLL_LDFLAGS macro to config.mk and adjust it on ASAN builds to pass in the right runtime library when linking DLLs.

3. Do not use any ASAN runtime library in .lib's, so that things such as mozglue can be linked into both DLLs and EXEs as needed.

I'm planning to keep all of my changes here specific to the MOZ_ASAN && CLANG_CL case.  Later on, we can think about integrating more of this support into --enable-address-sanitizer.

How does this sound?
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(gps)
glandium just moved all the library/linking metadata into moz.build in bug 1036894. That /could/ change how we implement things here, so adding a dependency.

I'll recuse myself from any detailed answers. It sounds like glandium has linking fully paged into his brain now, so he's your best bet.
Depends on: 1036894
Flags: needinfo?(gps)
Hmm, I read the patches in bug 1036894, but I'm not sure how they help with this...  Actually I think the plan in comment 0 would work either with or without that bug.
ping?
There's already a variable that's specific to shared libraries: EXTRA_DSO_LDOPTS. Just use that instead of adding yet another variable. As for HOST_WIN32_EXE_LDFLAGS, I think we don't need to care about ASAN'ing those at all.
Flags: needinfo?(mh+mozilla)
Thanks, I'll use EXTRA_DSO_LDOPTS.  As for HOST_WIN32_EXE_LDFLAGS, we need to add that so that we can put the ASAN libs for EXEs in WIN32_EXE_LDFLAGS without it affecting host tools.
You could abuse MOZ_GLUE_PROGRAM_LDFLAGS.
(In reply to comment #6)
> You could abuse MOZ_GLUE_PROGRAM_LDFLAGS.

I don't think that's going to work, since we define MOZ_GLUE_IN_PROGRAM here based on that <http://mxr.mozilla.org/mozilla-central/source/config/config.mk#316> and use __attribute__((weak)) based on that macro here: <http://mxr.mozilla.org/mozilla-central/source/mfbt/Types.h#92>.
(In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment #7)
> (In reply to comment #6)
> > You could abuse MOZ_GLUE_PROGRAM_LDFLAGS.
> 
> I don't think that's going to work, since we define MOZ_GLUE_IN_PROGRAM here
> based on that
> <http://mxr.mozilla.org/mozilla-central/source/config/config.mk#316> and use
> __attribute__((weak)) based on that macro here:
> <http://mxr.mozilla.org/mozilla-central/source/mfbt/Types.h#92>.

You can set its value after that MOZ_GLUE_IN_PROGRAM check. See http://mxr.mozilla.org/mozilla-central/source/config/config.mk#319 it's set there too.
Ah interesting, ok I'll try that.  Hopefully that is sufficient.
Depends on: 1041322
Depends on: 1043082
Depends on: 1043088
ASAN now supports -MD, so this bug is obsolete.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.