Bug 30568 - --dependency-file includes temporary LTO files
Summary: --dependency-file includes temporary LTO files
Status: ASSIGNED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.38
: P2 normal
Target Milestone: ---
Assignee: Nick Clifton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-06-20 12:42 UTC by Nikita Popov
Modified: 2023-07-31 13:42 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
Proposed patch (830 bytes, patch)
2023-06-21 13:49 UTC, Nick Clifton
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Popov 2023-06-20 12:42:01 UTC
The --dependency-file output currently includes temporary files created by linker plugins.

CMake 3.27 started making use of this option, which means that those temporary files are now recorded as dependencies. Of course, the temporary files are removed after the linker run and as such considered dirty on the next build system invocation, resulting in all libraries and binaries being rebuilt, even if none of the inputs changed.

This can be reproduced as follows (where cc=gcc and cc=clang both exhibit the problem):

echo "int main() { return 0; }" > test.c
cc -flto -c test.c
cc -flto -Wl,--dependency-file=test.dep test.o

The resulting dependency file will include an entry for something like /tmp/cc2EGrdy.ltrans0.ltrans.o (gcc) or /tmp/lto-llvm-a63ee9.o (clang).

This also happens with -fuse-ld=gold, so this affects both ld.bfd and ld.gold.

I think the right fix for ld.bfd would be to guard the track_dependency_files() call in https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=ld/ldfile.c;h=4976367bbf03b09df6e5625a676acb309e1f30ea;hb=HEAD#l355 with a !entry->flags.lto_output check. Not sure about ld.gold.
Comment 1 Nick Clifton 2023-06-21 13:49:36 UTC
Created attachment 14940 [details]
Proposed patch

Hi Nikita,

  Please could you try out the uploaded patch and let me know what you think.

  I have a couple of concerns with the patch.  The first is that it uses a 
  simple heuristic to detect LTO generated filenames.  I have no idea how
  robust this will be.

  The second is that the patch strips the filenames from the command line that
  forms the first part of the dependency file, as well as the list of files
  that comes afterwards.  I ma not sure if this is the correct thing to do.
  Maybe the command line should remain unfiltered.

Cheers
  Nick
Comment 2 Nikita Popov 2023-06-27 08:42:20 UTC
Thanks for the patch!

> I have a couple of concerns with the patch.  The first is that it uses a 
> simple heuristic to detect LTO generated filenames.  I have no idea how
> robust this will be.

Yes, I agree. The checked sub-strings are pretty short, so it seems likely that there will be conflicts with real linker inputs. For example, LLVM has a tool called llvm-lto, so it seems pretty conceivable that "llvm-lto-" appears as part of a file name.

It would be preferable if we could exclude all files added by the add_input_file hook from the dependency tracking.

In the original report I suggested using the existing lto_output flag to distinguish this (for ld.bfd) -- is there any reason why that wouldn't work?

