diff mbox series

gdb: fix incorrect search domain in find_function_in_inferior

Message ID cce972b6b291d18471d19a4946e8b900c28b67f7.1788365753.git.aburgess@redhat.com
State New
Headers
Series gdb: fix incorrect search domain in find_function_in_inferior |

Checks

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

Commit Message

Andrew Burgess Sept. 2, 2026, 4:16 p.m. UTC 
  The find_function_in_inferior function is used when GDB needs to make
an inferior function call as part of expression evaluation, for
example, calling malloc to allocate space in the inferior, or calling
an object's constructor.

The function lookup has two phases, first we search for full symbols.
If that search fails then we fallback to looking for a minimal symbol.

The problem I see here is that the full symbol search uses
SEARCH_TYPE_DOMAIN, and has done since commit:

  commit ccf41c248737eb6650211481366c4e1156ce01ae
  Date:   Thu Mar 30 23:00:26 2023 -0600

      Use domain_search_flags in lookup_symbol et al

Prior to this commit the search was done using VAR_DOMAIN, which would
find types, variables, and functions, there was even code in place to
raise an error if the symbol we found was not a function.

The ccf41c248737eb66 commit switched to SEARCH_TYPE_DOMAIN and removed
the "is a function" check.  I think this was a mistake.  Given that
find_function_in_inferior is always used to look for a function, I
think we should have switched to SEARCH_FUNCTION_DOMAIN.  The "is a
function" check can be removed as the search will now only find
functions.

So the first thing I fixed in this commit is to change
SEARCH_TYPE_DOMAIN to SEARCH_FUNCTION_DOMAIN in
find_function_in_inferior.

With that done the next problem we encounter is that if the full
symbol is for a GNU IFUNC then we need to handle this via the minimal
symbol path.  For inspiration here I looked at the 'variable:
name_not_typename' rule in the c-exp.y file, where we say:

      /* If we found a function, see if it's
	 an ifunc resolver that has the same
	 address as the ifunc symbol itself.
	 If so, prefer the ifunc symbol.  */

I think find_function_in_inferior should apply the same logic.  To
achieve this I added a call to find_gnu_ifunc and restructured the
code slightly so that after the full symbol lookup the minimal symbol
can come from either calling lookup_minimal_symbol, or from the
find_gnu_ifunc path.

There are no new tests, but I have been using gdb.base/gnu-ifunc.exp
as a smoke test for this change.  When I have glibc debug information
installed I can (by attaching GDB to GDB) see the full symbol lookup
path now triggering, so I know that the updated code path is now being
used.

It was while reviewing commits:

  commit ca0908d623605250e6d84afb90d742c328e6bb90
  Date:   Tue Aug 11 13:12:19 2026 +0000

      gdb: Keep original IFUNC return type when target type is unknown

  commit de930032d883219559d1dba575f2c0f5359e80fc
  Date:   Tue Aug 11 13:12:18 2026 +0000

      gdb: Preserve IFUNC marker when finding inferior functions

which touched gdb.base/gnu-ifunc.exp that I spotted this bug.
---
 gdb/valops.c | 79 ++++++++++++++++++++++++++--------------------------
 1 file changed, 40 insertions(+), 39 deletions(-)


base-commit: 9c1937eb7103bee8c329b9c4f5137fcd1726b23d

Comments

Tom Tromey Sept. 3, 2026, 8:23 p.m. UTC | #1 
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> The problem I see here is that the full symbol search uses
Andrew> SEARCH_TYPE_DOMAIN, and has done since commit:
Andrew>   commit ccf41c248737eb6650211481366c4e1156ce01ae
Andrew>   Date:   Thu Mar 30 23:00:26 2023 -0600
Andrew>       Use domain_search_flags in lookup_symbol et al

Sorry about that.  I guess it worked by falling back to minsyms?

I think this is ok.  Thank you.
Approved-By: Tom Tromey <tom@tromey.com>

