public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* Command line systemtap macro setting
@ 2021-01-08  5:07 Craig Ringer
  2021-01-13  4:39 ` Craig Ringer
  0 siblings, 1 reply; 4+ messages in thread
From: Craig Ringer @ 2021-01-08  5:07 UTC (permalink / raw)
  To: systemtap

Hi

I keep on finding situations where I want to set systemtap language macros
on the command line. AFAICS there is no such feature. Right? The closest
seems to be to generate a .stpm file with the desired macro definition and
put on the include path.

Would you object to a patch that added this capability? If so, how do you
think it should be spelled on the command line? "-D" is taken. So maybe
"-M" ?

The most important time I need command line macro definitions is to work
around the issues with relative paths. I pretty much always require
absolute paths for executables in my tapscripts. Otherwise @var("var@cu",
"exec_path") doesn't work properly or didn't last I checked. And probes
that fail to match seem to like to fail silently if the executable isn't
found on the PATH, where they'll report an explicit error if the executable
is a fully qualified path. So I try to run with absolute paths all the
time. But I don't really want users to have to hack the script to supply
them, so I'd like to be able to e.g.

    -D POSTGRES_EXECUTABLE="/path/to/postgres"

However, this won't work - it'd set a C preprocessor macro for the runtime
compilation, not the systemtap language processing phase. And you can't

    -G POSTGRES_EXECUTABLE="/path/to/postgres"

because probes don't accept globals in their arguments; this isn't legal:

    probe process(POSTGRES_EXECUTABLE).function("foo") { ...}

That can be handled with a positional parameter instead, but doing so is
less user friendly. It's much less obvious what the param is and does when
reading the cmdline.

Similarly, if I want to make it easy to customise script behaviour,
something like this won't work:

    global stats_report_interval_ms=5000
    probe timer.ms(stats_report_interval_ms ) { do_something(); }




-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 2ndQuadrant - PostgreSQL Solutions for the Enterprise

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Command line systemtap macro setting
  2021-01-08  5:07 Command line systemtap macro setting Craig Ringer
@ 2021-01-13  4:39 ` Craig Ringer
       [not found]   ` <20210113150431.GC25762@redhat.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Craig Ringer @ 2021-01-13  4:39 UTC (permalink / raw)
  To: systemtap, Frank Ch. Eigler

Frank,

Any thoughts on this? I'd be happy to submit a patch but I'd like to know
if you think it's reasonable to do first, so I don't spend the time writing
something that won't be committable.

It'd be a big usability improvement in systemtap for me to be able to
define systemtap language macros on the command line.


On Fri, 8 Jan 2021 at 13:07, Craig Ringer <craig@2ndquadrant.com> wrote:

> Hi
>
> I keep on finding situations where I want to set systemtap language macros
> on the command line. AFAICS there is no such feature. Right? The closest
> seems to be to generate a .stpm file with the desired macro definition and
> put on the include path.
>
> Would you object to a patch that added this capability? If so, how do you
> think it should be spelled on the command line? "-D" is taken. So maybe
> "-M" ?
>
> The most important time I need command line macro definitions is to work
> around the issues with relative paths. I pretty much always require
> absolute paths for executables in my tapscripts. Otherwise @var("var@cu",
> "exec_path") doesn't work properly or didn't last I checked. And probes
> that fail to match seem to like to fail silently if the executable isn't
> found on the PATH, where they'll report an explicit error if the executable
> is a fully qualified path. So I try to run with absolute paths all the
> time. But I don't really want users to have to hack the script to supply
> them, so I'd like to be able to e.g.
>
>     -D POSTGRES_EXECUTABLE="/path/to/postgres"
>
> However, this won't work - it'd set a C preprocessor macro for the runtime
> compilation, not the systemtap language processing phase. And you can't
>
>     -G POSTGRES_EXECUTABLE="/path/to/postgres"
>
> because probes don't accept globals in their arguments; this isn't legal:
>
>     probe process(POSTGRES_EXECUTABLE).function("foo") { ...}
>
> That can be handled with a positional parameter instead, but doing so is
> less user friendly. It's much less obvious what the param is and does when
> reading the cmdline.
>
> Similarly, if I want to make it easy to customise script behaviour,
> something like this won't work:
>
>     global stats_report_interval_ms=5000
>     probe timer.ms(stats_report_interval_ms ) { do_something(); }
>
>
>
>
> --
>  Craig Ringer                   http://www.2ndQuadrant.com/
>  2ndQuadrant - PostgreSQL Solutions for the Enterprise
>


-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 2ndQuadrant - PostgreSQL Solutions for the Enterprise

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Command line systemtap macro setting
       [not found]   ` <20210113150431.GC25762@redhat.com>
@ 2021-01-15  2:18     ` Craig Ringer
  2021-01-15  2:38       ` Craig Ringer
  0 siblings, 1 reply; 4+ messages in thread
From: Craig Ringer @ 2021-01-15  2:18 UTC (permalink / raw)
  To: Frank Ch. Eigler, systemtap

[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]

On Wed, 13 Jan 2021 at 23:04, Frank Ch. Eigler <fche@redhat.com> wrote:

