Re: [PATCH] loc2stap.cxx: Add partial support for DW_OP_bra in DWARF location lists

[Aaron Merey <amerey@redhat.com>]

On Thu, Apr 18, 2024 at 8:50 PM Aaron Merey <amerey@redhat.com> wrote:
>
> Add support for DW_OP_bra when operand is non-negative.  Previously
> systemtap would quit probe translation if DW_OP_bra was seen in a
> DWARF location list.
>
> Tested manually on RHEL 8.9 with kernel 4.18.0-513.24.1.el8_9.x86_64.
> Scripts containing a vfs.read probe require DW_OP_bra support when
> run with this kernel.
>
> Support for DW_OP_bra negative operands continues to be deferred due
> to lack of use as well as being more complex to implement.
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=31496
> ---
>  loc2stap.cxx | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/loc2stap.cxx b/loc2stap.cxx
> index 53316a480..883993040 100644
> --- a/loc2stap.cxx
> +++ b/loc2stap.cxx
> @@ -435,9 +435,26 @@ location_context::translate (const Dwarf_Op *expr, const size_t len,
>             }
>             break;
>
> -           /* Control flow operations.  */
> +         /* Control flow operations.  */
> +         case DW_OP_bra:
> +           {
> +             POP(taken)
> +             if (taken == 0)
> +               break;
> +           }
> +           /* FALLTHROUGH */
> +
>           case DW_OP_skip:
>             {
> +             /* A backward branch could lead to an infinite loop.  So
> +                punt it until we find we actually need it.
> +
> +                To support backward branches, we could copy the stack,
> +                recurse, and place both traces in arms of a
> +                ternary_expression.  */
> +             if ((Dwarf_Sword) expr[i].number < 0)
> +               DIE("backward branch in DW_OP operation not supported");
> +
>               Dwarf_Off target = expr[i].offset + 3 + expr[i].number;
>               while (i + 1 < len && expr[i + 1].offset < target)
>                 ++i;
> @@ -446,13 +463,6 @@ location_context::translate (const Dwarf_Op *expr, const size_t len,
>               break;
>             }
>
> -         case DW_OP_bra:
> -           // ??? Could copy the stack, recurse, and place both
> -           // traces in arms of a ternary_expression.  No point
> -           // in doing that if this is never used, however.
> -           DIE ("conditional branches not supported");
> -           break;
> -
>             /* Memory access.  */
>           case DW_OP_deref:
>             // ??? Target sizeof, not host sizeof.
> --
> 2.43.0

Pushed as commit 22e146b57ec

Aaron