Re: GDB internal error in pc_in_thread_step_range

[Eli Zaretskii <eliz@gnu.org>]

> Cc: gdb-patches@sourceware.org
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Sat, 15 Dec 2018 22:57:57 -0500
> 
> Hi Eli,
> 
> Sorry for the wait.  I don't really have an good answer for you, but I thought I'd
> reply anyway, maybe this will help generate ideas.

Thanks for replying.

> >   if (address)
> >     {
> >       if (pc_in_unmapped_range (pc, section))
> > 	*address = overlay_unmapped_address (cache_pc_function_low, section);
> >       else
> > 	*address = cache_pc_function_low;
> >     }
> > 
> >   if (name)
> >     *name = cache_pc_function_name;
> > 
> >   if (endaddr)
> >     {
> >       if (pc_in_unmapped_range (pc, section))
> > 	{
> > 	  /* Because the high address is actually beyond the end of
> > 	     the function (and therefore possibly beyond the end of
> > 	     the overlay), we must actually convert (high - 1) and
> > 	     then add one to that.  */
> > 
> > 	  *endaddr = 1 + overlay_unmapped_address (cache_pc_function_high - 1,
> > 						   section);
> > 	}
> >       else
> > 	*endaddr = cache_pc_function_high;
> >     }
> > 
> > The cached values are zero and 1, correspondingly.
> 
> Do you mean that cache_pc_function_low is 0 and cache_pc_function_high is 1?

Yes.

> Do these values even make sense?

What else can we expect from a code at PC for which there's absolutely
no symbolic information?  So yes, I think it's reasonable, but I'm far
from being an expert on these parts of GDB.

> They are supposed to hold a range of program addresses, so 0 and 1
> seem bogus.  Maybe this is the result of something going wrong
> before?  It would be interesting to understand how they end up with
> these values.

They are assigned here:

  cache_pc_function_low = BMSYMBOL_VALUE_ADDRESS (msymbol);
  cache_pc_function_name = MSYMBOL_LINKAGE_NAME (msymbol.minsym);
  cache_pc_function_section = section;
  cache_pc_function_high = minimal_symbol_upper_bound (msymbol);
  cache_pc_function_block = nullptr;

This is part of find_pc_partial_function.  I verified that
minimal_symbol_upper_bound returns 1 in this case, and that this value
of 1 is assigned here:

  obj_section = MSYMBOL_OBJ_SECTION (minsym.objfile, minsym.minsym);
  if (MSYMBOL_LINKAGE_NAME (msymbol + i) != NULL
      && (MSYMBOL_VALUE_ADDRESS (minsym.objfile, msymbol + i)
	  < obj_section_endaddr (obj_section)))
    result = MSYMBOL_VALUE_ADDRESS (minsym.objfile, msymbol + i); <<<<<<
  else

Once again, I'm not an expert on this stuff, but just thinking about
the situation, what else could GDB return in this case?

> If find_pc_partial_function is unable to determine a proper symbol and some proper
> bounds, it should return 0.  So if it returns 1 but returns some wrong data,
> something is fishy.

If it returns zero, we will emit an error message:

	      if (find_pc_partial_function (pc, &name,
					    &tp->control.step_range_start,
					    &tp->control.step_range_end) == 0)
		error (_("Cannot find bounds of current function"));

So I'm not sure this is a good idea.  Instead, I propose the following
change:

--- gdb/infrun.c~0	2018-07-04 18:41:59.000000000 +0300
+++ gdb/infrun.c	2018-12-16 11:02:24.103425700 +0200
@@ -2713,7 +2713,13 @@ resume_1 (enum gdb_signal sig)
       displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf));
     }
 
-  if (tp->control.may_range_step)
+  if (tp->control.may_range_step
+      /* If .step_range_start == 0 and .step_range_end == 1, we don't
+	 really know the step range, so don't check in that case.
+	 (This is known to happen on MinGW when stepping the program
+	 epilogue code after 'main' returns.)  */
+      && !(tp->control.step_range_start == 0x0
+	   && tp->control.step_range_end == 0x1))
     {
       /* If we're resuming a thread with the PC out of the step
 	 range, then we're doing some nested/finer run control


Thanks.