> Hi -
>
> > Any thoughts on this? I'd be happy to submit a patch but I'd like to know
> > if you think it's reasonable to do first, so I don't spend the time
> writing
> > something that won't be committable.
> > It'd be a big usability improvement in systemtap for me to be able to
> > define systemtap language macros on the command line.
>
> Thanks so much for doing the Work!  It's certainly a reasonable
> self-serve kind of way to do it outside the tool.
>
> I really think tho that stap translator proper should do this, and am
> pretty sure we can.  It'd be some hacking around within tapsets.cxx or
> loc2stap.cxx or somesuch, but it wouldn't be too much.  The enum
> values for $FOO_BAR could translate straight to literals.  Would you
> have any interest in trying to hack on that?
>

I made an attempt at doing this by implementing an @enum language feature,
but landed up getting totally bogged.

My WIP is attached - it's on top of commit 403e92779 so it might not merge
exactly, but it's unfinished anyway.

I got lost in all the visitors and kept hitting assertions I couldn't
understand or debug, IIRC, and ran out of time on it.

-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 2ndQuadrant - PostgreSQL Solutions for the Enterprise

[-- Attachment #2: 0001-wip-enum-support.patch --]
[-- Type: text/x-patch, Size: 26245 bytes --]

From 3cab5bfafcc169959d51d36f8c10f584bee17d6f Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig.ringer@2ndquadrant.com>
Date: Tue, 21 Jul 2020 15:55:02 +0800
Subject: [PATCH] wip @enum support

---
 buildrun.cxx  |   2 +-
 dwflpp.cxx    |  64 +++++++++++++++--
 dwflpp.h      |  15 ++++
 elaborate.cxx |  25 +++++++
 elaborate.h   |   1 +
 parse.cxx     |  38 +++++++++-
 staptree.cxx  |  56 +++++++++++++++
 staptree.h    |  15 ++++
 tapsets.cxx   | 187 +++++++++++++++++++++++++++++++++++++++++++++++++-
 translate.cxx |   6 ++
 10 files changed, 402 insertions(+), 7 deletions(-)

diff --git a/buildrun.cxx b/buildrun.cxx
index 668b42b6d..4981f086d 100644
--- a/buildrun.cxx
+++ b/buildrun.cxx
@@ -31,7 +31,7 @@ extern "C" {
 #include <sys/resource.h>
 }
 
-// A bit of obfuscation for Gentoo's sake.
+// A bit of obfuscation for Gentoo's sake so it cannot hide -Werror.
 // We *need* -Werror for stapconf to work correctly.
 // https://bugs.gentoo.org/show_bug.cgi?id=522908
 #define WERROR ("-W" "error")
diff --git a/dwflpp.cxx b/dwflpp.cxx
index 83a06d5cb..79ef26171 100644
--- a/dwflpp.cxx
+++ b/dwflpp.cxx
@@ -1193,8 +1193,61 @@ dwflpp::iterate_over_globals<void>(Dwarf_Die *cu_die,
 }
 
 template<> int
-dwflpp::iterate_over_types<void>(Dwarf_Die *top_die,
-                                 bool has_inner_types,
+dwflpp::iterate_over_enumerations<void>(Dwarf_Die *top_die,
+                                 int (* callback)(Dwarf_Die*, Dwarf_Die*, void*),
+                                 void *data)
+{
+  int rc = DWARF_CB_OK;
+  Dwarf_Die die;
+
+  assert (top_die);
+  assert (dwarf_tag(top_die) == DW_TAG_compile_unit);
+
+  if (dwarf_child(top_die, &die) != 0)
+    return rc;
+
+  do
+    switch (dwarf_tag(&die))
+      {
+      case DW_TAG_enumeration_type:
+	{
+	  /* Walk the enum type's enumeration members */
+	  Dwarf_die enum_die;
+  	  if (dwarf_child(die, &enum_die) == 0) {
+	    do {
+	      if (dwarf_tag(&enum_die) == DW_TAG_enumerator)
+		{
+		  /* Let the cb inspect the enum */
+		  rc = (*callback)(&enumdie, &die, data);
+	        }
+	      else if (sess.verbose > 3)
+		{
+		  clog << _F("unexpected child of DWARF enumeration_type %s: got %d, expected %d (DW_TAG_enumerator)", dwarf_diename(&die), dwarf_tag(&enum_die), DW_TAG_enumerator) << endl;
+		}
+	    } while (enum_rc == DWARF_CB_OK && dwarf_siblingof(&enumdie, &enumdie) == 0);
+	  }
+          break;
+	}
+
+      case DW_TAG_imported_unit:
+	{
+	  // Follow the imported_unit and iterate over its contents
+	  // (either a partial_unit or a full compile_unit), all its
+	  // children should be treated as if they appear in this place.
+	  Dwarf_die import;
+	  if (dwarf_attr_die(&die, DW_AT_import, &import))
+	    rc = iterate_over_enumerations(&import,
+				    callback, data);
+	  break;
+	}
+      }
+  while (rc == DWARF_CB_OK && dwarf_siblingof(&die, &die) == 0);
+
+  return rc;
+}
+
+template<> int
+dwflpp::iterate_over_enumerators<void>(Dwarf_Die *top_die,
                                  const string& prefix,
                                  int (* callback)(Dwarf_Die*,
                                                   bool,
@@ -1230,8 +1283,7 @@ dwflpp::iterate_over_types<void>(Dwarf_Die *top_die,
 	// (either a partial_unit or a full compile_unit), all its
 	// children should be treated as if they appear in this place.
 	if (dwarf_attr_die(&die, DW_AT_import, &import))
-	  rc = iterate_over_types(&import, has_inner_types, prefix,
-				  callback, data);
+	  rc = iterate_over_enumerators(&import, prefix, callback, data);
 	break;
       }
   while (rc == DWARF_CB_OK && dwarf_siblingof(&die, &die) == 0);
@@ -3558,6 +3610,10 @@ dwflpp::translate_components(location_context *ctx,
 
         case DW_TAG_enumeration_type:
         case DW_TAG_base_type:
+	  /*
+	   * Can't deref base types, and C enums are accessed by their
+	   * DW_TAG_enumerator not their DW_TAG_enumeration_type
+	   */
           throw SEMANTIC_ERROR (_F("invalid access '%s' vs. %s", lex_cast(c).c_str(),
                                    dwarf_type_name(typedie).c_str()), c.tok);
           break;
diff --git a/dwflpp.h b/dwflpp.h
index 76c77a427..16382bff9 100644
--- a/dwflpp.h
+++ b/dwflpp.h
@@ -435,6 +435,15 @@ struct dwflpp
                                         (void*)data);
     }
 
+  template<typename T>
+  static int iterate_over_enumerations(Dwarf_Die *cu_die,
+                                 int (* callback)(Dwarf_Die*, Dwarf_Die*, void*),
+                                 void *data) {
+      return iterate_over_enumerations<void>(cu_die,
+                                        (int (*)(Dwarf_Die*, Dwarf_Die*, void*)) callback,
+					(void*)data);
+  }
+
   GElf_Shdr * get_section(std::string section_name, GElf_Shdr *shdr_mem,
                           Elf **elf_ret=NULL);
 
@@ -743,6 +752,12 @@ dwflpp::iterate_over_types<void>(Dwarf_Die *top_die,
                                                   const std::string&,
                                                   void*),
                                  void *data);
+
+template<> int
+dwflpp::iterate_over_enumerations<void>(Dwarf_Die *top_die,
+                                 int (* callback)(Dwarf_Die*, Dwarf_Die*, void*),
+                                 void *data);
+
 template<> int
 dwflpp::iterate_over_notes<void>(void *object, void (*callback)(void*,
                                                                 const std::string&,
diff --git a/elaborate.cxx b/elaborate.cxx
index b580c0402..d6d820262 100644
--- a/elaborate.cxx
+++ b/elaborate.cxx
@@ -3551,6 +3551,11 @@ struct assignment_symbol_fetcher
     sym = NULL;
   }
 
+  void visit_atenum_op (atenum_op*)
+  {
+    sym = NULL;
+  }
+
   void visit_cast_op (cast_op*)
   {
     sym = NULL;
@@ -4030,6 +4035,7 @@ struct void_statement_reducer: public update_visitor
   void visit_print_format (print_format* e);
   void visit_target_symbol (target_symbol* e);
   void visit_atvar_op (atvar_op* e);
+  void visit_atenum_op (atenum_op* e);
   void visit_cast_op (cast_op* e);
   void visit_autocast_op (autocast_op* e);
   void visit_defined_op (defined_op* e);
@@ -4378,6 +4384,14 @@ void_statement_reducer::visit_atvar_op (atvar_op* e)
   reduce_target_symbol (e);
 }
 
+void
+void_statement_reducer::visit_atenum_op (atenum_op* e)
+{
+  if (session.verbose>2)
+    clog << _("Eliding unused target symbol ") << *e->tok << endl;
+  reduce_target_symbol (e);
+}
+
 void
 void_statement_reducer::visit_target_symbol (target_symbol* e)
 {
@@ -5905,6 +5919,7 @@ struct initial_typeresolution_info : public typeresolution_info
   // and not all substitutions are done, replace the functions that throw errors.
   void visit_target_symbol (target_symbol*) {}
   void visit_atvar_op (atvar_op*) {}
+  void visit_atenum_op (atenum_op*) {}
   void visit_defined_op (defined_op*) {}
   void visit_entry_op (entry_op*) {}
   void visit_cast_op (cast_op*) {}
@@ -6729,6 +6744,16 @@ typeresolution_info::visit_cast_op (cast_op* e)
                                            e->module.to_string().c_str()), e->tok));
 }
 
