Re: [PATCH v3 3/3] Fix range end handling of inlined subroutines

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

Oops, sorry.

I've missed some comments in my last reply.


On 7/19/24 20:48, Guinevere Larsen wrote:
> On 7/5/24 6:18 AM, Bernd Edlinger wrote:
>>   diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>> index 60fd8b45eb5..e40679611fe 100644
>> --- a/gdb/dwarf2/read.c
>> +++ b/gdb/dwarf2/read.c
>> @@ -10733,10 +10733,6 @@ dwarf2_rnglists_process (unsigned offset, struct dwarf2_cu *cu,
>>         return false;
>>       }
>>   -      /* Empty range entries have no effect.  */
>> -      if (range_beginning == range_end)
>> -    continue;
>> -
>>         /* Only DW_RLE_offset_pair needs the base address added.  */
>>         if (rlet == DW_RLE_offset_pair)
>>       {
>> @@ -10855,10 +10851,6 @@ dwarf2_ranges_process (unsigned offset, struct dwarf2_cu *cu, dwarf_tag tag,
>>         return 0;
>>       }
>>   -      /* Empty range entries have no effect.  */
>> -      if (range_beginning == range_end)
>> -    continue;
>> -
>>         range_beginning = (unrelocated_addr) ((CORE_ADDR) range_beginning
>>                           + (CORE_ADDR) *base);
>>         range_end = (unrelocated_addr) ((CORE_ADDR) range_end
>> @@ -11080,8 +11072,8 @@ dwarf2_get_pc_bounds (struct die_info *die, unrelocated_addr *lowpc,
>>     if (ret == PC_BOUNDS_NOT_PRESENT || ret == PC_BOUNDS_INVALID)
>>       return ret;
>>   -  /* partial_die_info::read has also the strict LOW < HIGH requirement.  */
>> -  if (high <= low)
>> +  /* partial_die_info::read has also the same low < high requirement.  */
> We use scream case to indicate variable names, so there's no need for this change.

Ah, good you mentioned this.
Actually the comment can go away completely, since there is no longer any partial_die_info,
since this commit apparently removed that function completely:

commit 6209cde4ddb85a991ed1dda6f143ef08b75558df
Author: Tom Tromey <tom@tromey.com>
Date:   Sun May 30 08:00:19 2021 -0600

    Delete DWARF psymtab code
    
    This removes the DWARF psymtab reader.

>>   @@ -3333,18 +3336,14 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
>>        We used to return alt->line - 1 here, but that could be
>>        anywhere; if we don't have line number info for this PC,
>>        don't make some up.  */
>> -      val.pc = pc;
>> -    }
>> -  else if (best->line == 0)
>> -    {
>> -      /* If our best fit is in a range of PC's for which no line
>> -     number info is available (line number is zero) then we didn't
>> -     find any valid line information.  */
> Why did you remove the logic handling not finding any line information? Surely it is still possible that we can't find debug information for other reasons.


This code is unreachable.

The only place where BEST is assigned to non-zero value,
is here:

      if (prev && prev->line
          && (!best || prev->unrelocated_pc () > best->unrelocated_pc ()
                    || (prev->unrelocated_pc () == best->unrelocated_pc ()
                        && (best->pc (objfile) == pc
                            ? !best->is_stmt : best->is_weak))))
        {
          best = prev;

and here:

          if (!best->is_stmt)
            {
              const linetable_entry *tmp = best;
              while (tmp > first
                     && (tmp - 1)->unrelocated_pc () == tmp->unrelocated_pc ()
                     && (tmp - 1)->line != 0 && !tmp->is_stmt)
                --tmp;
              if (tmp->is_stmt && (tmp->pc (objfile) == pc || !tmp->is_weak))
                best = tmp;

so as you can see, in both places it is ensured that best->line != 0.

This is so since in 2003 when this commit
commit 083ae9356e21082ea18fc21d91a84bbaa7a76fc7
Author: Daniel Jacobowitz <drow@false.org>
Date:   Mon Jan 13 21:59:53 2003 +0000

            * symtab.c (find_pc_sect_line): Don't consider end-of-function
            lines.

changed the if condition here:

-      if (prev && (!best || prev->pc > best->pc))
+      if (prev && prev->line && (!best || prev->pc > best->pc))
        {
          best = prev;
          best_symtab = s;

but forgot to remove the now no longer needed

  else if (best->line == 0)
    {
      /* If our best fit is in a range of PC's for which no line
         number info is available (line number is zero) then we didn't
         find any valid line information. */
      val.pc = pc;
    }


Thanks
Bernd.