On 6/16/22 10:00, Alexander Monakov wrote:
> On Thu, 16 Jun 2022, Martin Liška wrote:
>
>> Hi.
>>
>> I'm sending updated version of the patch where I addressed the comments.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
>
> I noticed a typo (no objection on the substance on the patch from me):
Good!
>
>> --- a/include/plugin-api.h
>> +++ b/include/plugin-api.h
>> @@ -483,6 +483,34 @@ enum ld_plugin_level
>> LDPL_FATAL
>> };
>>
>> +/* Contract between a plug-in and a linker. */
>> +
>> +enum linker_api_version
>> +{
>> + /* The linker/plugin do not implement any of the API levels below, the
>> API
>> + is determined solely via the transfer vector. */
>> + LAPI_UNSPECIFIED = 0,
>> +
>> + /* API level v1. The linker provides add_symbols_v3, add_symbols_v2,
>
> This should be '*get_*symbols_v3, add_symbols_v2'.
Sure, fixed.
>
>> + the plugin will use that and not any lower versions.
>> + claim_file is thread-safe on the plugin side and
>> + add_symbols on the linker side. */
>> + LAPI_V1 = 1
>> +};
>> +
>> +/* The linker's interface for API version negotiation. A plug-in calls
>> + the function (with its IDENTIFIER and VERSION), plus minimal and maximal
>> + version of linker_api_version is provided. Linker then returns selected
>> + API version and provides its IDENTIFIER and VERSION. */
>> +
>> +typedef
>> +enum linker_api_version
>> +(*ld_plugin_get_api_version) (const char *plugin_identifier, unsigned
>> plugin_version,
>> + enum linker_api_version minimal_api_supported,
>> + enum linker_api_version maximal_api_supported,
>> + const char **linker_identifier,
>> + unsigned *linker_version);
>
> IIRC Richi asked to mention which side owns the strings (does the receiver
> need
> to 'free' or 'strdup' them). Perhaps we could say they are owned by the
> originating side, but it might be even better to say they are unchanging to
> allow simply using string literals. Perhaps add something like this to the
> comment?
>
> Identifier pointers remain valid as long as the plugin is loaded.
I welcome the change and I'm sending a patch that incorporates that.
>
>> /* Values for the tv_tag field of the transfer vector. */
>>
>> enum ld_plugin_tag
>> @@ -521,6 +549,7 @@ enum ld_plugin_tag
>> LDPT_REGISTER_NEW_INPUT_HOOK,
>> LDPT_GET_WRAP_SYMBOLS,
>> LDPT_ADD_SYMBOLS_V2,
>> + LDPT_GET_API_VERSION,
>> };
>
> I went checking if this is in sync with Binutils header and noticed that
> get_wrap_symbols and add_symbols_v2 are not even mentioned on the wiki page
> with
> plugin API documentation.
Yes, I know about that. I'm going to update wiki page once we get this in.
Cheers,
Martin
>
> Alexander
From 3e01823f22282c7a46771c82f9622f7ff2929b18 Mon Sep 17 00:00:00 2001
From: Martin Liska <[email protected]>
Date: Mon, 16 May 2022 14:01:52 +0200
Subject: [PATCH] lto-plugin: implement LDPT_GET_API_VERSION
include/ChangeLog:
* plugin-api.h (enum linker_api_version): New enum.
(ld_plugin_get_api_version): New.
(enum ld_plugin_tag): Add LDPT_GET_API_VERSION.
(struct ld_plugin_tv): Add tv_get_api_version.
lto-plugin/ChangeLog:
* lto-plugin.c (negotiate_api_version): New.
(onload): Negotiate API version.
---
include/plugin-api.h | 31 +++++++++++++++++++++++++++++++
lto-plugin/lto-plugin.c | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 69 insertions(+)
diff --git a/include/plugin-api.h b/include/plugin-api.h
index 8aebe2ff267..cc3a43b44a6 100644
--- a/include/plugin-api.h
+++ b/include/plugin-api.h
@@ -483,6 +483,35 @@ enum ld_plugin_level
LDPL_FATAL
};
+/* Contract between a plug-in and a linker. */
+
+enum linker_api_version
+{
+ /* The linker/plugin do not implement any of the API levels below, the API
+ is determined solely via the transfer vector. */
+ LAPI_UNSPECIFIED = 0,
+
+ /* API level v1. The linker provides get_symbols_v3, add_symbols_v2,
+ the plugin will use that and not any lower versions.
+ claim_file is thread-safe on the plugin side and
+ add_symbols on the linker side. */
+ LAPI_V1 = 1
+};
+
+/* The linker's interface for API version negotiation. A plug-in calls
+ the function (with its IDENTIFIER and VERSION), plus minimal and maximal
+ version of linker_api_version is provided. Linker then returns selected
+ API version and provides its IDENTIFIER and VERSION.
+ Identifier pointers remain valid as long as the plugin is loaded. */
+
+typedef
+enum linker_api_version
+(*ld_plugin_get_api_version) (const char *plugin_identifier, unsigned plugin_version,
+ enum linker_api_version minimal_api_supported,
+ enum linker_api_version maximal_api_supported,
+ const char **linker_identifier,
+ unsigned *linker_version);
+
/* Values for the tv_tag field of the transfer vector. */
enum ld_plugin_tag
@@ -521,6 +550,7 @@ enum ld_plugin_tag
LDPT_REGISTER_NEW_INPUT_HOOK,
LDPT_GET_WRAP_SYMBOLS,
LDPT_ADD_SYMBOLS_V2,
+ LDPT_GET_API_VERSION,
};
/* The plugin transfer vector. */
@@ -556,6 +586,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_get_api_version tv_get_api_version;
} tv_u;
};
diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
index 13118c4983c..208b26d5c4b 100644
--- a/lto-plugin/lto-plugin.c
+++ b/lto-plugin/lto-plugin.c
@@ -70,6 +70,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. */
@@ -170,6 +171,10 @@ static ld_plugin_add_input_file add_input_file;
static ld_plugin_add_input_library add_input_library;
static ld_plugin_message message;
static ld_plugin_add_symbols add_symbols, add_symbols_v2;
+static ld_plugin_get_api_version get_api_version;
+
+/* By default, use version LAPI_UNSPECIFIED if there is not negotiation. */
+static enum linker_api_version api_version = LAPI_UNSPECIFIED;
static struct plugin_file_info *claimed_files = NULL;
static unsigned int num_claimed_files = 0;
@@ -1415,6 +1420,33 @@ process_option (const char *option)
verbose = verbose || debug;
}
+/* Negotiate linker API version. */
+
+static void
+negotiate_api_version (void)
+{
+ const char *linker_identifier;
+ unsigned linker_version;
+
+ api_version = get_api_version ("GCC", GCC_VERSION, LAPI_UNSPECIFIED,
+ LAPI_V1, &linker_identifier, &linker_version);
+
+ switch (api_version)
+ {
+ case LAPI_UNSPECIFIED:
+ break;
+ case LAPI_V1:
+ check (get_symbols_v3, LDPL_FATAL,
+ "get_symbols_v3 required for API version 1");
+ check (add_symbols_v2, LDPL_FATAL,
+ "add_symbols_v2 required for API version 1");
+ break;
+ default:
+ fprintf (stderr, "unsupported API version\n");
+ abort ();
+ }
+}
+
/* Called by a linker after loading the plugin. TV is the transfer vector. */
enum ld_plugin_status
@@ -1481,12 +1513,18 @@ 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_GET_API_VERSION:
+ get_api_version = p->tv_u.tv_get_api_version;
+ break;
default:
break;
}
p++;
}
+ if (get_api_version)
+ negotiate_api_version ();
+
check (register_claim_file, LDPL_FATAL, "register_claim_file not found");
check (add_symbols, LDPL_FATAL, "add_symbols not found");
status = register_claim_file (claim_file_handler);
--
2.36.1
