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.

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