Message ID | 20240226-gdb-compile-align-v1-1-0f95d6435299@tromey.com |
---|---|
State | New |
Headers | show |
Series | Fix libcc1 failure | expand |
On 2/26/24 20:12, Tom Tromey wrote: > 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. Given this is all libcc1 related and thus primarily of interest to gdb, if you're happy with it, then it's OK for the trunk. jeff
>>>>> "Jeff" == Jeff Law <jeffreyalaw@gmail.com> writes:
Jeff> Given this is all libcc1 related and thus primarily of interest to
Jeff> gdb, if you're happy with it, then it's OK for the trunk.
Thank you.
I could not push this because:
remote: *** ChangeLog format failed:
remote: *** ERR: invalid PR component in subject: "Fix PR libcc1/113977"
I guess this script isn't in sync with the components in bugzilla.
I don't know how to fix this.
Tom
On 2/28/24 15:57, Tom Tromey wrote: >>>>>> "Jeff" == Jeff Law <jeffreyalaw@gmail.com> writes: > I could not push this because: > > remote: *** ChangeLog format failed: > remote: *** ERR: invalid PR component in subject: "Fix PR libcc1/113977" > > I guess this script isn't in sync with the components in bugzilla. > > I don't know how to fix this. Me neither, but I can suggest a hacky workaround. Change the component in bugzilla to something the pre-commit hooks understand, push the fix, then change the component back a little while later and adjust the ChangeLog after it gets generated overnight. Ugly as sin. jeff
On Wed, Feb 28, 2024 at 3:26 PM Jeff Law <jeffreyalaw@gmail.com> wrote: > > > > On 2/28/24 15:57, Tom Tromey wrote: > >>>>>> "Jeff" == Jeff Law <jeffreyalaw@gmail.com> writes: > > > I could not push this because: > > > > remote: *** ChangeLog format failed: > > remote: *** ERR: invalid PR component in subject: "Fix PR libcc1/113977" > > > > I guess this script isn't in sync with the components in bugzilla. > > > > I don't know how to fix this. > Me neither, but I can suggest a hacky workaround. Change the component > in bugzilla to something the pre-commit hooks understand, push the fix, > then change the component back a little while later and adjust the > ChangeLog after it gets generated overnight. Ugly as sin. I don't know how to update the script server side after it is committed in git. the checker script is located in git though: ``` [apinski@xeond2 contrib]$ git diff gcc-changelog/git_commit.py diff --git a/contrib/gcc-changelog/git_commit.py b/contrib/gcc-changelog/git_commit.py index 87bec4e00f5..4a3720de7fb 100755 --- a/contrib/gcc-changelog/git_commit.py +++ b/contrib/gcc-changelog/git_commit.py @@ -109,6 +109,7 @@ bug_components = { 'libffi', 'libfortran', 'libgcc', + 'libcc1', 'libgcj', 'libgomp', 'libitm', ``` Thanks, Andrew Pinski > > jeff
>>>>> "Andrew" == Andrew Pinski <pinskia@gmail.com> writes:
Andrew> I don't know how to update the script server side after it is
Andrew> committed in git. the checker script is located in git though:
Thanks, I didn't realize it was there.
Could you check in your patch?
IMO it seems obvious.
Tom
On Wed, Feb 28, 2024 at 5:35 PM Tom Tromey <tom@tromey.com> wrote: > > >>>>> "Andrew" == Andrew Pinski <pinskia@gmail.com> writes: > > Andrew> I don't know how to update the script server side after it is > Andrew> committed in git. the checker script is located in git though: > > Thanks, I didn't realize it was there. > > Could you check in your patch? > IMO it seems obvious. Pushed as r14-9230-g5ff49272bf4eb6 (https://gcc.gnu.org/pipermail/gcc-patches/2024-February/646819.html). I noticed there should be other cleanup of the bug components there too but I will leave that for another time. Thanks, Andrew Pinski > > Tom
>>>>> "Jeff" == Jeff Law <jeffreyalaw@gmail.com> writes: >> I don't know how to fix this. Jeff> Me neither, but I can suggest a hacky workaround. FTR, I asked Jakub on irc and he fixed it, so thankfully I didn't have to resort to the hack :-) thanks, Tom
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));