On 5/2/22 11:00, Richard Biener 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
Yes, feature querying would be much better approach.
> specify the plugin has to or has not to close fds passed in
> ld_plugin_input_file.
Anyway, I don't want to over-engineer things here and I think the mold
linker can live with some heuristics.
That said, I don't insist on this patch.
Martin
>
> 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
>>>>>>
>>>>
>>