+void
+typeresolution_info::visit_atenum_op (atenum_op* e)
+{
+  if (e->saved_conversion_error)
+    session.print_error (* (e->saved_conversion_error));
+  else
+    session.print_error (SEMANTIC_ERROR(_F("enumeration definition '%s' not found in '%s'",
+                                           e->enumeration_name.to_string().c_str(),
+                                           e->module.to_string().c_str()), e->tok));
+}
 
 void
 typeresolution_info::visit_autocast_op (autocast_op* e)
diff --git a/elaborate.h b/elaborate.h
index 28c04abce..6201a7849 100644
--- a/elaborate.h
+++ b/elaborate.h
@@ -172,6 +172,7 @@ struct typeresolution_info: public visitor
   void visit_cast_op (cast_op* e);
   void visit_autocast_op (autocast_op* e);
   void visit_atvar_op (atvar_op* e);
+  void visit_atenum_op (atenum_op* e);
   void visit_defined_op (defined_op* e);
   void visit_entry_op (entry_op* e);
   void visit_perf_op (perf_op* e);
diff --git a/parse.cxx b/parse.cxx
index cd6234362..8981e8d24 100644
--- a/parse.cxx
+++ b/parse.cxx
@@ -204,6 +204,7 @@ private: // nonterminals
   target_symbol *parse_target_symbol ();
   cast_op *parse_cast_op ();
   atvar_op *parse_atvar_op ();
