On Mon, May 2, 2022 at 11:00 AM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Mon, May 2, 2022 at 10:57 AM Martin Liška <mli...@suse.cz> wrote:
> >
> > On 5/2/22 10:51, Richard Biener wrote:
> > > On Mon, May 2, 2022 at 10:19 AM Martin Liška <mli...@suse.cz> wrote:
> > >>
> > >> On 5/2/22 10:09, Richard Biener wrote:
> > >>> On Mon, May 2, 2022 at 9:52 AM Martin Liška <mli...@suse.cz> wrote:
> > >>>>
> > >>>> Hi.
> > >>>>
> > >>>> This in a new plug-in function that helps identifying compiler
> > >>>> by a linker. Will be used in mold linker.
> > >>>>
> > >>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> > >>>>
> > >>>> Ready to be installed?
> > >>>
> > >>> It looks a bit backward to query the compiler (and even more so to
> > >>> have a fixed set) from the linker.  What is this going to be used for?
> > >>
> > >> It's going to be used by mold, there are small behavior divergence
> > >> in between LLVM and GCC, where one compiler closes provided file 
> > >> descriptors:
> > >> https://github.com/rui314/mold/blob/a15f3875269b575d024725590ae3ef87b611b8d8/elf/lto.cc#L545-L550
> > >
> > > But that then seems to be a defect in the plugin API specification (or
> > > one of the two implementations).  Instead of adding a new API entry
> > > that defect should be fixed - both require changes in the actual plugin
> > > specification.
> >
> > Well, the question is if it was specified somewhere or not. Plus there are 
> > released
> > versions of both compilers and plug-ins, so it's not easily fixable :/
>
> Well, without changing plug-ins the new version callback doesn't exist
> either.  The
> problem of existing discrepancies is not fixable without heuristics in
> the linker,
> the _actual_ problem can of course be fixed but the solution is of course 
> _not_
> to make if (gcc) or if (llvm) but if then something like if (plugin
> closed fd) or
> specify the plugin has to or has not to close fds passed in
> ld_plugin_input_file.

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 suspect the BFD
linker will also not work with LLVM due to this?

Richard.

> Richard.
>
> > Martin
> >
> > >
> > > Richard.
> > >
> > >>> If then this should probably receive a unformatted string the linker
> > >>> has to decipher itself like "gcc (SUSE Linux) 7.5.0" or something
> > >>> (what we produce with gcc --version), and clang would produce
> > >>> "clang version 11.0.1" or so?
> > >>
> > >> Well, it brings its own set of problems such string parsing. Note that 
> > >> the plugin API
> > >> currently provides something like:
> > >>
> > >> /* The version of gold being used, or -1 if not gold.  The number is
> > >>    MAJOR * 100 + MINOR.  */
> > >> static int gold_version = -1;
> > >>
> > >> So my suggestion is quite aligned with that.
> > >>
> > >> Martin
> > >>
> > >>>
> > >>> Richard.
> > >>>
> > >>>> Thanks,
> > >>>> Martin
> > >>>>
> > >>>> include/ChangeLog:
> > >>>>
> > >>>>         * plugin-api.h (enum ld_plugin_compiler): New enum.
> > >>>>         (enum ld_plugin_version): New typedef.
> > >>>>         (struct ld_plugin_tv): Add new field.
> > >>>>
> > >>>> lto-plugin/ChangeLog:
> > >>>>
> > >>>>         * lto-plugin.c (onload): Call ld_plugin_version.
> > >>>> ---
> > >>>>  include/plugin-api.h    | 18 +++++++++++++++++-
> > >>>>  lto-plugin/lto-plugin.c |  4 ++++
> > >>>>  2 files changed, 21 insertions(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/include/plugin-api.h b/include/plugin-api.h
> > >>>> index 4e12c0320d6..4d0989028cc 100644
> > >>>> --- a/include/plugin-api.h
> > >>>> +++ b/include/plugin-api.h
> > >>>> @@ -211,6 +211,13 @@ enum ld_plugin_symbol_section_kind
> > >>>>    LDSSK_BSS
> > >>>>  };
> > >>>>
> > >>>> +enum ld_plugin_compiler
> > >>>> +{
> > >>>> +  LDPC_UNKNOWN,
> > >>>> +  LDPC_GCC,
> > >>>> +  LDPC_LLVM
> > >>>> +};
> > >>>> +
> > >>>>  /* How a symbol is resolved.  */
> > >>>>
> > >>>>  enum ld_plugin_symbol_resolution
> > >>>> @@ -475,6 +482,13 @@ enum ld_plugin_status
> > >>>>  (*ld_plugin_get_wrap_symbols) (uint64_t *num_symbols,
> > >>>>                                 const char ***wrap_symbol_list);
> > >>>>
> > >>>> +/* The linker's interface for registering the "plugin_version" 
> > >>>> handler.
> > >>>> +   This handler is called directly after onload and provides compiler
> > >>>> +   and its number version (MAJOR * 1000 + MINOR).  */
> > >>>> +
> > >>>> +typedef
> > >>>> +void (*ld_plugin_version) (enum ld_plugin_compiler compiler, int 
> > >>>> version);
> > >>>> +
> > >>>>  enum ld_plugin_level
> > >>>>  {
> > >>>>    LDPL_INFO,
> > >>>> @@ -520,7 +534,8 @@ enum ld_plugin_tag
> > >>>>    LDPT_GET_INPUT_SECTION_SIZE = 30,
> > >>>>    LDPT_REGISTER_NEW_INPUT_HOOK = 31,
> > >>>>    LDPT_GET_WRAP_SYMBOLS = 32,
> > >>>> -  LDPT_ADD_SYMBOLS_V2 = 33
> > >>>> +  LDPT_ADD_SYMBOLS_V2 = 33,
> > >>>> +  LDPT_PLUGIN_VERSION = 34,
> > >>>>  };
> > >>>>
> > >>>>  /* The plugin transfer vector.  */
> > >>>> @@ -556,6 +571,7 @@ struct ld_plugin_tv
> > >>>>      ld_plugin_get_input_section_size tv_get_input_section_size;
> > >>>>      ld_plugin_register_new_input tv_register_new_input;
> > >>>>      ld_plugin_get_wrap_symbols tv_get_wrap_symbols;
> > >>>> +    ld_plugin_version tv_plugin_version;
> > >>>>    } tv_u;
> > >>>>  };
> > >>>>
> > >>>> diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
> > >>>> index 7a1c77812f6..698a0617ca7 100644
> > >>>> --- a/lto-plugin/lto-plugin.c
> > >>>> +++ b/lto-plugin/lto-plugin.c
> > >>>> @@ -69,6 +69,7 @@ along with this program; see the file COPYING3.  If 
> > >>>> not see
> > >>>>  #include "../gcc/lto/common.h"
> > >>>>  #include "simple-object.h"
> > >>>>  #include "plugin-api.h"
> > >>>> +#include "ansidecl.h"
> > >>>>
> > >>>>  /* We need to use I64 instead of ll width-specifier on native Windows.
> > >>>>     The reason for this is that older MS-runtimes don't support the 
> > >>>> ll.  */
> > >>>> @@ -1466,6 +1467,9 @@ onload (struct ld_plugin_tv *tv)
> > >>>>           /* We only use this to make user-friendly temp file names.  
> > >>>> */
> > >>>>           link_output_name = p->tv_u.tv_string;
> > >>>>           break;
> > >>>> +       case LDPT_PLUGIN_VERSION:
> > >>>> +         p->tv_u.tv_plugin_version (LDPC_GCC, GCC_VERSION);
> > >>>> +         break;
> > >>>>         default:
> > >>>>           break;
> > >>>>         }
> > >>>> --
> > >>>> 2.36.0
> > >>>>
> > >>
> >

Reply via email to