(Another possibility would be to check whether the file still exists when writing the dependency file -- but I'm not sure whether these files have already been deleted at that point, or whether that happens later.)

> The second is that the patch strips the filenames from the command line that
> forms the first part of the dependency file, as well as the list of files
> that comes afterwards.  I ma not sure if this is the correct thing to do.
> Maybe the command line should remain unfiltered.

This is the right thing to do: The part at the start isn't a command line, but rather the actual dependency list in Makefile format, so we do want to filter that.
Comment 3 Sourceware Commits 2023-06-28 12:50:40 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=b25c1a15cbac78e592d7a0c749dec2fcc175ec39

commit b25c1a15cbac78e592d7a0c749dec2fcc175ec39
Author: Nick Clifton <nickc@redhat.com>
Date:   Wed Jun 28 13:49:43 2023 +0100

    Stop the linker's --dependency-file option from including temporary lto files.
    
      PR 30568
      * ldfile.c (ldfile_try_open_bfd): Do not track lto generated temporary files.
Comment 4 Nick Clifton 2023-06-28 12:54:34 UTC
> In the original report I suggested using the existing lto_output flag to distinguish > this (for ld.bfd) -- is there any reason why that wouldn't work?

That makes much more sense.  Sorry for not reading *all* of your original bug report.

I checked in that patch, but I have not changed gold.  For gold I think that a lot more changes are necessary - recording object files that are added via the plugin and then skipping them when the dependency list is generated - and I do not feel comfortable doing this.  So I will leave it to a C++ programmer to have a go.
Comment 5 Jan-Benedict Glaw 2023-07-05 18:27:23 UTC
While this patch works for most targets, it breaks for --target=alpha-dec-vms and --target=alpha64-dec-vms (the latter is a bit of an odd one, but listed in config-list.mk nonetheless). See eg. http://toolchain.lug-owl.de/laminar/jobs/binutils-alpha-dec-vms/39 :

[all 2023-07-05 16:20:23] /var/lib/laminar/run/binutils-alpha-dec-vms/39/local-toolchain-install/bin/gcc -DHAVE_CONFIG_H -I.  -I. -I. -I../bfd -I./../bfd -I./../include -I./../zlib  -g -O2   -DLOCALEDIR="\"/tmp/binutils-alpha-dec-vms/share/locale\""  -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Wstack-usage=262144 -Werror -DELF_LIST_OPTIONS=false -DELF_SHLIB_LIST_OPTIONS=false -DELF_PLT_UNWIND_LIST_OPTIONS=false   -g -O2   -MT ldfile.o -MD -MP -MF .deps/ldfile.Tpo -c -o ldfile.o \
[all 2023-07-05 16:20:23] -DSCRIPTDIR='"/tmp/binutils-alpha-dec-vms/alpha-dec-vms/lib"' -DBINDIR='"/tmp/binutils-alpha-dec-vms/bin"' -DTOOLBINDIR='"/tmp/binutils-alpha-dec-vms/alpha-dec-vms/bin"' \
[all 2023-07-05 16:20:23]  ./ldfile.c
[all 2023-07-05 16:20:24] ./ldfile.c: In function 'ldfile_try_open_bfd':
[all 2023-07-05 16:20:24] ./ldfile.c:356:20: error: 'struct lang_input_statement_flags' has no member named 'lto_output'
[all 2023-07-05 16:20:24]   356 |   if (!entry->flags.lto_output)
[all 2023-07-05 16:20:24]       |                    ^
[all 2023-07-05 16:20:24] make[3]: *** [Makefile:2311: ldfile.o] Error 1
[all 2023-07-05 16:20:24] make[3]: Leaving directory '/var/lib/laminar/run/binutils-alpha-dec-vms/39/binutils-gdb/ld'
[all 2023-07-05 16:20:24] make[2]: *** [Makefile:1881: all-recursive] Error 1
[all 2023-07-05 16:20:24] make[2]: Leaving directory '/var/lib/laminar/run/binutils-alpha-dec-vms/39/binutils-gdb/ld'
[all 2023-07-05 16:20:24] make[1]: *** [Makefile:1077: all] Error 2
[all 2023-07-05 16:20:24] make[1]: Leaving directory '/var/lib/laminar/run/binutils-alpha-dec-vms/39/binutils-gdb/ld'
[all 2023-07-05 16:20:24] make: *** [Makefile:8162: all-ld] Error 2



So this needs a `#if BFD_SUPPORTS_PLUGINS` (cf. ldlang.h)
Comment 6 Jan-Benedict Glaw 2023-07-05 19:10:36 UTC
Slowly showing up on other builds as well: i686-pc-msdosdjgpp (http://toolchain.lug-owl.de/laminar/jobs/binutils-i686-pc-msdosdjgpp/38)
Comment 7 Sourceware Commits 2023-07-06 01:31:24 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=5f60df9974516867c02562b56c3a98cf4714a915

commit 5f60df9974516867c02562b56c3a98cf4714a915
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Jul 5 21:33:01 2023 +0930

    Re: Stop the linker's --dependency-file option from including temporary lto files.
    
            PR 30568
            * ldfile.c (ldfile_try_open_bfd): Fix build failure when
            !BFD_SUPPORTS_PLUGINS.
Comment 8 Jan-Benedict Glaw 2023-07-06 20:03:45 UTC
That fixes the intermediate build issue, thanks.
Comment 9 Sourceware Commits 2023-07-12 01:15:55 UTC
The binutils-2_41-branch branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=74e20e445b358c2f6884ab29cd832561a178e646

commit 74e20e445b358c2f6884ab29cd832561a178e646
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Jul 5 21:33:01 2023 +0930

    Re: Stop the linker's --dependency-file option from including temporary lto files.
    
            PR 30568
            * ldfile.c (ldfile_try_open_bfd): Fix build failure when
            !BFD_SUPPORTS_PLUGINS.
    
    (cherry picked from commit 5f60df9974516867c02562b56c3a98cf4714a915)