search

mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.65k stars 1.64k forks source link

After reconfiguring old object files are still linked in #9414

Open lb90 opened 3 years ago

lb90 commented 3 years ago

Describe the bug

After removing a source file in the Meson build definition, Meson reconfigures and then that source file is not compiled, but the corresponding object file (available from previous compilations) is still passed as input to the linker.

To Reproduce I have created a very simple branch from the master branch that adds a source file to GdkWin32. That source file uses a function from cabinet.dll so it adds a new dependency on a system DLL.

So I can run Meson and build that branch just fine. But then if I checkout the master branch and try a rebuild I get:

Build targets in project: 114
gtk 4.5.0
Option buildtype is: debug [default: debugoptimized]
Found ninja-1.10.2 at D:\msys64\mingw64\bin/ninja.EXE
[1/41] Generating demo-header with a custom command (wrapped by meson to capture output)
[2/36] Compiling C object gdk/win32/libgdk-win32.a.p/gdkdisplay-win32.c.obj
[3/36] Linking static target gdk/win32/libgdk-win32.a
[4/36] Linking static target gdk/libgdk.a
[5/36] Linking target tools/gtk4-update-icon-cache.exe
[6/36] Linking target gtk/libgtk-4-1.dll
FAILED: gtk/libgtk-4-1.dll
"cc"  -o gtk/libgtk-4-1.dll  "-Wl,--allow-shlib-undefined" "-shared" "-Wl,--start-group" "-Wl,--out-implib=gtk/libgtk-4.dll.a" "-Wl,--whole-archive" "gtk/libgtk.a" "gtk/css/libgtk_css.a" "gdk/libgdk.a" "gsk/libgsk.a" "-Wl,--no-whole-archive" "gdk/win32/libgdk-win32.a" "gsk/libgsk_f16c.a" "-Wl,-Bsymbolic" "D:/msys64/mingw64/lib/libgmodule-2.0.dll.a" "-pthread" "D:/msys64/mingw64/lib/libglib-2.0.dll.a" "D:/msys64/mingw64/lib/libintl.dll.a" "D:/msys64/mingw64/lib/libgobject-2.0.dll.a" "D:/msys64/mingw64/lib/libgio-2.0.dll.a" "D:/msys64/mingw64/lib/libpangocairo-1.0.dll.a" "D:/msys64/mingw64/lib/libpango-1.0.dll.a" "D:/msys64/mingw64/lib/libharfbuzz.dll.a" "D:/msys64/mingw64/lib/libcairo.dll.a" "D:/msys64/mingw64/lib/libfribidi.dll.a" "D:/msys64/mingw64/lib/libcairo-gobject.dll.a" "D:/msys64/mingw64/lib/libgdk_pixbuf-2.0.dll.a" "D:/msys64/mingw64/lib/libepoxy.dll.a" "-lm" "D:/msys64/mingw64/lib/libgraphene-1.0.dll.a" "D:/msys64/mingw64/lib/libpangoft2-1.0.dll.a" "D:/msys64/mingw64/lib/libfontconfig.dll.a" "D:/msys64/mingw64/lib/libfreetype.dll.a" "D:/msys64/mingw64/lib/libpangowin32-1.0.dll.a" "-ladvapi32" "-lcomctl32" "-lcrypt32" "-ldwmapi" "-limm32" "-lsetupapi" "-lwinmm" "-lintl" "D:/msys64/mingw64/lib/libpng16.dll.a" "D:/msys64/mingw64/lib/libz.dll.a" "D:/msys64/mingw64/lib/libtiff.dll.a" "D:/msys64/mingw64/lib/libjpeg.dll.a" "-lhid" "-lcairo-script-interpreter" "-ladvapi32" "-lcomctl32" "-lcrypt32" "-ldwmapi" "-limm32" "-lsetupapi" "-lwinmm" "-lintl" "-lhid" "-lcairo-script-interpreter" "-lhid" "-lhid" "-lcairo-script-interpreter" "-lhid" "-lcairo-script-interpreter" "-lhid" "-lkernel32" "-luser32" "-lgdi32" "-lwinspool" "-lshell32" "-lole32" "-loleaut32" "-luuid" "-lcomdlg32" "-Wl,--end-group"
D:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: gdk/libgdk.a(gdkfoo-win32.c.obj): in function `gdk_foo_win32_do_work':
D:\gtk4-test-build/../gtk4/gdk/win32/gdkfoo-win32.c:9: undefined reference to `FCICreate'
D:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: D:\gtk4-test-build/../gtk4/gdk/win32/gdkfoo-win32.c:15: undefined reference to `FCIDestroy'
collect2.exe: error: ld returned 1 exit status
[7/36] Linking target tools/gtk4-encode-symbolic-svg.exe
[8/36] Linking target gtk/compose/compose-parse.exe
ninja: build stopped: subcommand failed.

But gdkfoo-win32.c does not exist at all in the master branch!

Links

Step by step

  1. git clone https://gitlab.gnome.org/lb90/gtk.git --branch new-feature
  2. mkdir gtk-build && cd gtk-build
  3. meson ../gtk --buildtype=debug -Dintrospection=disabled -Dbuild-tests=false && ninja
  4. (Everythin builds) Now checkout the master branch
  5. Run ninja again

Expected behavior The master branch is built just fine

What happens Building the libgdk-3-0.dll target fails at the link stage because gdkfoo-win32.c.o is still linked-in, though there's no reference of it anymore in the meson.build file.

system parameters

lb90 commented 3 years ago

I suspect it has to do with the ninja_deps file...

lb90 commented 3 years ago

The build rule for libgdk-win32.a in build.ninja is:

build gdk/win32/libgdk-win32.a: STATIC_LINKER
  gdk/win32/libgdk-win32.a.p/gdkcairocontext-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkcursor-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkclipboard-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkclipdrop-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkdevicemanager-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkdevice-virtual.c.obj
  gdk/win32/libgdk-win32.a.p/gdkdevice-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkdevice-winpointer.c.obj
  gdk/win32/libgdk-win32.a.p/gdkdevice-wintab.c.obj
  gdk/win32/libgdk-win32.a.p/gdkdisplay-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkdisplaymanager-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkdrag-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkdrop-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkevents-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkglcontext-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkglcontext-win32-wgl.c.obj
  gdk/win32/libgdk-win32.a.p/gdkglobals-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkhdataoutputstream-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkinput-winpointer.c.obj
  gdk/win32/libgdk-win32.a.p/gdkkeys-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkwin32langnotification.c.obj
  gdk/win32/libgdk-win32.a.p/gdkmain-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkmonitor-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkproperty-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkscreen-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkvulkancontext-win32.c.obj
  gdk/win32/libgdk-win32.a.p/gdkwin32id.c.obj
  gdk/win32/libgdk-win32.a.p/gdksurface-win32.c.obj
  LINK_ARGS = "csrD"

STATIC_LINKER is:

# Rules for linking.

rule STATIC_LINKER
 command = "gcc-ar" $LINK_ARGS $out $in
 description = Linking static target $out

And here's the gcc-ar --help output describing the command line arguments:

$ gcc-ar --help
Usage: D:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/bin/ar.exe [emulation options] [-]{dmpqrstx}[abcDfilMNoOPsSTuvV] [--plugin <name>] [member-name] [count] archive-file file...
       D:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/bin/ar.exe -M [<mri-script]
 commands:
  d            - delete file(s) from the archive
  m[ab]        - move file(s) in the archive
  p            - print file(s) found in the archive
  q[f]         - quick append file(s) to the archive
  r[ab][f][u]  - replace existing or insert new file(s) into the archive
  s            - act as ranlib
  t[O][v]      - display contents of the archive
  x[o]         - extract file(s) from the archive
 command specific modifiers:
  [a]          - put file(s) after [member-name]
  [b]          - put file(s) before [member-name] (same as [i])
  [D]          - use zero for timestamps and uids/gids (default)
  [U]          - use actual timestamps and uids/gids
  [N]          - use instance [count] of name
  [f]          - truncate inserted file names
  [P]          - use full path names when matching
  [o]          - preserve original dates
  [O]          - display offsets of files in the archive
  [u]          - only replace files that are newer than current archive contents
 generic modifiers:
  [c]          - do not warn if the library had to be created
  [s]          - create an archive index (cf. ranlib)
  [l <text> ]  - specify the dependencies of this library
  [S]          - do not build a symbol table
  [T]          - make a thin archive
  [v]          - be verbose
  [V]          - display the version number
  @<file>      - read options from <file>
  --target=BFDNAME - specify the target object format as BFDNAME
  --output=DIRNAME - specify the output directory for extraction operations
  --record-libdeps=<text> - specify the dependencies of this library
 optional:
  --plugin <p> - load the specified plugin
 emulation options:
  No emulation specific options
D:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/bin/ar.exe: supported targets: pe-x86-64 pei-x86-64 pe-bigobj-x86-64 elf64-x86-64 elf64-l1om elf64-k1om pe-i386 pei-i386 elf32-i386 elf32-iamcu elf64-little elf64-big elf32-little elf32-big srec symbolsrec verilog tekhex binary ihex plugin
Report bugs to <https://www.sourceware.org/bugzilla/>

So object files are added (modified) in the archive, but existing object files in the archive are not deleted! I guess that's the problem. Maybe we should delete the archive on reconfigure?

eli-schwartz commented 3 years ago

That's fascinating. On my machine, this rule looks like this:

# Rules for linking.

rule STATIC_LINKER
 command = rm -f $out && gcc-ar $LINK_ARGS $out $in
 description = Linking static target $out

It turns out the problem is that this got explicitly disabled on Windows in #1527 due to #1517

lb90 commented 3 years ago

What about adding something like meson --internal rm <path>?

xclaesse commented 3 years ago

We could, but it's slower because it needs to load the Python interpreter each time. But it's better than something broken.

lb90 commented 3 years ago

Adding to this, we could use cmd to replace forward slashes with back slashes: set variable=$out&&del %variable:\=/%. See https://stackoverflow.com/a/23544993/12126234.

lb90 commented 3 years ago

However, I wouldn't rely too much on cmd.exe as it can be a source of obscure bugs or security issues. Consider a source file named like e.g. %PATH%\main.c

We have these problems on unix shells too.

lb90 commented 3 years ago

Regarding security, I think that Meson should ban special characters in file names: &, %, $, |, /, \, ', ", <, >, :, ; or even invisible characters

eli-schwartz commented 3 years ago

It's not a security issue in the general case, at least on unix the actual command line that gets run is single-quoted and thus treated as a string literal.

It's also a solvable problem on Windows too, e.g. python's subprocess module with shell=False has a security promise that you don't need to worry about shell injection vulnerabilities, since it will safely handle everything for you when given a list of command+argv. As for Meson itself, we should be safe there, as we implement this specially in a cross-platform way by using shlex.quote on unix and a custom implementation at mesonbuild.mesonlib.universal.quote_arg which should follow documented Microsoft practice.

That being said, this is a build system, it runs arbitrary commands arbitrarily, by design. I don't think allowing shell metacharacter injection is a security danger, merely a robustness danger.