+  atenum_op *parse_atenum_op ();
   expression* parse_entry_op (const token* t);
   expression* parse_defined_op (const token* t);
   expression* parse_const_op (const token* t);
@@ -1478,6 +1479,10 @@ lexer::lexer (istream& input, const string& in, systemtap_session& s, bool cc):
           atwords.insert("kderef");
           atwords.insert("uderef");
         }
+      if (has_version("4.4"))
+        {
+	  atwords.insert("enum");
+	}
     }
 }
 
@@ -3712,6 +3717,8 @@ parser::parse_dwarf_value ()
   else if (addressof && !input.has_version("2.6"))
     // '&' on old version only allowed specific target_symbol types
     throw PARSE_ERROR (_("expected @cast, @var or $var"));
+  else if (input.has_version("4.4") && tok_is(t, tok_operator, "@enum"))
+    expr = tsym = parse_atenum_op ();
   else
     {
       // Otherwise just get a plain value of any sort.
@@ -3834,7 +3841,7 @@ parser::parse_indexable ()
 
 
 // var, indexable[index], func(parms), printf("...", ...),
-// @defined, @entry, @stat_op(stat)
+// @defined, @entry, @stat_op(stat), @enum
 expression* parser::parse_symbol ()
 {
   hist_op *hop = NULL;
@@ -4175,6 +4182,34 @@ atvar_op* parser::parse_atvar_op ()
   throw PARSE_ERROR (_("expected @var"));
 }
 
+// Parse a @enum
+atenum_op* parser::parse_atenum_op ()
+{
+  const token* t = next ();
+  if (t->type == tok_operator && t->content == "@enum")
+    {
+      atenum_op *eop = new atenum_op;
+      eop->tok = t;
+      eop->name = t->content;
+      expect_op("(");
+      /* Read enumeration name (required) */
+      expect_unknown(tok_string, eop->enumeration_name);
+      if (eop->enumeration_name.empty())
+        throw PARSE_ERROR (_("@enum expected non-empty literal string for first argument"));
+      /* Optional second argument specifies the CU / module to look in */
+      if (peek_op (","))
+        {
+          swallow ();
+          expect_unknown(tok_string, eop->module);
+        }
+      else
+	eop->module = "";
+      expect_op(")");
+      return eop;
+    }
+
+  throw PARSE_ERROR (_("expected @enum"));
+}
 
 // Parse a @defined().  Given head token has already been consumed.
 expression* parser::parse_defined_op (const token* t)
@@ -4189,6 +4224,7 @@ expression* parser::parse_defined_op (const token* t)
 
 
 // Parse a @const().  Given head token has already been consumed.
+// This expands to an embedded_expr so there is no const_op tree type.
 expression* parser::parse_const_op (const token* t)
 {
   if (! privileged)
diff --git a/staptree.cxx b/staptree.cxx
index c8fecadb8..34fcc6d70 100644
--- a/staptree.cxx
+++ b/staptree.cxx
@@ -564,6 +564,15 @@ void atvar_op::print (ostream& o) const
     o << components[i];
 }
 
+void atenum_op::print (ostream& o) const
+{
+  o << name << '(' << lex_cast_qstring(enumeration_name);
+  if (module.length() > 0)
+    o << ", " << lex_cast_qstring (module);
+  o << ')';
+  for (unsigned i = 0; i < components.size(); ++i)
+    o << components[i];
+}
 
 void cast_op::print (ostream& o) const
 {
@@ -1798,6 +1807,12 @@ atvar_op::visit (visitor* u)
   u->visit_atvar_op(this);
 }
 
+void
+atenum_op::visit (visitor* u)
+{
+  u->visit_atenum_op(this);
+}
+
 
 void
 defined_op::visit (visitor* u)
@@ -2180,6 +2195,12 @@ traversing_visitor::visit_atvar_op (atvar_op* e)
   e->visit_components (this);
 }
 
+void
+traversing_visitor::visit_atenum_op (atenum_op* e)
+{
+  e->visit_components (this);
+}
+
 void
 traversing_visitor::visit_defined_op (defined_op* e)
 {
@@ -2442,6 +2463,13 @@ expression_visitor::visit_atvar_op (atvar_op* e)
   visit_expression (e);
 }
 
+void
+expression_visitor::visit_atenum_op (atenum_op* e)
+{
+  traversing_visitor::visit_atenum_op (e);
+  visit_expression (e);
+}
+
 void
 expression_visitor::visit_defined_op (defined_op* e)
 {
@@ -2717,6 +2745,14 @@ varuse_collecting_visitor::visit_atvar_op (atvar_op *e)
   functioncall_traversing_visitor::visit_atvar_op (e);
 }
 
