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

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

[Aaron Merey]

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

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

[Aaron Merey]

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