From 2e526dabcf4b15fb102e295b282df3af54d5c9d3 Mon Sep 17 00:00:00 2001
From: "Frank Ch. Eigler" <fche@elastic.org>
Date: Mon, 19 Oct 2009 11:33:24 -0400
Subject: [PATCH] PR10799: warn on possibly uintended local-vs-global namespace
 collision

* elaborate.cxx (find_var): Take extra token parameter.
  Look for cross-file global variable resolution, signal
  a warning.
* testsuite/systemtap.examples/io/traceio2.stp: Fix it.
* testsuite/systemtap.syscall/sys.stp: Fix it.
* NEWS: Document it.
---
 NEWS                                         | 13 +++++++++++++
 elaborate.cxx                                | 20 +++++++++++++++-----
 elaborate.h                                  |  2 +-
 testsuite/systemtap.examples/io/traceio2.stp |  2 +-
 testsuite/systemtap.syscall/sys.stp          | 10 +++++-----
 5 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/NEWS b/NEWS
index 9fe26ba65..2c7ca4a6a 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,18 @@
 * What's new
 
+- Systemtap now warns about global variables being referenced from other
+  script files.  This aims to protect against unintended local-vs-global 
+  namespace collisions such as:
+
+     % cat some_tapset.stp
+     probe baz.one = bar { foo = $foo; bar = $bar }
+     % cat end_user_script.stp
+     global foo # intended to be private variable
+     probe timer.s(1) { foo ++ }
+     probe baz.* { println(foo, pp()) }
+     % stap end_user_script.stp
+     WARNING: cross-file global variable reference to foo from some_tapset.stp
+
 - Preprocessor conditional for kernel configuration testing:
   %( CONFIG_foo == "y" %? ... %) 
 
diff --git a/elaborate.cxx b/elaborate.cxx
index 2446e4f81..c3f29603e 100644
--- a/elaborate.cxx
+++ b/elaborate.cxx
@@ -1711,7 +1711,7 @@ symresolution_info::visit_foreach_loop (foreach_loop* e)
     {
       if (!array->referent)
 	{
-	  vardecl* d = find_var (array->name, e->indexes.size ());
+	  vardecl* d = find_var (array->name, e->indexes.size (), array->tok);
 	  if (d)
 	    array->referent = d;
 	  else
@@ -1760,7 +1760,7 @@ delete_statement_symresolution_info:
     if (e->referent)
       return;
 
-    vardecl* d = parent->find_var (e->name, -1);
+    vardecl* d = parent->find_var (e->name, -1, e->tok);
     if (d)
       e->referent = d;
     else
@@ -1782,7 +1782,7 @@ symresolution_info::visit_symbol (symbol* e)
   if (e->referent)
     return;
 
-  vardecl* d = find_var (e->name, 0);
+  vardecl* d = find_var (e->name, 0, e->tok);
   if (d)
     e->referent = d;
   else
@@ -1818,7 +1818,7 @@ symresolution_info::visit_arrayindex (arrayindex* e)
       if (array->referent)
 	return;
 
-      vardecl* d = find_var (array->name, e->indexes.size ());
+      vardecl* d = find_var (array->name, e->indexes.size (), array->tok);
       if (d)
 	array->referent = d;
       else
@@ -1877,7 +1877,7 @@ symresolution_info::visit_functioncall (functioncall* e)
 
 
 vardecl*
-symresolution_info::find_var (const string& name, int arity)
+symresolution_info::find_var (const string& name, int arity, const token* tok)
 {
   if (current_function || current_probe)
     {
@@ -1912,6 +1912,16 @@ symresolution_info::find_var (const string& name, int arity)
 	&& session.globals[i]->compatible_arity(arity))
       {
 	session.globals[i]->set_arity (arity);
+        if (! session.suppress_warnings)
+          {
+            vardecl* v = session.globals[i];
+            // clog << "resolved " << *tok << " to global " << *v->tok << endl;
+            if (v->tok->location.file != tok->location.file)
+              {
+                session.print_warning ("cross-file global variable reference to " + lex_cast (*v->tok) + " from",
+                                       tok);
+              }
+          }
 	return session.globals[i];
       }
 
diff --git a/elaborate.h b/elaborate.h
index bee71a50e..fec62d598 100644
--- a/elaborate.h
+++ b/elaborate.h
@@ -37,7 +37,7 @@ public:
   derived_probe* current_probe;
   symresolution_info (systemtap_session& s);
 
-  vardecl* find_var (const std::string& name, int arity);
+  vardecl* find_var (const std::string& name, int arity, const token *tok);
   functiondecl* find_function (const std::string& name, unsigned arity);
 
   void visit_block (block *s);
diff --git a/testsuite/systemtap.examples/io/traceio2.stp b/testsuite/systemtap.examples/io/traceio2.stp
index 1abea45d6..797f30624 100755
--- a/testsuite/systemtap.examples/io/traceio2.stp
+++ b/testsuite/systemtap.examples/io/traceio2.stp
@@ -1,6 +1,6 @@
 #! /usr/bin/env stap
 
-global device_of_interest, dev
+global device_of_interest
 
 probe begin {
   /* The following is not the most efficient way to do this.
diff --git a/testsuite/systemtap.syscall/sys.stp b/testsuite/systemtap.syscall/sys.stp
index e3564a15b..79c7ff576 100755
--- a/testsuite/systemtap.syscall/sys.stp
+++ b/testsuite/systemtap.syscall/sys.stp
@@ -1,4 +1,4 @@
-global indent, indent_str, entry
+global indent, indent_str, entry_p
 
 probe begin {
 	indent = 0
@@ -13,22 +13,22 @@ probe begin {
 
 probe syscall.* ? {
 	if (pid() == target()) {
-		if (entry) printf("\n")
+		if (entry_p) printf("\n")
 		printf("%s%s: %s (%s) = ", indent_str[indent], execname(), name, argstr)
 #		printf("%s%s: %s (%s) = ", indent_str[indent], execname(), probefunc(), argstr)
 		indent++
-		entry = 1
+		entry_p = 1
 	}
 }
 
 probe syscall.*.return ? {
 	if (pid() == target()) {
 		if (indent) indent--
-		if (entry)
+		if (entry_p)
 			printf("%s\n", retstr)
 		else
 			printf("%s%s\n", indent_str[indent],retstr)
-		entry = 0
+		entry_p = 0
 	}
 }
 
-- 
2.43.5