+void
+varuse_collecting_visitor::visit_atenum_op (atenum_op *e)
+{
+  if (is_active_lvalue (e))
+    throw SEMANTIC_ERROR (_("@enum cannot be used as an lvalue"), e->tok);
+
+  functioncall_traversing_visitor::visit_atenum_op (e);
+}
 
 void
 varuse_collecting_visitor::visit_cast_op (cast_op *e)
@@ -3244,6 +3280,12 @@ throwing_visitor::visit_atvar_op (atvar_op* e)
   throwone (e->tok);
 }
 
+void
+throwing_visitor::visit_atenum_op (atenum_op* e)
+{
+  throwone (e->tok);
+}
+
 void
 throwing_visitor::visit_cast_op (cast_op* e)
 {
@@ -3562,6 +3604,7 @@ update_visitor::visit_target_symbol (target_symbol* e)
   provide (e);
 }
 
+/* XXX does this make sense ? */
 void
 update_visitor::visit_cast_op (cast_op* e)
 {
@@ -3570,6 +3613,13 @@ update_visitor::visit_cast_op (cast_op* e)
   provide (e);
 }
 
+void
+update_visitor::visit_atenum_op (atenum_op* e)
+{
+  e->visit_components (this);
+  provide (e);
+}
+
 void
 update_visitor::visit_autocast_op (autocast_op* e)
 {
@@ -3879,6 +3929,12 @@ deep_copy_visitor::visit_atvar_op (atvar_op* e)
   update_visitor::visit_atvar_op(new atvar_op(*e));
 }
 
+void
+deep_copy_visitor::visit_atenum_op (atenum_op* e)
+{
+  update_visitor::visit_atenum_op(new atenum_op(*e));
+}
+
 void
 deep_copy_visitor::visit_defined_op (defined_op* e)
 {
diff --git a/staptree.h b/staptree.h
index 8902d3aa9..56e027d2c 100644
--- a/staptree.h
+++ b/staptree.h
@@ -425,6 +425,13 @@ struct atvar_op: public target_symbol
   void visit (visitor* u);
 };
 
+struct atenum_op: public target_symbol
+{
+  interned_string enumeration_name, module;
+  void print (std::ostream& o) const;
+  void visit (visitor* u);
+};
+
 struct defined_op: public expression
 {
   expression *operand;
@@ -978,6 +985,7 @@ struct visitor
   virtual void visit_cast_op (cast_op* e) = 0;
   virtual void visit_autocast_op (autocast_op* e) = 0;
   virtual void visit_atvar_op (atvar_op* e) = 0;
+  virtual void visit_atenum_op (atenum_op* e) = 0;
   virtual void visit_defined_op (defined_op* e) = 0;
   virtual void visit_entry_op (entry_op* e) = 0;
   virtual void visit_perf_op (perf_op* e) = 0;
@@ -1031,6 +1039,7 @@ struct nop_visitor: public visitor
   virtual void visit_cast_op (cast_op*) {};
   virtual void visit_autocast_op (autocast_op*) {};
   virtual void visit_atvar_op (atvar_op*) {};
+  virtual void visit_atenum_op (atenum_op*) {};
   virtual void visit_defined_op (defined_op*) {};
   virtual void visit_entry_op (entry_op*) {};
   virtual void visit_perf_op (perf_op*) {};
@@ -1084,6 +1093,7 @@ struct traversing_visitor: public visitor
   void visit_cast_op (cast_op* e);
   void visit_autocast_op (autocast_op* e);
   void visit_atvar_op (atvar_op* e);
+  void visit_atenum_op (atenum_op* e);
   void visit_defined_op (defined_op* e);
   void visit_entry_op (entry_op* e);
   void visit_perf_op (perf_op* e);
@@ -1124,6 +1134,7 @@ struct expression_visitor: public traversing_visitor
   void visit_cast_op (cast_op* e);
   void visit_autocast_op (autocast_op* e);
   void visit_atvar_op (atvar_op* e);
+  void visit_atenum_op (atenum_op* e);
   void visit_defined_op (defined_op* e);
   void visit_entry_op (entry_op* e);
   void visit_perf_op (perf_op* e);
@@ -1186,6 +1197,7 @@ struct varuse_collecting_visitor: public functioncall_traversing_visitor
   void visit_cast_op (cast_op* e);
   void visit_autocast_op (autocast_op* e);
   void visit_atvar_op (atvar_op *e);
+  void visit_atenum_op (atenum_op *e);
   void visit_defined_op (defined_op* e);
   void visit_entry_op (entry_op* e);
   void visit_perf_op (perf_op* e);
@@ -1247,6 +1259,7 @@ struct throwing_visitor: public visitor
   void visit_cast_op (cast_op* e);
   void visit_autocast_op (autocast_op* e);
   void visit_atvar_op (atvar_op* e);
+  void visit_atenum_op (atenum_op* e);
   void visit_defined_op (defined_op* e);
   void visit_entry_op (entry_op* e);
   void visit_perf_op (perf_op* e);
@@ -1361,6 +1374,7 @@ struct update_visitor: public visitor
   virtual void visit_cast_op (cast_op* e);
   virtual void visit_autocast_op (autocast_op* e);
   virtual void visit_atvar_op (atvar_op* e);
+  virtual void visit_atenum_op (atenum_op* e);
   virtual void visit_defined_op (defined_op* e);
   virtual void visit_entry_op (entry_op* e);
   virtual void visit_perf_op (perf_op* e);
@@ -1429,6 +1443,7 @@ struct deep_copy_visitor: public update_visitor
   virtual void visit_cast_op (cast_op* e);
   virtual void visit_autocast_op (autocast_op* e);
   virtual void visit_atvar_op (atvar_op* e);
+  virtual void visit_atenum_op (atenum_op* e);
   virtual void visit_defined_op (defined_op* e);
   virtual void visit_entry_op (entry_op* e);
   virtual void visit_perf_op (perf_op* e);
diff --git a/tapsets.cxx b/tapsets.cxx
index 9db41cd87..e3e00cef7 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -2854,6 +2854,7 @@ struct dwarf_var_expanding_visitor: public var_expanding_visitor
   void visit_target_symbol (target_symbol* e);
   void visit_atvar_op (atvar_op* e);
   void visit_cast_op (cast_op* e);
+  void visit_atenum_op (atenum_op* e);
   void visit_entry_op (entry_op* e);
   void visit_perf_op (perf_op* e);
 
@@ -3074,6 +3075,8 @@ public:
   { context_op_p = true; traversing_visitor::visit_defined_op(e); }
   void visit_atvar_op (atvar_op* e)
   { context_op_p = true; traversing_visitor::visit_atvar_op(e); }
+  void visit_atenum_op (atenum_op* e)
+  { context_op_p = true; traversing_visitor::visit_atenum_op(e); }
   void visit_cast_op (cast_op* e) // if module is specified, not a context_op_p
   { if (e->module == "") context_op_p = true; traversing_visitor::visit_cast_op(e); }
   void visit_autocast_op (autocast_op* e) // XXX do these show up early?
@@ -4702,6 +4705,16 @@ dwarf_var_expanding_visitor::visit_cast_op (cast_op *e)
   var_expanding_visitor::visit_cast_op(e);
 }
 
+void
+dwarf_var_expanding_visitor::visit_atenum_op (atenum_op *e)
+{
+  // Fill in our current module context if needed
+  if (e->module.empty())
+    e->module = q.dw.module_name;
+
+  var_expanding_visitor::visit_atenum_op(e);
+}
+
 
 void
 dwarf_var_expanding_visitor::visit_entry_op (entry_op *e)
@@ -4972,6 +4985,7 @@ void dwarf_cast_expanding_visitor::filter_special_modules(string& module)
 }
 
 
+/* Guts of @cast(...) implementation */
 void dwarf_cast_expanding_visitor::visit_cast_op (cast_op* e)
 {
   bool lvalue = is_active_lvalue(e);
@@ -5112,7 +5126,7 @@ exp_type_dwarf::expand(autocast_op* e, bool lvalue)
     }
 }
 
-
+/* Guts of @var(...) implementation */
 struct dwarf_atvar_expanding_visitor: public var_expanding_visitor
 {
   dwarf_builder& db;
@@ -5292,6 +5306,169 @@ dwarf_atvar_expanding_visitor::visit_atvar_op (atvar_op* e)
   provide(e);
 }
 
+/* Guts of @enum(...) implementation */
+struct dwarf_atenum_expanding_visitor: public var_expanding_visitor
+{
+  dwarf_builder& db;
+
+  dwarf_atenum_expanding_visitor(systemtap_session& s, dwarf_builder& db):
+    var_expanding_visitor(s), db(db) {}
+  void visit_atenum_op (atenum_op* e);
+};
+
+
+struct dwarf_atenum_query: public base_query
+{
+  atenum_op& e;
+  functioncall*& result;
+
+  dwarf_atenum_query(dwflpp& dw, const string& module, atenum_op& e,
+                    functioncall*& result):
+    base_query(dw, module), e(e), result(result),
+    {}
+
+  void handle_query_module ();
+  void query_library (const char *) {}
+  void query_plt (const char *, size_t) {}
+  /* Search each type in a CU for the target enumeration during @enum expansion */
+  static int atenum_query_cu_cb(Dwarf_Die* cudie, dwarf_atenum_query *query) {
+    dw.iterate_over_enumerations(cudie, atenum_query_enumeration_cb, query);
+  }
+  static int atenum_query_enumeration_cb(Dwarf_Die* enumtype_die, Dwarf_Die *enumeration_die, dwarf_atenum_query *query) {
+    query->atenum_query_enumeration(enumtype_die, enumeration_die);
+  }
+private:
+  int atenum_query_enumeration(Dwarf_Die* enumtype_die, Dwarf_Die *enumeration_die, dwarf_atenum_query *query);
+};
+
+/*
+ * Check if the matched enumeration is the one we want
+ */
+int dwarf_atenum_query::atenum_query_enumeration(Dwarf_Die* enumtype_die,
+						 Dwarf_Die *enumeration_die,
+						 dwarf_atenum_query *query)
+{
+  if (e.enumeration_name == dwarf_diename(enumeration_die))
+    {
+      /* The enum type itself determines signedness */
+      Dwarf_Word encoding = (Dwarf_Word) -1;
+      Dwarf_Attribute encoding_attr;
+      if (!dwarf_attr_integrate (enumtype_die, DW_AT_encoding, &encoding_attr)
+	  || dwarf_formudata(&encoding_attr, &encoding) != 0)
+	throw SEMANTIC_ERROR(_("@enum(%s) data-type detection failed: cannot get encoding attribute: %s",
+			       e->eumeration_name.c_str(), encoding),
+			     e->tok);
+
+      if (!(encoding == DW_ATE_signed || encoding == DW_ATE_signed_char
+	     || encoding == DW_ATE_unsigned || encoding == DW_ATE_unsigned_char))
+	throw SEMANTIC_ERROR(_("@enum(%s) data-type detection failed: unexpected encoding-type %d",
+			       e->eumeration_name.c_str(), encoding),
+			     e->tok);
+
+      bool issigned = encoding == DW_ATE_signed || encoding == DW_ATE_signed_char;
+
+      /* Get the enum value */
+      Dwarf_Word enumval = (Dwarf_Word) -1;
+      Dwarf_Attribute constattr;
+      if (!dwarf_attr_integrate(enumeration_die, DW_TAG_const_value, &constattr)
+	  || (issigned && dwarf_formsdata(&constattr, &enumval) != 0)
+	  || (!issigned && dwarf_formudata(&constattr, &enumval) != 0))
+	throw SEMANTIC_ERROR(_("@enum(%s) constant-expansion failed: %s",
+			       e->eumeration_name.c_str(), dwarf_errmsg(-1)),
+			     e->tok);
+
+      /* Sub in a literal. TODO: signedness? */
+      expression *ret = new literal_number(enumval);
+      provide(ret);
+
+      return DWARF_CB_ABORT;
+    }
+  else
+    return DWARF_CB_OK;
+}
+
+/* Search each CU in a module for the target enumeration during @enum expansion */
+void
+dwarf_atenum_query::handle_query_module ()
+{
+  if (result)
+    return;
+
+  if (startswith(tns, "enum "))
+    throw SEMANTIC_ERROR(_("@enum(...) takes an enumeration as a value, not an enum type-name"), e->tok);
+
+  dw.iterate_over_cus(atenum_query_cu, this, null, true);
+}
+
+
+// TODO: share with dwarf_atvar_expanding_visitor from which this was mostly copied
+// Expand @enum into the constant
+void
+dwarf_atenum_expanding_visitor::visit_atenum_op (atenum_op* e)
+{
+  if (is_active_lvalue(e))
+    throw SEMANTIC_ERROR(_("cannot use @enum as an lvalue"), e->tok);
+
+  functioncall* result = NULL;
+
+  // split the module string by ':' for alternatives
+  vector<string> modules;
+  tokenize(e->module, modules, ":");
+  bool userspace_p = false;
+  for (unsigned i = 0; !result && i < modules.size(); ++i)
+    {
+      string& module = modules[i];
+
+      dwflpp* dw;
+      try
+        {
+          userspace_p = is_user_module(module);
+          if (!userspace_p)
+            {
+              // kernel or kernel module target
+              dw = db.get_kern_dw(sess, module);
+            }
+          else
+            {
+              module = find_executable(module, "", sess.sysenv);
+              dw = db.get_user_dw(sess, module);
+            }
+        }
+      catch (const semantic_error& er)
+        {
+          /* ignore and go to the next module */
+          continue;
+        }
+
+      // Search each target module with the iterate_over_types callback
+      dwarf_atenum_query q (*dw, module, *e, userspace_p, false, result, tick);
+      dw->iterate_over_modules<base_query>(&query_module, &q);
+
+      if (result)
+        {
+          sess.unwindsym_modules.insert(module);
+          result->visit(this);
+          return;
+        }
+
+      /* Unable to find the variable in the current module, so we chain
+       * an error in atenum_op */
+      string esn = e->sym_name();
+      string mn = module;
+      string cun = e->cu_name;
+      semantic_error  er(ERR_SRC, _F("unable to find enumeration '%s' in %s%s%s",
+                                     esn.c_str(), mn.c_str(),
+                                     cun.empty() ? "" : _(", in "),
+                                     cun.c_str()));
+      if (sess.verbose > 3)
+        clog << "chaining to " << *e->tok << endl
+             << sess.build_error_msg(er) << endl;
+      e->chain (er);
+    }
+
+  provide(e);
+}
+
 
 void
 dwarf_derived_probe::printsig (ostream& o, bool nest) const
