[rfc] Move PC in-range check in objc-lang.c:find_methods (Re: [ia64] Regression)

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Jan Kratochvil wrote:

> mst_data looks suspicious but on ppc64 case the function descriptor has really
> mst_data there:

Looking at the location in objc-lang.c:find_methods where the mst_text
check used to be a bit closer, I noticed something odd anyway:

      ALL_OBJFILE_MSYMBOLS (objfile, msymbol)
        {
          struct gdbarch *gdbarch = get_objfile_arch (objfile);
          CORE_ADDR pc = SYMBOL_VALUE_ADDRESS (msymbol);

          QUIT;

          /* The minimal symbol might point to a function descriptor;
             resolve it to the actual code address instead.  */
          pc = gdbarch_convert_from_func_ptr_addr (gdbarch, pc,
                                                   &current_target);

          if (symtab)
            if (pc < BLOCK_START (block) || pc >= BLOCK_END (block))
              /* Not in the specified symtab.  */
              continue;

          symname = SYMBOL_NATURAL_NAME (msymbol);
          if (symname == NULL)
            continue;

          if ((symname[0] != '-' && symname[0] != '+') || (symname[1] != '['))
            /* Not a method name.  */
            continue;

          while ((strlen (symname) + 1) >= tmplen)
            {
              tmplen = (tmplen == 0) ? 1024 : tmplen * 2;
              tmp = xrealloc (tmp, tmplen);
            }
          strcpy (tmp, symname);

          if (parse_method (tmp, &ntype, &nclass, &ncategory, &nselector) == NULL)
            continue;

          objfile_csym++;

For every msymbol in the objfile, the very *first* check is whether
its address is in range of the given symbol table.  Only then come the
checks against the symbol name up to and including the parse_method call.
If those checks pass, the objfile_csym count is increased.

However, that count is supposed to be the number of *all* ObjC symbols
in the objfile, not just those that match the given symtab.  Note the
sanity check at the end:

        /* Count of ObjC methods in this objfile should be constant.  */
        gdb_assert (*objc_csym == objfile_csym);

It would appear that if find_methods is first called with a non-NULL
symtab, objfile_csym is set to the number of ObjC symbols within that
symtab.  Assuming this is non-zero, and find_methods is later called
with a different (or NULL) symtab value, the loop will now possibly
compute a *different* count of ObjC symbols, triggering the assertion.

Therefore, it seems that the range check against the symtab ought
to be performed only *after* the symbol name checks are done and
objfile_csym is incremented ...

As a side effect, this would also reduce the number of calls to
gdbarch_convert_from_func_ptr_addr for symbols that clearly are not
ObjC symbols, reducing the potential of running into problems in
targets that cannot deterministically tell whether an address
points to a function descriptor or not.

This is implemented by the following patch.  Does this look
reasonable?

Tested on ppc(64)-linux with no regressions.

Bye,
Ulrich

ChangeLog:

	* objc-lang.c (find_methods): Move function descriptor conversion
	and PC in-range check until after basic checks for ObjC symbols.


Index: gdb/objc-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/objc-lang.c,v
retrieving revision 1.84
diff -c -p -r1.84 objc-lang.c
*** gdb/objc-lang.c	29 Sep 2009 00:48:31 -0000	1.84
--- gdb/objc-lang.c	29 Sep 2009 17:11:19 -0000
*************** find_methods (struct symtab *symtab, cha
*** 1178,1193 ****
  
  	  QUIT;
  
- 	  /* The minimal symbol might point to a function descriptor;
- 	     resolve it to the actual code address instead.  */
- 	  pc = gdbarch_convert_from_func_ptr_addr (gdbarch, pc,
- 						   &current_target);
- 
- 	  if (symtab)
- 	    if (pc < BLOCK_START (block) || pc >= BLOCK_END (block))
- 	      /* Not in the specified symtab.  */
- 	      continue;
- 
  	  symname = SYMBOL_NATURAL_NAME (msymbol);
  	  if (symname == NULL)
  	    continue;
--- 1178,1183 ----
*************** find_methods (struct symtab *symtab, cha
*** 1208,1213 ****
--- 1198,1213 ----
        
  	  objfile_csym++;
  
+ 	  /* The minimal symbol might point to a function descriptor;
+ 	     resolve it to the actual code address instead.  */
+ 	  pc = gdbarch_convert_from_func_ptr_addr (gdbarch, pc,
+ 						   &current_target);
+ 
+ 	  if (symtab)
+ 	    if (pc < BLOCK_START (block) || pc >= BLOCK_END (block))
+ 	      /* Not in the specified symtab.  */
+ 	      continue;
+ 
  	  if ((type != '\0') && (ntype != type))
  	    continue;
  
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com