Closed Bug 1448749 Opened 7 years ago Closed 6 years ago

undefined reference to `__imp_mozalloc_abort' error in pdfium in MinGW x64

Categories

(Firefox Build System :: General, defect)

3 Branch
defect
Not set
normal

Tracking

(firefox-esr60 fixed, firefox62 fixed)

RESOLVED FIXED
mozilla62
Tracking Status
firefox-esr60 --- fixed
firefox62 --- fixed

People

(Reporter: tjr, Assigned: tjr)

References

Details

(Whiteboard: [tor])

Attachments

(1 file)

In Bug 1443823 I solved this for crashinject with no-keep-inline-dllexport

But I get the same errors in pdfium, and have that flag.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f83c084fe6463d32f56c290fafe79949c604c3fd&selectedJob=170019791

> /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/mingw32/bin/x86_64-w64-mingw32-g++ -mwindows -o Unified_cpp_modules_pdfium19.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -DDEBUG=1 -DOPJ_STATIC -DPNG_PREFIX -DPNG_USE_READ_MACROS -DPDFIUM_PRINT_TEXT_WITH_GDI -DV8_DEPRECATION_WARNINGS -DUSE_LIBJPEG_TURBO -DMOZ_HAS_MOZGLUE -I/builds/worker/workspace/build/src/modules/pdfium -I/builds/worker/workspace/build/src/obj-firefox/modules/pdfium -I/builds/worker/workspace/build/src/modules/pdfium/pdfium -I/builds/worker/workspace/build/src/modules/freetype2/include -I/builds/worker/workspace/build/src/modules/freetype2/src -I/builds/worker/workspace/build/src/modules/zlib/src -I/builds/worker/workspace/build/src/media/libjpeg -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wno-unknown-pragmas -Wno-unused-function -Wno-conversion-null -Wno-switch -Wno-enum-compare -fno-sized-deallocation -fno-exceptions -fno-strict-aliasing -mms-bitfields -mstackrealign -fno-keep-inline-dllexport -fno-rtti -ffunction-sections -fdata-sections -Wa,-mbig-obj -fno-exceptions -fno-math-errno -pthread -pipe -g -fno-omit-frame-pointer  -MD -MP -MF .deps/Unified_cpp_modules_pdfium19.o.pp   /builds/worker/workspace/build/src/obj-firefox/modules/pdfium/Unified_cpp_modules_pdfium19.cpp
> ...
> ...
> /builds/worker/workspace/build/src/obj-firefox/_virtualenv/bin/python /builds/worker/workspace/build/src/config/expandlibs_exec.py --uselist --  /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/mingw32/bin/x86_64-w64-mingw32-g++ -mwindows -shared -Wl,--gc-sections -Wl,--out-implib -Wl,libpdfium.a -o pdfium.dll cmsnamed.o cmsps2.o cmstypes.o Unified_c_modules_pdfium0.o Unified_c_modules_pdfium1.o fx_crypt.o fx_crypt_sha.o cpdf_dibsource.o cpdf_interform.o cpdf_nametree.o doc_tagged.o fx_codec_fax.o fx_win32_dwrite.o fx_win32_gdipext.o BigInteger.o BigUnsigned.o Unified_cpp_modules_pdfium0.o Unified_cpp_modules_pdfium1.o Unified_cpp_modules_pdfium10.o Unified_cpp_modules_pdfium11.o Unified_cpp_modules_pdfium12.o Unified_cpp_modules_pdfium13.o Unified_cpp_modules_pdfium14.o Unified_cpp_modules_pdfium15.o Unified_cpp_modules_pdfium16.o Unified_cpp_modules_pdfium17.o Unified_cpp_modules_pdfium18.o Unified_cpp_modules_pdfium19.o Unified_cpp_modules_pdfium2.o Unified_cpp_modules_pdfium20.o Unified_cpp_modules_pdfium21.o Unified_cpp_modules_pdfium3.o Unified_cpp_modules_pdfium4.o Unified_cpp_modules_pdfium5.o Unified_cpp_modules_pdfium6.o Unified_cpp_modules_pdfium7.o Unified_cpp_modules_pdfium8.o Unified_cpp_modules_pdfium9.o ./module.res -lpthread -Wl,--build-id -static /builds/worker/workspace/build/src/modules/pdfium/pdfium.def   ../../config/external/freetype2/libfreetype.a ../../config/external/zlib/libzlib.a ../../media/libjpeg/libmedia_libjpeg.a  ../../mozglue/build/libmozglue.a   -luuid -lgdi32 -lwinmm -lwsock32 -luserenv -lsecur32 -ladvapi32 -lgdi32 -luser32
> Executing: ../../../sccache2/sccache ../../../mingw32/bin/x86_64-w64-mingw32-g++ -mwindows -shared -Wl,--gc-sections -Wl,--out-implib -Wl,libpdfium.a -o pdfium.dll /builds/worker/workspace/build/src/obj-firefox/modules/pdfium/tmprzmb52.list module.res -lpthread -Wl,--build-id -static ../../../modules/pdfium/pdfium.def ../../mozglue/build/libmozglue.a -luuid -lgdi32 -lwinmm -lwsock32 -luserenv -lsecur32 -ladvapi32 -lgdi32 -luser32
> /builds/worker/workspace/build/src/obj-firefox/modules/pdfium/tmprzmb52.list:
>     INPUT("cmsnamed.o")
>     ...
>     INPUT("Unified_cpp_modules_pdfium19.o")
>     ...
> Unified_cpp_modules_pdfium19.o: In function `__throw_length_error':
> /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/throw_gcc.h:102: undefined reference to `__imp_mozalloc_abort'
> /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/throw_gcc.h:102: undefined reference to `__imp_mozalloc_abort'
> /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/throw_gcc.h:102: undefined reference to `__imp_mozalloc_abort'
> Unified_cpp_modules_pdfium19.o: In function `__throw_bad_alloc':
> /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/throw_gcc.h:59: undefined reference to `__imp_mozalloc_abort'
> /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/throw_gcc.h:59: undefined reference to `__imp_mozalloc_abort'
> Unified_cpp_modules_pdfium19.o:/builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/throw_gcc.h:59: more undefined references to `__imp_mozalloc_abort' follow
> collect2: error: ld returned 1 exit status


I compared x86 and x64 and found a missing linker flag: -Wl,--enable-stdcall-fixup

That sounds promising, but it gave the same errors: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6e7b1ccc7be900ba93692bd44211a989c5fd52ed&selectedJob=170205478


After adding --enable-stdcall-fixup, on the debug builds this also appears on in libangle (but not in the optimized builds...
Flags: needinfo?(jacek)
The link line shows it linking against mozglue as a static library: `../../mozglue/build/libmozglue.a`, which is presumably why it can't link against `dllimport` symbols. I don't know if that's intentional or not.
Hm... that same pattern appears in other link lines for other dlls...

> /builds/worker/workspace/build/src/obj-firefox/_virtualenv/bin/python /builds/worker/workspace/build/src/config/expandlibs_exec.py --uselist --  /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/mingw32/bin/x86_64-w64-mingw32-g++ -mwindows -shared -Wl,--gc-sections -Wl,--out-implib -Wl,libmozglue.a -o mozglue.dll Authenticode.o SSE.o WindowsDllBlocklist.o dummy.o ./module.res -lpthread -Wl,--build-id -static mozglue.def -DELAYLOAD:user32.dll -DELAYLOAD:crypt32.dll -DELAYLOAD:wintrust.dll   ../../memory/build/libmemory.a ../../memory/mozalloc/libmemory_mozalloc.a ../../mozglue/misc/libmozglue_misc.a ../../mfbt/libmfbt.a     -luuid -lgdi32 -lwinmm -lwsock32 -luserenv -lsecur32 -ldbghelp -lcrypt32 -lversion -lwintrust -ldelayimp
> 
> /builds/worker/workspace/build/src/obj-firefox/_virtualenv/bin/python /builds/worker/workspace/build/src/config/expandlibs_exec.py --uselist --  /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/mingw32/bin/x86_64-w64-mingw32-g++ -mwindows -shared -Wl,--gc-sections -Wl,--out-implib -Wl,liblgpllibs.a -o lgpllibs.dll  ./module.res -lpthread -Wl,--build-id -static /builds/worker/workspace/build/src/config/external/lgpllibs/lgpllibs.def   ../../../media/libav/libmedia_libav.a ../../../media/libav/libavutil/x86/libmedia_libav_libavutil_x86.a ../../../media/libsoundtouch/src/libmedia_libsoundtouch_src.a  ../../../mozglue/build/libmozglue.a   -luuid -lgdi32 -lwinmm -lwsock32 -luserenv -lsecur32
>
> /builds/worker/workspace/build/src/obj-firefox/_virtualenv/bin/python /builds/worker/workspace/build/src/config/expandlibs_exec.py --uselist --  /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/mingw32/bin/x86_64-w64-mingw32-gcc -std=gnu99 -mwindows -shared -Wl,--gc-sections -Wl,--out-implib -Wl,libnss3.a -o nss3.dll  ./module.res -lpthread -Wl,--build-id -static   ../db/sqlite3/src/libdb_sqlite3_src.a ../config/external/nspr/pr/libnspr4.a ../security/nss/lib/nss/nss_nss3_static/libnss3_static.a ../security/nss/lib/util/util_nssutil/libnssutil.a ../config/external/nspr/libc/libplc4.a ../config/external/nspr/ds/libplds4.a ../security/nss/lib/smime/smime_smime3_static/libsmime3_static.a ../security/nss/lib/ssl/ssl_ssl/libssl.a  ../mozglue/build/libmozglue.a nss3.dll.def  -luuid -lgdi32 -lwinmm -lwsock32 -luserenv -lsecur32 -ladvapi32 -lws2_32 -lmswsock -lwinmm
fallout from bug 1429875?
(In reply to Mike Hommey [:glandium] from comment #3)
> fallout from bug 1429875?

My tree doesn't have that patch (and actually, now that --disable-stylo is gone if it's not backport friendly it will be difficult to test with it.)

Jacek suggested:

> 15:59:06 J<jacek> tjr: would it be easy to change mozalloc_declaration depending on how we link to mozglue
> 15:59:32 J<jacek> if it's dllimport issue, it sounds similar to what we had to deal with library folding
> 15:59:56 J<jacek> with the hack to make dllimport noop
> 16:00:14 J<jacek> it would be good not to do that for the rest of the tree
> 16:01:35 T<tjr> Hm. I will look around at MOZ_FOLD_LIBS and see if I can understand :)
> 16:03:50 J<jacek> the problem is that gcc and/or ld can handle if you mark functions as dllimport, but try to link with actual implementation

I'm not sure I fully understood that, but I tried adding -mnop-fun-dllimport here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8766acf9430c7b6548ad65d5987625e279f36d4a&selectedJob=170667490

But even though I see it in the g++ command that ' -o Unified_cpp_modules_pdfium19.o' - it doesn't make a difference.
The point is that you shouldn't be linking against a mozglue.a, but against a mozglue.dll.a.
I found a combination of things that resolved this. I've attached a patch for folks to look at; but I haven't done a full try run yet to see what adding extern "C" breaks. (The compiler flags I add would definitely be MinGW-only)
Flags: needinfo?(jacek)
It seems like this did not actually have any adverse affects for the other builds: https://treeherder.mozilla.org/#/jobs?repo=try&revision=030b88690e7966e2f16c97006e44327797119894

glandium, what do you think? What would you like to see here?
Please rebase on current central and do another try. Your try is still using expandlibs, which is gone.
Attachment #8963456 - Flags: review?(mh+mozilla)
(In reply to Tom Ritter [:tjr] from comment #11)
> Seems to be good!
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=87baf25efb64129e39df688cfa6933dfc30b600e

This try didn't have mingw builds, so I triggered them and they are busted wrt --disable-stylo.
(In reply to Mike Hommey [:glandium] from comment #13)
> (In reply to Tom Ritter [:tjr] from comment #11)
> > Seems to be good!
> > https://treeherder.mozilla.org/#/
> > jobs?repo=try&revision=87baf25efb64129e39df688cfa6933dfc30b600e
> 
> This try didn't have mingw builds, so I triggered them and they are busted
> wrt --disable-stylo.

Yes, the success try run for MinGW was earlier; and is off an older tree before --disable-stylo was removed. Once --disable-stylo was removed we changed it so MinGW doesn't run unless you explicitly request it.
Can you link to a mingw try with the latest patch, then?
This is a build with this patch (+ other MinGW x64 patches) and the backported Bug 1429875

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b12e42d280011ac6c10ae4fb0a1b3ad9c4a15b29
Comment on attachment 8963456 [details]
Bug 1448749 Resolve undefined references to '_imp__mozalloc_abort' in the MinGW build

https://reviewboard.mozilla.org/r/232378/#review240856

::: memory/mozalloc/mozalloc_abort.cpp:22
(Diff revision 3)
>  #include <stdio.h>
>  
>  #include "mozilla/Assertions.h"
>  #include "mozilla/Sprintf.h"
>  
> +extern "C"

This is not needed in the .cpp, considering the .h is included.

::: modules/pdfium/moz.build:567
(Diff revision 3)
>  GeckoSharedLibrary('pdfium', linkage=None)
>  
>  if CONFIG['OS_TARGET'] == 'WINNT':
>      DEFFILE = SRCDIR + '/pdfium.def'
> +
> +    if CONFIG['CC_TYPE'] == 'gcc' and CONFIG['CPU_ARCH'] == 'x86_64':

I'm not sure why you'd need that, but in your try build, pdfium was not built at all, so why not skip this?
Attachment #8963456 - Flags: review?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #17)
> ::: modules/pdfium/moz.build:567
> (Diff revision 3)
> >  GeckoSharedLibrary('pdfium', linkage=None)
> >  
> >  if CONFIG['OS_TARGET'] == 'WINNT':
> >      DEFFILE = SRCDIR + '/pdfium.def'
> > +
> > +    if CONFIG['CC_TYPE'] == 'gcc' and CONFIG['CPU_ARCH'] == 'x86_64':
> 
> I'm not sure why you'd need that, but in your try build, pdfium was not
> built at all, so why not skip this?

I don't have the faintest idea why pdfium didn't get built in the build you looked at. It normally does.
Example from today: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0458dd4c8991aa3feafd52deba0fb4a90d7f3f2d&selectedJob=173344260

I'm going another try run with everything to make sure removing the extern C from the .cpp is okay.

MinGW: https://treeherder.mozilla.org/#/jobs?repo=try&revision=18e1d13a1bee5b4ac33ad3787d09224c974f244c
Everything else:  https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8a7eb1b00e3ce8ee2a5639f12dc43bca0590f78
(In reply to Tom Ritter [:tjr] from comment #19)
> (In reply to Mike Hommey [:glandium] from comment #17)
> > ::: modules/pdfium/moz.build:567
> > (Diff revision 3)
> > >  GeckoSharedLibrary('pdfium', linkage=None)
> > >  
> > >  if CONFIG['OS_TARGET'] == 'WINNT':
> > >      DEFFILE = SRCDIR + '/pdfium.def'
> > > +
> > > +    if CONFIG['CC_TYPE'] == 'gcc' and CONFIG['CPU_ARCH'] == 'x86_64':
> > 
> > I'm not sure why you'd need that, but in your try build, pdfium was not
> > built at all, so why not skip this?
> 
> I don't have the faintest idea why pdfium didn't get built in the build you
> looked at. It normally does.
> Example from today:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=0458dd4c8991aa3feafd52deba0fb4a90d7f3f2d&selectedJob=1
> 73344260

That push is based on a mozilla-central revision that is > 1 month old, and predates bug 1435230, which removed skia pdf on windows.
(In reply to Mike Hommey [:glandium] from comment #20)
> That push is based on a mozilla-central revision that is > 1 month old, and
> predates bug 1435230, which removed skia pdf on windows.

Ah okay. I have a distant hope of uplifting the MinGW x64 build patches to 60 ESR for Tor; so I'll let this sit for now until either or both of being ready for that or being ready to turn on MinGW x64 in -central.
skia pdf is not riding the trains, so it was already disabled on non-nightly. IOW, you don't need to patch pdfium whether for uplifts or not.
Comment on attachment 8963456 [details]
Bug 1448749 Resolve undefined references to '_imp__mozalloc_abort' in the MinGW build

Resetting per comment 22.
Attachment #8963456 - Flags: review?(mh+mozilla)
So it turns out this is not just related to pdfium. With pdfium not riding the trains; we don't need that component of the patch, but we do still need the extern "C".

Build showing the MinGW failures without this: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b332ee10f7fe9095b4da50ce9f0a263157be9a6e&selectedJob=178857099

Successful build with this: https://treeherder.mozilla.org/#/jobs?repo=try&revision=16b1ef34d77a23a0bd3599ab92c8ae4c99b2d890

Successful build of everything else on -central with this patch (and a few other unrelated ones): https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e33104c217d2bb2b4470a9087dd0b997ac7e4f9
Comment on attachment 8963456 [details]
Bug 1448749 Resolve undefined references to '_imp__mozalloc_abort' in the MinGW build

https://reviewboard.mozilla.org/r/232378/#review250590
Attachment #8963456 - Flags: review?(mh+mozilla) → review+
Keywords: checkin-needed
Comment on attachment 8963456 [details]
Bug 1448749 Resolve undefined references to '_imp__mozalloc_abort' in the MinGW build

[Approval Request Comment]

This is one of several MinGW Build patches I'd like to land in esr60 for Tor. It will prevent them from carrying their own patches for the lifetime of esr60 and will enable us to keep the MinGW build functioning and know if/when/how it was broken by new commits into esr60.

This commit does affect non-MinGW builds; I'm ensuring a try run is green. In general this change is pretty structural, so it's likely that if it broke anything we would know very quickly.

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e33104c217d2bb2b4470a9087dd0b997ac7e4f9
Attachment #8963456 - Flags: approval-mozilla-esr60?
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8c97493f2d4c
Resolve undefined references to '_imp__mozalloc_abort' in the MinGW build r=glandium
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8c97493f2d4c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Comment on attachment 8963456 [details]
Bug 1448749 Resolve undefined references to '_imp__mozalloc_abort' in the MinGW build

mingw build fix, approved for 60.1esr
Attachment #8963456 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Version: Version 3 → 3 Branch
See Also: → 1548624
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: