public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
From: Aaron Merey <amerey@redhat.com>
To: systemtap@sourceware.org
Cc: Aaron Merey <amerey@redhat.com>
Subject: [PATCH] loc2stap.cxx: Add partial support for DW_OP_bra in DWARF location lists
Date: Thu, 18 Apr 2024 20:50:35 -0400	[thread overview]
Message-ID: <20240419005035.960853-1-amerey@redhat.com> (raw)

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


             reply	other threads:[~2024-04-19  0:50 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-19  0:50 Aaron Merey [this message]
2024-04-23 18:12 ` Aaron Merey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240419005035.960853-1-amerey@redhat.com \
    --to=amerey@redhat.com \
    --cc=systemtap@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).