@@ -6473,6 +6650,7 @@ struct sdt_uprobe_var_expanding_visitor: public var_expanding_visitor
   void visit_target_symbol_context (target_symbol* e);
   void visit_atvar_op (atvar_op* e);
   void visit_cast_op (cast_op* e);
+  void visit_atenum_op (atenum_op* e);
 };
 
 void
@@ -7457,6 +7635,12 @@ sdt_uprobe_var_expanding_visitor::visit_cast_op (cast_op* e)
   var_expanding_visitor::visit_cast_op(e);
 }
 
+void
+sdt_uprobe_var_expanding_visitor::visit_atenum_op (atenum_op* e)
+{
+    throw SEMANTIC_ERROR(_F("@enum(...) resolution is not available in uprobes. Try a DWARF probe.",
+			    e->tok);
+}
 
 void
 plt_expanding_visitor::visit_target_symbol (target_symbol *e)
@@ -11281,6 +11465,7 @@ is_signed_type(Dwarf_Die *die)
   switch (dwarf_tag(die))
     {
     case DW_TAG_base_type:
+    case DW_TAG_enumeration_type:
       {
         Dwarf_Attribute attr;
         Dwarf_Word encoding = (Dwarf_Word) -1;
diff --git a/translate.cxx b/translate.cxx
index 10b3d320e..23030bd57 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -237,6 +237,7 @@ struct c_unparser: public unparser, public visitor
   void visit_cast_op (cast_op* e);
   void visit_autocast_op (autocast_op* e);
   void visit_atvar_op (atvar_op* e);
+  void visit_atenum_op (atenum_op* e);
   void visit_defined_op (defined_op* e);
   void visit_entry_op (entry_op* e);
   void visit_perf_op (perf_op* e);
@@ -5347,6 +5348,11 @@ c_unparser::visit_atvar_op (atvar_op* e)
   throw SEMANTIC_ERROR(_("cannot translate general @var expression"), e->tok);
 }
 
+void
+c_unparser::visit_atenum_op (atenum_op* e)
+{
+  throw SEMANTIC_ERROR(_("cannot translate general @enum expression"), e->tok);
+}
 
 void
 c_unparser::visit_cast_op (cast_op* e)
-- 
2.29.2


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Command line systemtap macro setting
  2021-01-15  2:18     ` Craig Ringer
@ 2021-01-15  2:38       ` Craig Ringer
  0 siblings, 0 replies; 4+ messages in thread
From: Craig Ringer @ 2021-01-15  2:38 UTC (permalink / raw)
  To: Frank Ch. Eigler, systemtap

On Fri, 15 Jan 2021 at 10:18, Craig Ringer <craig@2ndquadrant.com> wrote:

> On Wed, 13 Jan 2021 at 23:04, Frank Ch. Eigler <fche@redhat.com> wrote:
>
>> Hi -
>>
>> > Any thoughts on this? I'd be happy to submit a patch but I'd like to
>> know
>> > if you think it's reasonable to do first, so I don't spend the time
>> writing
>> > something that won't be committable.
>> > It'd be a big usability improvement in systemtap for me to be able to
>> > define systemtap language macros on the command line.
>>
>> Thanks so much for doing the Work!  It's certainly a reasonable
>> self-serve kind of way to do it outside the tool.
>>
>> I really think tho that stap translator proper should do this, and am
>> pretty sure we can.  It'd be some hacking around within tapsets.cxx or
>> loc2stap.cxx or somesuch, but it wouldn't be too much.  The enum
>> values for $FOO_BAR could translate straight to literals.  Would you
>> have any interest in trying to hack on that?
>>
>
> I made an attempt at doing this by implementing an @enum language feature,
> but landed up getting totally bogged.
>
>

Note that @enum wouldn't help with macros.

For them we'd need -ggdb3 builds of the target binaries, probably processed
by dwz, which is sadly not yet standard in packages but is easy enough to
build when required. And we'd need a way to evaluate simple
const-expressions in macros.

One possible approach for macro expression evaluation would be to let the C
compiler that processes the tap module evaluate the macros, such that with
something like:

     @c_macro(FOO)

it might expand to the code:

     (%{ (<<literal text of C macro FOO from DWARF>>)  %})

so if the original C program defined

    #define FOO 2<<4

the generated stap code would be

    (%{ (2<<4) %})

But  that would not work if FOO referenced other macros, or if its
expression evaluated to a different value in kernel code to user code,
referred to typedefs or types not available, etc. Consider for example
macros based on sizeof() or offsetof(). It could also be quite unsafe.

A safer option would probably be to use something like LLVM to evaluate the
macro-expressions in the stap executable during code expansion. But that's
way beyond the scope of anything I can tackle right now, and could still
have issues with missing types, different include paths, different compiler
options, etc affecting the expansion of nontrivial macros.

The only reasonably reliable way I can see of handling macros is getting
the help of the target program. Perhaps systemtap's 'dtrace' script or an
auxillary could automate this though. If this was done as a "macros.d" for
example, one might write:

    # file macros.d
    #include "application_headers_needed.h"
    provider "foo" {
        string_macro("BAR");
        macro("int","BAZ");
    }

and the dtrace script might process it to a macros.c file:

    #include "application_headers_needed.h"

    extern const char _stp_macro_foo_BAR[] __attribute__ ((section
(".stap.macros")));
    const char _stp_macro_foo_BAR[] __attribute__ ((section
(".stap.macros"))) = (BAR);

    extern const int BAZ __attribute__ ((section (".stap.macros")));
    const int BAZ __attribute__ ((section (".stap.macros"))) = (BAZ);

which the program would then compile and link.

The same approach would make it possible for <sys/sdt.h> to expose
STAP_MACRO helpers for use in program code directly, like

    #define SHOULD_BE_AN_ENUM 4

    #include <sys/sdt.h>

    STAP_MACRO(int,SHOULD_BE_AN_ENUM);

It's not as transparent as I'd like, but people seem to cope ok with
probes.d, it'd be possible to autogenerate it for many programs, it'd be
relatively easy to add, and no debuginfo at all would be required.

-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 2ndQuadrant - PostgreSQL Solutions for the Enterprise

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-01-15  2:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-08  5:07 Command line systemtap macro setting Craig Ringer
2021-01-13  4:39 ` Craig Ringer
     [not found]   ` <20210113150431.GC25762@redhat.com>
2021-01-15  2:18     ` Craig Ringer
2021-01-15  2:38       ` Craig Ringer

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).