On Mon, May 2, 2022 at 11:36 AM Richard Biener <richard.guent...@gmail.com> wrote: > > On Mon, May 2, 2022 at 11:27 AM Martin Liška <mli...@suse.cz> wrote: > > > > On 5/2/22 11:14, Richard Biener wrote: > > > On Mon, May 2, 2022 at 11:08 AM Martin Liška <mli...@suse.cz> wrote: > > >> > > >> On 5/2/22 11:03, Richard Biener wrote: > > >>> In fact I think GCC is correct in its handling and LLVM would need to > > >>> dup () the > > >>> file descriptor. Was this issue raised with the LLVM folks? > > >> > > >> I don't know here. > > >> > > >>> I suspect the BFD > > >>> linker will also not work with LLVM due to this? > > >> > > >> I'm not sure BFD is supported by LLVM: > > >> https://llvm.org/docs/GoldPlugin.html > > >> > > >> They explicitly name it Gold plugin. > > > > > > BFD does > > > > > > if (plugin_call_claim_file (&file, &claimed)) > > > einfo (_("%F%P: %s: plugin reported error claiming file\n"), > > > plugin_error_plugin ()); > > > > > > if (input->fd != -1 > > > && (!claimed || !bfd_plugin_target_p (ibfd->xvec))) > > > { > > > /* FIXME: fd belongs to us, not the plugin. GCC plugin, which > > > doesn't need fd after plugin_call_claim_file, doesn't use > > > BFD plugin target vector. Since GCC plugin doesn't call > > > release_input_file, we close it here. LLVM plugin, which > > > needs fd after plugin_call_claim_file and calls > > > release_input_file after it is done, uses BFD plugin target > > > vector. This scheme doesn't work when a plugin needs fd and > > > doesn't use BFD plugin target vector neither. */ > > > release_plugin_file_descriptor (input); > > > } > > > > > > so it knows about the problem. The above suggests that the plugin > > > could set ->fd to -1 ... > > > > That would work. So we can add LDPT_REGISTER_CLAIM_FILE_HOOK_V2 > > hook that will release fd based on if claim_file_v2 resets it to -1 or not. > > > > Does it worth implementing? > > I'd say the folks to ask are the binutils, mold and lld linker plus of course > LLVM which would need to adjust its handling. It would be also possible > to add a int *claimed_fd argument or just explicitely document that > the file descriptor belongs to the linker and thus just LLVM needs to be > fixed to dup() it when it stores it away. > > Linkers will need heuristics anyway for older plugins and I think > LLVM could just behave here.
Btw, binutils f4b78d1898203363e7f551497b6231d0f891d6f9 more accurately outlines the issue (though I admit I don't fully understand the difference) and it hints at the linkers more accurately need to track where the FD was obtained through. Richard. > Richard. > > > > > Cheers, > > Martin > > > > > > > > Richard. > > > > > >> > > >> Martin > > >> > >
