Clang build fixes

Clang build fixes

[Tom Stellard]

Hi,

Here are some patches to get systemtap to build with clang.

-Tom

[PATCH 1/8] Fix a few class/struct mixups

[Tom Stellard]

From: Timm Bäder <tbaeder@redhat.com>

This breaks the build with clang, e.g.:

./elaborate.h:47:1: error: 'symresolution_info' defined as a struct here but previously declared as a class; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Werror,-Wmismatched-tags]
---
 dwflpp.h   | 2 +-
 loc2stap.h | 2 +-
 session.h  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/dwflpp.h b/dwflpp.h
index 25e583908..46813805f 100644
--- a/dwflpp.h
+++ b/dwflpp.h
@@ -182,7 +182,7 @@ struct inline_instance_info : base_func_info
 };
 
 struct location;
-struct location_context;
+class location_context;
 
 struct dwflpp
 {
diff --git a/loc2stap.h b/loc2stap.h
index 818314bb2..df60571a2 100644
--- a/loc2stap.h
+++ b/loc2stap.h
@@ -41,7 +41,7 @@ struct location
   { }
 };
 
-class dwflpp;
+struct dwflpp;
 class location_context
 {
 public:
diff --git a/session.h b/session.h
index 38f1d66de..f60da3e1e 100644
--- a/session.h
+++ b/session.h
@@ -123,7 +123,7 @@ struct parse_error: public std::runtime_error
 };
 
 
