* [PATCH 1/3] Change 'v1' float and int code to fall back to v0
2024-02-27 3:12 [PATCH 0/3] Fix libcc1 failure Tom Tromey
@ 2024-02-27 3:12 ` Tom Tromey
2024-02-27 15:14 ` Jeff Law
2024-02-27 3:12 ` [PATCH 2/3] Fix version negotiation in libcc1 plugins Tom Tromey
2024-02-27 3:12 ` [PATCH 3/3] Fix PR libcc1/113977 Tom Tromey
2 siblings, 1 reply; 11+ messages in thread
From: Tom Tromey @ 2024-02-27 3:12 UTC (permalink / raw)
To: gcc-patches
While working on another patch, I discovered that the libcc1 plugin
code never did version negotiation correctly. So, the patches to
introduce v1 never did anything -- the new code, as far as I know, has
never been run.
Making version negotiation work shows that the existing code causes
crashes. For example, safe_lookup_builtin_type might return
error_mark_node in some cases, which the callers aren't prepared to
accept.
Looking into it some more, I couldn't find any justification for this
v1 code for the C compiler plugin. Since it's not run at all, it's
also clear that removing it doesn't cause any regressions in gdb.
However, rather than remove it, this patch changes it to handle
ERROR_MARK better, and then to fall back to the v0 code if the new
code fails to find the type it's looking for.
2024-02-26 Tom Tromey <tom@tromey.com>
* libcc1plugin.cc (safe_lookup_builtin_type): Handle ERROR_MARK.
(plugin_int_type): Fall back to plugin_int_type_v0.
(plugin_float_type): Fall back to plugin_float_type_v0.
---
libcc1/ChangeLog | 6 ++++++
libcc1/libcc1plugin.cc | 19 ++++++++++---------
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/libcc1/ChangeLog b/libcc1/ChangeLog
index f81fe389e71..b0b31ee6586 100644
--- a/libcc1/ChangeLog
+++ b/libcc1/ChangeLog
@@ -1,3 +1,9 @@
+2024-02-26 Tom Tromey <tom@tromey.com>
+
+ * libcc1plugin.cc (safe_lookup_builtin_type): Handle ERROR_MARK.
+ (plugin_int_type): Fall back to plugin_int_type_v0.
+ (plugin_float_type): Fall back to plugin_float_type_v0.
+
2024-02-09 Marek Polacek <polacek@redhat.com>
PR c++/98388
diff --git a/libcc1/libcc1plugin.cc b/libcc1/libcc1plugin.cc
index 00d3963029d..f1082d8e9d3 100644
--- a/libcc1/libcc1plugin.cc
+++ b/libcc1/libcc1plugin.cc
@@ -555,7 +555,7 @@ safe_lookup_builtin_type (const char *builtin_name)
gcc_assert (TREE_CODE (result) == TYPE_DECL);
result = TREE_TYPE (result);
- return result;
+ return TREE_CODE (result) == ERROR_MARK ? nullptr : result;
}
static gcc_type
@@ -592,13 +592,14 @@ plugin_int_type (cc1_plugin::connection *self,
int is_unsigned, unsigned long size_in_bytes,
const char *builtin_name)
{
- if (!builtin_name)
- return plugin_int_type_v0 (self, is_unsigned, size_in_bytes);
-
- tree result = safe_lookup_builtin_type (builtin_name);
- gcc_assert (!result || TREE_CODE (result) == INTEGER_TYPE);
-
- return plugin_int_check (self, is_unsigned, size_in_bytes, result);
+ if (builtin_name != nullptr)
+ {
+ tree result = safe_lookup_builtin_type (builtin_name);
+ gcc_assert (!result || TREE_CODE (result) == INTEGER_TYPE);
+ if (result != nullptr)
+ return plugin_int_check (self, is_unsigned, size_in_bytes, result);
+ }
+ return plugin_int_type_v0 (self, is_unsigned, size_in_bytes);
}
gcc_type
@@ -631,7 +632,7 @@ plugin_float_type (cc1_plugin::connection *self,
tree result = safe_lookup_builtin_type (builtin_name);
if (!result)
- return convert_out (error_mark_node);
+ return plugin_float_type_v0 (self, size_in_bytes);
gcc_assert (SCALAR_FLOAT_TYPE_P (result));
gcc_assert (BITS_PER_UNIT * size_in_bytes == TYPE_PRECISION (result));
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] Fix version negotiation in libcc1 plugins
2024-02-27 3:12 [PATCH 0/3] Fix libcc1 failure Tom Tromey
2024-02-27 3:12 ` [PATCH 1/3] Change 'v1' float and int code to fall back to v0 Tom Tromey
@ 2024-02-27 3:12 ` Tom Tromey
2024-02-27 3:12 ` [PATCH 3/3] Fix PR libcc1/113977 Tom Tromey
2 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2024-02-27 3:12 UTC (permalink / raw)
To: gcc-patches
This fixes version negotiation in the libcc1 plugins. It's done in a
simple way: the version number from the context object is now passed
to base_gdb_plugin.
The idea behind this is that when the client (gdb) requests version N,
the plugin should respond with the newest version that it knows of
that is backward compatible to N. That is, the connection can be
upgraded. Note that the protocol does not change much, and no
backward incompatibilities have ever been needed.
The C plugin is also changed to advertise GCC_C_FE_VERSION_1.
The version negotiation approach should of course be documented, but I
did that in a subsequent patch, in order to only have one patch
touching the 'include' directory and thus needing a merge to
binutils-gdb.
2024-02-26 Tom Tromey <tom@tromey.com>
* libcp1.cc (libcp1::libcp1): Use FE version number from context.
* libcc1.cc (libcc1::libcc1): Use FE version number from context.
(c_vtable): Use GCC_C_FE_VERSION_1.
---
libcc1/ChangeLog | 6 ++++++
libcc1/libcc1.cc | 4 ++--
libcc1/libcp1.cc | 2 +-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/libcc1/ChangeLog b/libcc1/ChangeLog
index b0b31ee6586..b4072574ba8 100644
--- a/libcc1/ChangeLog
+++ b/libcc1/ChangeLog
@@ -1,3 +1,9 @@
+2024-02-26 Tom Tromey <tom@tromey.com>
+
+ * libcp1.cc (libcp1::libcp1): Use FE version number from context.
+ * libcc1.cc (libcc1::libcc1): Use FE version number from context.
+ (c_vtable): Use GCC_C_FE_VERSION_1.
+
2024-02-26 Tom Tromey <tom@tromey.com>
* libcc1plugin.cc (safe_lookup_builtin_type): Handle ERROR_MARK.
diff --git a/libcc1/libcc1.cc b/libcc1/libcc1.cc
index 8d4ddc5ddfe..992181e8fdc 100644
--- a/libcc1/libcc1.cc
+++ b/libcc1/libcc1.cc
@@ -54,7 +54,7 @@ struct libcc1 : public cc1_plugin::base_gdb_plugin<gcc_c_context>
libcc1::libcc1 (const gcc_c_fe_vtable *cv)
: cc1_plugin::base_gdb_plugin<gcc_c_context> ("libcc1plugin",
C_COMPILER_NAME,
- GCC_C_FE_VERSION_1)
+ cv->c_version)
{
c_ops = cv;
}
@@ -108,7 +108,7 @@ set_callbacks (struct gcc_c_context *s,
static const struct gcc_c_fe_vtable c_vtable =
{
- GCC_C_FE_VERSION_0,
+ GCC_C_FE_VERSION_1,
set_callbacks,
#define GCC_METHOD0(R, N) \
diff --git a/libcc1/libcp1.cc b/libcc1/libcp1.cc
index ec3eec2c606..cc2915d30af 100644
--- a/libcc1/libcp1.cc
+++ b/libcc1/libcp1.cc
@@ -55,7 +55,7 @@ struct libcp1 : public cc1_plugin::base_gdb_plugin<gcc_cp_context>
libcp1::libcp1 (const gcc_cp_fe_vtable *cv)
: cc1_plugin::base_gdb_plugin<gcc_cp_context> ("libcp1plugin",
CP_COMPILER_NAME,
- GCC_CP_FE_VERSION_0)
+ cv->cp_version)
{
cp_ops = cv;
}
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] Fix PR libcc1/113977
2024-02-27 3:12 [PATCH 0/3] Fix libcc1 failure Tom Tromey
2024-02-27 3:12 ` [PATCH 1/3] Change 'v1' float and int code to fall back to v0 Tom Tromey
2024-02-27 3:12 ` [PATCH 2/3] Fix version negotiation in libcc1 plugins Tom Tromey
@ 2024-02-27 3:12 ` Tom Tromey
2 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2024-02-27 3:12 UTC (permalink / raw)
To: gcc-patches
PR libcc1/113977 points out a case where a simple expression is
rejected with a compiler error message. The bug here is that gdb does
not inform the plugin of the correct alignment -- in fact, there is no
way to do that.
This patch adds a new method to allow the alignment to be set, and
bumps the C front end protocol version.
It also includes some updates to various comments in 'include', done
here to simplify the merge to binutils-gdb.
include/ChangeLog
2024-02-26 Tom Tromey <tom@tromey.com>
* gcc-cp-interface.h (gcc_cp_fe_context_function): Update
comment.
* gcc-c-interface.h (enum gcc_c_api_version) <GCC_C_FE_VERSION_2>:
New constant.
(gcc_c_fe_context_function): Update comment.
* gcc-c-fe.def (finish_record_with_alignment): New method.
Update documentation.
libcc1/ChangeLog
2024-02-26 Tom Tromey <tom@tromey.com>
PR libcc1/113977
* libcc1plugin.cc (plugin_finish_record_or_union): New function.
(plugin_finish_record_or_union): Rewrite.
(plugin_init): Use GCC_C_FE_VERSION_2.
* libcc1.cc (c_vtable): Use GCC_C_FE_VERSION_2.
(gcc_c_fe_context): Check for GCC_C_FE_VERSION_2.
---
include/ChangeLog | 10 ++++++++++
include/gcc-c-fe.def | 13 ++++++++++++-
include/gcc-c-interface.h | 11 +++++++++--
include/gcc-cp-interface.h | 6 +++++-
libcc1/ChangeLog | 9 +++++++++
libcc1/libcc1.cc | 5 +++--
libcc1/libcc1plugin.cc | 26 ++++++++++++++++++--------
7 files changed, 66 insertions(+), 14 deletions(-)
diff --git a/include/ChangeLog b/include/ChangeLog
index 8bfaec6faa1..4c8b5b28039 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,3 +1,13 @@
+2024-02-26 Tom Tromey <tom@tromey.com>
+
+ * gcc-cp-interface.h (gcc_cp_fe_context_function): Update
+ comment.
+ * gcc-c-interface.h (enum gcc_c_api_version) <GCC_C_FE_VERSION_2>:
+ New constant.
+ (gcc_c_fe_context_function): Update comment.
+ * gcc-c-fe.def (finish_record_with_alignment): New method.
+ Update documentation.
+
2024-01-13 Jakub Jelinek <jakub@redhat.com>
* demangle.h (enum demangle_component_type): Add
diff --git a/include/gcc-c-fe.def b/include/gcc-c-fe.def
index 36a765484a7..cb7cf197525 100644
--- a/include/gcc-c-fe.def
+++ b/include/gcc-c-fe.def
@@ -89,7 +89,10 @@ GCC_METHOD5 (int /* bool */, build_add_field,
/* After all the fields have been added to a struct or union, the
struct or union type must be "finished". This does some final
- cleanups in GCC. */
+ cleanups in GCC.
+
+ Note that when using GCC_C_FE_VERSION_2, it is preferable to call
+ finish_record_with_alignment instead. */
GCC_METHOD2 (int /* bool */, finish_record_or_union,
gcc_type, /* Argument RECORD_OR_UNION_TYPE. */
@@ -220,3 +223,11 @@ GCC_METHOD2 (gcc_type, float_type,
unsigned long, /* Argument SIZE_IN_BYTES. */
const char *) /* Argument BUILTIN_NAME. */
+/* New in GCC_FE_VERSION_2. Like finish_record_or_union but the caller also
+ supplies the alignment. If the alignment is 0, this acts identically to
+ finish_record_or_union. */
+
+GCC_METHOD3 (int /* bool */, finish_record_with_alignment,
+ gcc_type, /* Argument RECORD_OR_UNION_TYPE. */
+ unsigned long, /* Argument SIZE_IN_BYTES. */
+ unsigned long) /* Argument ALIGNMENT. */
diff --git a/include/gcc-c-interface.h b/include/gcc-c-interface.h
index feece1e38a2..700d7483a4a 100644
--- a/include/gcc-c-interface.h
+++ b/include/gcc-c-interface.h
@@ -45,7 +45,10 @@ enum gcc_c_api_version
/* Added char_type. Added new version of int_type and float_type,
deprecated int_type_v0 and float_type_v0. */
- GCC_C_FE_VERSION_1 = 1
+ GCC_C_FE_VERSION_1 = 1,
+
+ /* Added finish_record_with_alignment method. */
+ GCC_C_FE_VERSION_2 = 2,
};
/* Qualifiers. */
@@ -198,7 +201,11 @@ struct gcc_c_context
/* The type of the initialization function. The caller passes in the
desired base version and desired C-specific version. If the
request can be satisfied, a compatible gcc_context object will be
- returned. Otherwise, the function returns NULL. */
+ returned. In particular, this may return a context object with a higher
+ actual version number than was requested, provided the higher version is
+ fully compatible. (As of GCC_C_FE_VERSION_2, this is always true.)
+
+ Otherwise, the function returns NULL. */
typedef struct gcc_c_context *gcc_c_fe_context_function
(enum gcc_base_api_version,
diff --git a/include/gcc-cp-interface.h b/include/gcc-cp-interface.h
index 2f950729b9b..15b911cb216 100644
--- a/include/gcc-cp-interface.h
+++ b/include/gcc-cp-interface.h
@@ -483,7 +483,11 @@ struct gcc_cp_context
/* The type of the initialization function. The caller passes in the
desired base version and desired C-specific version. If the
request can be satisfied, a compatible gcc_context object will be
- returned. Otherwise, the function returns NULL. */
+ returned. In particular, this may return a context object with a higher
+ actual version number than was requested, provided the higher version is
+ fully compatible.
+
+ Otherwise, the function returns NULL. */
typedef struct gcc_cp_context *gcc_cp_fe_context_function
(enum gcc_base_api_version,
diff --git a/libcc1/ChangeLog b/libcc1/ChangeLog
index b4072574ba8..a23130bac15 100644
--- a/libcc1/ChangeLog
+++ b/libcc1/ChangeLog
@@ -1,3 +1,12 @@
+2024-02-26 Tom Tromey <tom@tromey.com>
+
+ PR libcc1/113977
+ * libcc1plugin.cc (plugin_finish_record_or_union): New function.
+ (plugin_finish_record_or_union): Rewrite.
+ (plugin_init): Use GCC_C_FE_VERSION_2.
+ * libcc1.cc (c_vtable): Use GCC_C_FE_VERSION_2.
+ (gcc_c_fe_context): Check for GCC_C_FE_VERSION_2.
+
2024-02-26 Tom Tromey <tom@tromey.com>
* libcp1.cc (libcp1::libcp1): Use FE version number from context.
diff --git a/libcc1/libcc1.cc b/libcc1/libcc1.cc
index 992181e8fdc..1c570f3054d 100644
--- a/libcc1/libcc1.cc
+++ b/libcc1/libcc1.cc
@@ -108,7 +108,7 @@ set_callbacks (struct gcc_c_context *s,
static const struct gcc_c_fe_vtable c_vtable =
{
- GCC_C_FE_VERSION_1,
+ GCC_C_FE_VERSION_2,
set_callbacks,
#define GCC_METHOD0(R, N) \
@@ -165,7 +165,8 @@ gcc_c_fe_context (enum gcc_base_api_version base_version,
enum gcc_c_api_version c_version)
{
if ((base_version != GCC_FE_VERSION_0 && base_version != GCC_FE_VERSION_1)
- || (c_version != GCC_C_FE_VERSION_0 && c_version != GCC_C_FE_VERSION_1))
+ || (c_version != GCC_C_FE_VERSION_0 && c_version != GCC_C_FE_VERSION_1
+ && c_version != GCC_C_FE_VERSION_2))
return NULL;
return new libcc1 (&c_vtable);
diff --git a/libcc1/libcc1plugin.cc b/libcc1/libcc1plugin.cc
index f1082d8e9d3..72d17c3b81c 100644
--- a/libcc1/libcc1plugin.cc
+++ b/libcc1/libcc1plugin.cc
@@ -387,9 +387,10 @@ plugin_build_add_field (cc1_plugin::connection *,
}
int
-plugin_finish_record_or_union (cc1_plugin::connection *,
- gcc_type record_or_union_type_in,
- unsigned long size_in_bytes)
+plugin_finish_record_with_alignment (cc1_plugin::connection *,
+ gcc_type record_or_union_type_in,
+ unsigned long size_in_bytes,
+ unsigned long align)
{
tree record_or_union_type = convert_in (record_or_union_type_in);
@@ -407,10 +408,10 @@ plugin_finish_record_or_union (cc1_plugin::connection *,
}
else
{
- // FIXME there's no way to get this from DWARF,
- // or even, it seems, a particularly good way to deduce it.
- SET_TYPE_ALIGN (record_or_union_type,
- TYPE_PRECISION (pointer_sized_int_node));
+ if (align == 0)
+ align = TYPE_PRECISION (pointer_sized_int_node);
+
+ SET_TYPE_ALIGN (record_or_union_type, align);
TYPE_SIZE (record_or_union_type) = bitsize_int (size_in_bytes
* BITS_PER_UNIT);
@@ -441,6 +442,15 @@ plugin_finish_record_or_union (cc1_plugin::connection *,
return 1;
}
+int
+plugin_finish_record_or_union (cc1_plugin::connection *conn,
+ gcc_type record_or_union_type_in,
+ unsigned long size_in_bytes)
+{
+ return plugin_finish_record_with_alignment (conn, record_or_union_type_in,
+ size_in_bytes, 0);
+}
+
gcc_type
plugin_build_enum_type (cc1_plugin::connection *self,
gcc_type underlying_int_type_in)
@@ -755,7 +765,7 @@ int
plugin_init (struct plugin_name_args *plugin_info,
struct plugin_gcc_version *)
{
- generic_plugin_init (plugin_info, GCC_C_FE_VERSION_1);
+ generic_plugin_init (plugin_info, GCC_C_FE_VERSION_2);
register_callback (plugin_info->base_name, PLUGIN_PRAGMAS,
plugin_init_extra_pragmas, NULL);
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread