diff mbox series

[v3,PR,symtab/30520,1/4] gdb/symtab: check name matches before expanding a CU

Message ID 20240506150920.1220092-1-dmitry.neverov@jetbrains.com
State New
Headers
Series [v3,PR,symtab/30520,1/4] gdb/symtab: check name matches before expanding a CU |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Dmitry.Neverov May 6, 2024, 3:09 p.m. UTC 
  From: "Dmitry.Neverov" <dmitry.neverov@jetbrains.com>

The added check fixes the case when an unqualified lookup
name without template arguments causes expansion of many CUs
which contain the name with template arguments.

This is similar to what dw2_expand_symtabs_matching_symbol does
before expanding the CU.

In the referenced issue the lookup name was wxObjectDataPtr and many
CUs had names like wxObjectDataPtr<wxBitmapBundleImpl>. This caused
their expansion and the lookup took around a minute. The added check
helps to avoid the expansion and makes the symbol lookup to return in
a second or so.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30520
---
 gdb/dwarf2/read.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

Comments

Tom Tromey May 7, 2024, 5:03 p.m. UTC | #1 
>>>>> "Dmitry" == Dmitry Neverov <dmitry.neverov@jetbrains.com> writes:

Thank you for your continued work on this.

Dmitry> +		      const language_defn *lang_def = language_def (entry->lang);
Dmitry> +		      lookup_name_info last_segment_lookup_name (
Dmitry> +			last_name.data (), symbol_name_match_type::FULL,
Dmitry> +			false, true);

Normally in gdb the open paren would go on the next line, like

> +		      lookup_name_info last_segment_lookup_name
> +			(last_name.data (), symbol_name_match_type::FULL,
> +			 false, true);

... depending on how long the lines are -- it would be split after some
',' if each line would then fit in 80 columns.

Dmitry> +		      symbol_name_matcher_ftype *name_matcher
Dmitry> +			= lang_def->get_symbol_name_matcher
Dmitry> +			  (last_segment_lookup_name);

This also looks weird by gdb rules.

Tom
Dmitry.Neverov May 8, 2024, 11:32 a.m. UTC | #2 
Tom Tromey <tom@tromey.com> writes:
> Dmitry> +		      symbol_name_matcher_ftype *name_matcher
> Dmitry> +			= lang_def->get_symbol_name_matcher
> Dmitry> +			  (last_segment_lookup_name);
>
> This also looks weird by gdb rules.

Do I understand correctly that it should look like this:

		      symbol_name_matcher_ftype *name_matcher = lang_def
			->get_symbol_name_matcher (last_segment_lookup_name);

?

How do I send an updated patch? I've read the Contribution Checklist,
but didn't find the answer. Do I amend the commit and 'git send-mail' it
again with proper '--in-reply-to'?

--
Dmitry
Tom Tromey May 8, 2024, 3:48 p.m. UTC | #3 
>>>>> "Dmitry" == Dmitry Neverov <dmitry.neverov@jetbrains.com> writes:

Dmitry> Tom Tromey <tom@tromey.com> writes:
Dmitry> +		      symbol_name_matcher_ftype *name_matcher
Dmitry> +			= lang_def->get_symbol_name_matcher
Dmitry> +			  (last_segment_lookup_name);
>> 
>> This also looks weird by gdb rules.

Dmitry> Do I understand correctly that it should look like this:

I'd write:

		      symbol_name_matcher_ftype *name_matcher
			= lang_def->get_symbol_name_matcher (last_segment_lookup_name);

Sometimes wrapping is unavoidable but there's also

		      symbol_name_matcher_ftype *name_matcher
			= (lang_def->get_symbol_name_matcher
                             (last_segment_lookup_name));

Dmitry> How do I send an updated patch? I've read the Contribution Checklist,
Dmitry> but didn't find the answer. Do I amend the commit and 'git send-mail' it
Dmitry> again with proper '--in-reply-to'?

I just re-send it with --reroll-count=...

I don't normally worry about in-reply-to.  Not sure, maybe I should?
For series I often use b4 these days, though I've also been bitten by
its bizarre behavior, so I'm not sure I can really recommend it.

Tom
Dmitry.Neverov May 9, 2024, 5:55 a.m. UTC | #4 
Tom Tromey <tom@tromey.com> writes:

> I just re-send it with --reroll-count=...

Thanks, I fixed formatting and re-sent all pathces:

https://sourceware.org/pipermail/gdb-patches/2024-May/209010.html
diff mbox series

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 049ee4d52ff..e2f010b7849 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -16694,9 +16694,25 @@  cooked_index_functions::expand_symtabs_matching
 		= lookup_name_without_params.match_type ();
 	      if ((match_type == symbol_name_match_type::FULL
 		   || (lang != language_ada
-		       && match_type == symbol_name_match_type::EXPRESSION))
-		  && parent != nullptr)
-		continue;
+		       && match_type == symbol_name_match_type::EXPRESSION)))
+		{
+		  if (parent != nullptr)
+		    continue;
+
+		  if (entry->lang != language_unknown)
+		    {
+		      const language_defn *lang_def = language_def (entry->lang);
+		      lookup_name_info last_segment_lookup_name (
+			last_name.data (), symbol_name_match_type::FULL,
+			false, true);
+		      symbol_name_matcher_ftype *name_matcher
+			= lang_def->get_symbol_name_matcher
+			  (last_segment_lookup_name);
+		      if (!name_matcher (entry->canonical,
+					 last_segment_lookup_name, nullptr))
+			continue;
+		    }
+	      }
 	    }
 	  else
 	    {