-class symresolution_info;
+struct symresolution_info;
 
 struct systemtap_session
 {
-- 
2.26.2

[PATCH 2/8] util: Use abs() instead of labs()

[Tom Stellard]

From: Timm Bäder <tbaeder@redhat.com>

Taking the absolute value of unsigned values is pointless, as reported
by clang:

util.cxx:1545:28: error: taking the absolute value of unsigned type 'unsigned long' has no effect [-Werror,-Wabsolute-value]
      unsigned min_score = labs(target.size() - it->size());
                           ^
util.cxx:1545:28: note: remove the call to 'labs' since unsigned values cannot be negative
      unsigned min_score = labs(target.size() - it->size());
---
 util.cxx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util.cxx b/util.cxx
index c31b668a3..bcacacaaf 100644
--- a/util.cxx
+++ b/util.cxx
@@ -1542,7 +1542,7 @@ levenshtein_suggest(const string& target,        // string to match against
 
       // Approximate levenshtein by size-difference only; real score
       // is at least this high
-      unsigned min_score = labs(target.size() - it->size());
+      unsigned min_score = abs(static_cast<signed>(target.size()) - static_cast<signed>(it->size()));
 
       if (min_score > threshold) // min-score too high for threshold
         continue;
-- 
2.26.2

[PATCH 3/8] Add some override specifiers where missing

[Tom Stellard]

From: Timm Bäder <tbaeder@redhat.com>

---
 elaborate.h   | 4 ++--
 translate.cxx | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/elaborate.h b/elaborate.h
index ee68e7b0b..860068620 100644
--- a/elaborate.h
+++ b/elaborate.h
@@ -207,8 +207,8 @@ struct derived_probe: public probe
   // return arguments of probe if there
   virtual void getargs (std::list<std::string> &) const {}
   void printsig_nested (std::ostream &o) const;
-  virtual void collect_derivation_chain (std::vector<probe*> &probes_list) const;
-  virtual void collect_derivation_pp_chain (std::vector<probe_point*> &pp_list) const;
+  virtual void collect_derivation_chain (std::vector<probe*> &probes_list) const override;
+  virtual void collect_derivation_pp_chain (std::vector<probe_point*> &pp_list) const override;
   std::string derived_locations (bool firstFrom = true);
 
   virtual void print_dupe_stamp(std::ostream&) {}
diff --git a/translate.cxx b/translate.cxx
index 53f1d0725..f763c2277 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -300,8 +300,8 @@ struct c_tmpcounter cxx_final: public c_unparser
   // var_declare(), which will forward to the parent c_unparser for output;
   void var_declare(string const&, var const& v) cxx_override;
 
-  void emit_function (functiondecl* fd);
-  void emit_probe (derived_probe* dp);
+  void emit_function (functiondecl* fd) override;
+  void emit_probe (derived_probe* dp) override;
 
   const string& get_compiled_printf (bool print_to_stream,
 				     const string& format) cxx_override;
-- 
2.26.2

[PATCH 4/8] Add missing copy constructors to set1_ref and set1_const_ref

[Tom Stellard]

From: Timm Bäder <tbaeder@redhat.com>

Clang complains about the missing copy constructors if a user-defined
copy assignment operator exists, e.g.:

./bpf-bitset.h:108:19: error: definition of implicit copy constructor for 'set1_const_ref' is deprecated because it has a user-declared copy assignment operator [-Werror,-Wdeprecated-copy]
  set1_const_ref& operator= (const set1_const_ref &);   // not present
                  ^
./bpf-bitset.h:256:12: note: in implicit copy constructor for 'bpf::bitset::set1_const_ref' first required here
    return set1_const_ref(data + w2 * i, w2);
           ^
---
 bpf-bitset.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/bpf-bitset.h b/bpf-bitset.h
index db4eb0846..6c2f74f0f 100644
--- a/bpf-bitset.h
+++ b/bpf-bitset.h
@@ -115,6 +115,7 @@ public:
   static const size_t npos = -1;
 
   set1_const_ref(word_t *d, size_t w) : data(d), words(w) { }
+  set1_const_ref(const set1_const_ref &o) : data(o.data), words(o.words) { }
 
   bool operator!= (const set1_const_ref &o) const
   {
@@ -149,6 +150,7 @@ private:
 
 public:
   set1_ref(size_t *d, size_t w) : set1_const_ref(d, w) { }
+  set1_ref(const set1_ref &o) : set1_const_ref(o.data, o.words) { }
 
   bit_ref operator[] (size_t i)
   {
-- 
2.26.2

[PATCH 5/8] set2: Return this from assignment operator

[Tom Stellard]

From: Timm Bäder <tbaeder@redhat.com>

This should return something but didn't.
---
 bpf-bitset.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/bpf-bitset.h b/bpf-bitset.h
index 6c2f74f0f..4b9402a51 100644
--- a/bpf-bitset.h
+++ b/bpf-bitset.h
@@ -240,6 +240,7 @@ public:
     if (n1 != o.n1 || w2 != o.w2)
       throw_out_of_range("bpf::bitset::set2::operator=");
     memcpy(data, o.data, n1 * w2 * sizeof(word_t));
+    return *this;
   }
 
   set1_ref operator[] (size_t i)
-- 
2.26.2

[PATCH 6/8] Fix -Woverloaded-virtual warnings when building with clang

[Tom Stellard]

---
 elaborate.cxx |  8 +++++++-
 elaborate.h   |  3 ++-
 main.cxx      |  2 +-
 tapsets.cxx   | 15 +++++++++------
 4 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/elaborate.cxx b/elaborate.cxx
index b069fb5ba..28cd326d6 100644
--- a/elaborate.cxx
+++ b/elaborate.cxx
@@ -78,10 +78,16 @@ derived_probe::derived_probe (probe *p, probe_point *l, bool rewrite_loc):
   this->locations.push_back (l);
 }
 
+void
+derived_probe::printsig_nonest (ostream& o) const
+{
+  probe::printsig (o);
+}
 
 void
-derived_probe::printsig (ostream& o, bool nest) const
+derived_probe::printsig (ostream& o) const
 {
+  bool nest = true;
   probe::printsig (o);
 
   if (nest)
diff --git a/elaborate.h b/elaborate.h
index 860068620..935027f40 100644
--- a/elaborate.h
+++ b/elaborate.h
@@ -203,7 +203,8 @@ struct derived_probe: public probe
   virtual void join_group (systemtap_session& s) = 0;
   virtual probe_point* sole_location () const;
   virtual probe_point* script_location () const;
-  virtual void printsig (std::ostream &o, bool nest=true) const;
+  virtual void printsig (std::ostream &o) const override;
+  void printsig_nonest (std::ostream &o) const;
   // return arguments of probe if there
   virtual void getargs (std::list<std::string> &) const {}
   void printsig_nested (std::ostream &o) const;
diff --git a/main.cxx b/main.cxx
index f3a0d9830..b41cb58a6 100644
--- a/main.cxx
+++ b/main.cxx
@@ -177,7 +177,7 @@ printscript(systemtap_session& s, ostream& o)
                     {
                       // We want to print the probe point signature (without the nested components).
                       std::ostringstream sig;
-                      p->printsig(sig, false);
+                      p->printsig_nonest(sig);
 
                       if (s.dump_mode == systemtap_session::dump_matched_probes_vars && isatty(STDOUT_FILENO))
                         o << s.colorize(sig.str(), "source");
diff --git a/tapsets.cxx b/tapsets.cxx
index 52c59fa50..b235e72fd 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -559,7 +559,7 @@ struct dwarf_derived_probe: public generic_kprobe_derived_probe
   interned_string user_lib;
   bool access_vars;
 
-  void printsig (std::ostream &o, bool nest=true) const;
+  void printsig (std::ostream &o) const;
   virtual void join_group (systemtap_session& s);
   void emit_probe_local_init(systemtap_session& s, translator_output * o);
   void getargs(std::list<std::string> &arg_set) const;
@@ -5320,8 +5320,9 @@ dwarf_atvar_expanding_visitor::visit_atvar_op (atvar_op* e)
 
 
 void
-dwarf_derived_probe::printsig (ostream& o, bool nest) const
+dwarf_derived_probe::printsig (ostream& o) const
 {
+  bool nest = true;
   // Instead of just printing the plain locations, we add a PC value
   // as a comment as a way of telling e.g. apart multiple inlined
   // function instances.  This is distinct from the verbose/clog
@@ -10191,7 +10192,7 @@ struct kprobe_derived_probe: public generic_kprobe_derived_probe
   string path;
   string library;
   bool access_var;
-  void printsig (std::ostream &o, bool nest=true) const;
+  void printsig (std::ostream &o) const;
   void join_group (systemtap_session& s);
 };
 
@@ -10320,8 +10321,9 @@ kprobe_derived_probe::kprobe_derived_probe (systemtap_session& sess,
   this->sole_location()->components = comps;
 }
 
-void kprobe_derived_probe::printsig (ostream& o, bool nest) const
+void kprobe_derived_probe::printsig (ostream& o) const
 {
+  bool nest = true;
   sole_location()->print (o);
   o << " /* " << " name = " << symbol_name << "*/";
 
@@ -10553,7 +10555,7 @@ struct hwbkpt_derived_probe: public derived_probe
   string symbol_name;
   unsigned int hwbkpt_access,hwbkpt_len;
 
-  void printsig (std::ostream &o, bool nest) const;
+  void printsig (std::ostream &o) const;
   void join_group (systemtap_session& s);
 };
 
@@ -10620,8 +10622,9 @@ hwbkpt_derived_probe::hwbkpt_derived_probe (probe *base,
   this->sole_location()->components = comps;
 }
 
-void hwbkpt_derived_probe::printsig (ostream& o, bool nest) const
+void hwbkpt_derived_probe::printsig (ostream& o) const
 {
+  bool nest = true;
   sole_location()->print (o);
 
   if (nest)
-- 
2.26.2

[PATCH 7/8] Fix -Wformat-nonliteral and -Wformat warnings with clang

[Tom Stellard]

---
 stapbpf/stapbpf.cxx | 4 ++--
 staprun/common.c    | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/stapbpf/stapbpf.cxx b/stapbpf/stapbpf.cxx
index 933164851..7b614015a 100644
--- a/stapbpf/stapbpf.cxx
+++ b/stapbpf/stapbpf.cxx
@@ -246,7 +246,7 @@ static std::vector<uprobe_data> uprobes;
 
 // TODO: Move fatal() to bpfinterp.h and replace abort() calls in the interpreter.
 // TODO: Add warn() option.
-static void __attribute__((noreturn))
+static void __attribute__((noreturn))  __attribute__ ((format (printf, 1, 2)))
 fatal(const char *str, ...)
 {
   if (module_name)
@@ -773,7 +773,7 @@ kprobe_collect_from_syms(Elf_Data *sym_data, Elf_Data *str_data)
       if (syms[i].st_name < str_data->d_size)
 	name = static_cast<char *>(str_data->d_buf) + syms[i].st_name;
       else
-	fatal("symbol %u has invalid string index\n", i);
+	fatal("symbol %zu has invalid string index\n", i);
       maybe_collect_kprobe(name, i, syms[i].st_shndx, syms[i].st_value);
     }
 }
diff --git a/staprun/common.c b/staprun/common.c
index 6a603cd2b..6f1b3d683 100644
--- a/staprun/common.c
+++ b/staprun/common.c
@@ -683,6 +683,7 @@ int send_request(int type, void *data, int len)
 
 static int use_syslog = 0;
 
+ __attribute__ ((format (printf, 1, 2)))
 void eprintf(const char *fmt, ...)
 {
 	va_list va;
@@ -712,7 +713,7 @@ void print_color(const char *type)
 		char *seq = parse_stap_color(type);
 		if (seq != NULL) {
 			eprintf("\033[");
-			eprintf(seq);
+			eprintf("%s", seq);
 			eprintf("m\033[K");
 			free(seq);
 		}
-- 
2.26.2

[PATCH 8/8] dtrace: Use -o option to specify output file for CPP

[Tom Stellard]

This makes dtrace work correctly with clang-cpp.  clang-cpp does not
support interpreting the second filename as an output file.
---
 dtrace.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dtrace.in b/dtrace.in
index aa5f544a8..7cfe19c60 100644
--- a/dtrace.in
+++ b/dtrace.in
@@ -359,7 +359,7 @@ def main():
     if s_filename != "" and use_cpp:
         (ignore, fname) = mkstemp(suffix=".d")
         cpp = os.environ.get("CPP", "cpp")
-        retcode = call(split(cpp) + includes + defines + [s_filename, fname])
+        retcode = call(split(cpp) + includes + defines + [s_filename, '-o', fname])
         if retcode != 0:
             print("\"cpp includes s_filename\" failed")
             usage()
-- 
2.26.2