Tom
Andrew Burgess Sept. 4, 2026, 9:54 a.m. UTC | #2 
Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> The problem I see here is that the full symbol search uses
> Andrew> SEARCH_TYPE_DOMAIN, and has done since commit:
> Andrew>   commit ccf41c248737eb6650211481366c4e1156ce01ae
> Andrew>   Date:   Thu Mar 30 23:00:26 2023 -0600
> Andrew>       Use domain_search_flags in lookup_symbol et al
>
> Sorry about that.  I guess it worked by falling back to minsyms?

Exactly.

>
> I think this is ok.  Thank you.
> Approved-By: Tom Tromey <tom@tromey.com>

Pushed.

Thanks,
Andrew
diff mbox series

Patch

diff --git a/gdb/valops.c b/gdb/valops.c
index 82c796bd254..e214342c40d 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -113,52 +113,53 @@  struct value *
 find_function_in_inferior (const char *name, struct objfile **objf_p)
 {
   struct block_symbol sym;
+  bound_minimal_symbol msymbol;
 
-  sym = lookup_symbol (name, nullptr, SEARCH_TYPE_DOMAIN, nullptr);
-  if (sym.symbol != NULL)
+  sym = lookup_symbol (name, nullptr, SEARCH_FUNCTION_DOMAIN, nullptr);
+  if (sym.symbol != nullptr)
     {
-      if (objf_p)
-	*objf_p = sym.symbol->objfile ();
+      msymbol = find_gnu_ifunc (sym.symbol);
+      if (msymbol.minsym == nullptr)
+	{
+	  if (objf_p != nullptr)
+	    *objf_p = sym.symbol->objfile ();
+	  return value_of_variable (sym.symbol, sym.block);
+	}
+    }
+  else
+    msymbol = lookup_minimal_symbol (current_program_space, name);
 
-      return value_of_variable (sym.symbol, sym.block);
+  if (msymbol.minsym != nullptr)
+    {
+      struct objfile *objfile = msymbol.objfile;
+      struct gdbarch *gdbarch = objfile->arch ();
+
+      struct type *type;
+      CORE_ADDR maddr;
+      type = lookup_pointer_type (builtin_type (gdbarch)->builtin_char);
+      type = lookup_function_type (type);
+      type = lookup_pointer_type (type);
+      maddr = msymbol.value_address ();
+      minimal_symbol_type minsym_type = msymbol.minsym->type ();
+
+      if (minsym_type == mst_text_gnu_ifunc
+	  || minsym_type == mst_data_gnu_ifunc)
+	type->target_type ()->set_is_gnu_ifunc (true);
+
+      if (objf_p != nullptr)
+	*objf_p = objfile;
+
+      return value_from_pointer (type, maddr);
     }
   else
     {
-      bound_minimal_symbol msymbol
-	= lookup_minimal_symbol (current_program_space, name);
-
-      if (msymbol.minsym != NULL)
-	{
-	  struct objfile *objfile = msymbol.objfile;
-	  struct gdbarch *gdbarch = objfile->arch ();
-
-	  struct type *type;
-	  CORE_ADDR maddr;
-	  type = lookup_pointer_type (builtin_type (gdbarch)->builtin_char);
-	  type = lookup_function_type (type);
-	  type = lookup_pointer_type (type);
-	  maddr = msymbol.value_address ();
-	  minimal_symbol_type minsym_type = msymbol.minsym->type ();
-
-	  if (minsym_type == mst_text_gnu_ifunc
-	      || minsym_type == mst_data_gnu_ifunc)
-	    type->target_type ()->set_is_gnu_ifunc (true);
-
-	  if (objf_p)
-	    *objf_p = objfile;
-
-	  return value_from_pointer (type, maddr);
-	}
+      if (!target_has_execution ())
+	error (_("evaluation of this expression "
+		 "requires the target program to be active"));
       else
-	{
-	  if (!target_has_execution ())
-	    error (_("evaluation of this expression "
-		     "requires the target program to be active"));
-	  else
-	    error (_("evaluation of this expression requires the "
-		     "program to have a function \"%s\"."),
-		   name);
-	}
+	error (_("evaluation of this expression requires the "
+		 "program to have a function \"%s\"."),
+	       name);
     }
 }