Re: [PATCH v2 2/2] gdb: Add support for DW_LNS_set_prologue_end in line-table

[Pedro Alves <pedro@palves.net>]

On 2022-03-30 10:34, Lancelot SIX via Gdb-patches wrote:

> This commit proposes to add support for the prologue_end flag in the
> line-program processing.
> 
> The processing of this prologue_end flag is made in skip_prologue_sal,
> before it calls gdbarch_skip_prologue_noexcept.  The intent is that if
> the compiler gave information on where the prologue ends, we should use
> this information and not try to rely on architecture dependent logic to
> guess it.

There's risk that this runs into compiler bugs, making the experience
worse than if we use the existing code path.  I wonder whether we should have
a "maint set foo" knob for this.  It's still early in the release cycle,
so I'm OK with waiting a bit to see if we really need it, though.  OTOH,
having such a switch regardless would be useful for force-testing the prologue
scanners.  Hmm.

> 
> The testsuite have been executed using this patch on GNU/Linux x86_64.
> Testcases have been combiled with both gcc/g++ and clang/clang++ since

combiled -> compiled

> GCC does not set the prologue_end marker.  No regression have been
> observed with GCC or Clang.  Note that when using Clang, this patch
> fixes a failure in gdb.opt/inline-small-func.exp.

Please mention GCC and Clang versions, for the record.

>  \f
> diff --git a/gdb/buildsym.h b/gdb/buildsym.h
> index 09c2d563ec9..252bad187dc 100644
> --- a/gdb/buildsym.h
> +++ b/gdb/buildsym.h
> @@ -115,6 +115,7 @@ struct context_stack
>  enum linetable_entry_flag : unsigned
>  {
>    LEF_IS_STMT = 1 << 1,
> +  LEF_PROLOGUE_END = 1 << 2,

Please document this enumerator too.

> +
> +  const struct symtab_and_line prologue_sal = find_pc_line (start_pc, 0);
> +  if (prologue_sal.symtab != nullptr
> +      && prologue_sal.symtab->language () != language_asm)
> +    {
> +      struct linetable *linetable = prologue_sal.symtab->linetable ();
> +
> +      auto it = std::lower_bound
> +	(linetable->item, linetable->item + linetable->nitems, start_pc,
> +	 [] (const linetable_entry &lte, CORE_ADDR pc) -> bool
> +	 { return lte.pc < pc; });
> +
> +      for (;
> +	   it < linetable->item + linetable->nitems && it->pc <= end_pc ;
> +	   it++)
> +	if (it->prologue_end)
> +	  return {it->pc};
> +    }

Space/tab mismatch, I guess, since { and } don't align here in the patch.

> +
> +  return {};
> +}


Otherwise LGTM.