2005-09-03 Frank Ch. Eigler <fche@elastic.org>

	PR 1187 prime
	* tapset.cxx (literal_stmt_for_local): Don't automgaically copy
	target char*'s to systemtap strings.
	* tapset/conversions.stp (user_string, kernel_string): New functions.
	* stapfuncs.5.in: Document new functions.

5 files changed, 64 insertions, 30 deletions

diff --git a/ChangeLog b/ChangeLog
index 32364afa..9d9c515c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2005-09-03  Frank Ch. Eigler  <fche@elastic.org>
 
+	PR 1187 prime
+	* tapset.cxx (literal_stmt_for_local): Don't automgaically copy
+	target char*'s to systemtap strings.
+	* tapset/conversions.stp (user_string, kernel_string): New functions.
+	* stapfuncs.5.in: Document new functions.
+
+2005-09-03  Frank Ch. Eigler  <fche@elastic.org>
+
 	PR 1292, by popular request.
 	* parse.cxx (parse_functiondecl): Allow optional value/param type
 	declarations.
diff --git a/stapfuncs.5.in b/stapfuncs.5.in
index 597dc674..e4eedb94 100644
--- a/stapfuncs.5.in
+++ b/stapfuncs.5.in
@@ -70,6 +70,16 @@ the leading "0x".
 string:string (num:long)
 Return a decimal string representation of the given integer.
 
+.TP
+kernel_string:string (addr:long)
+Copy a string from kernel space at given address.  The validation of this
+address is only partial at present.
+
+.TP
+user_string:string (addr:long)
+Copy a string from user space at given address.  The validation of this
+address is only partial at present.
+
 .SS TIMESTAMP
 
 .TP
diff --git a/tapset/context.stp b/tapset/context.stp
index 32b009bf..6d600190 100644
--- a/tapset/context.stp
+++ b/tapset/context.stp
@@ -10,8 +10,9 @@ function print_backtrace () %{
 	}
 %}
 
-function backtrace () %{
+function backtrace:string () %{
 	if (CONTEXT->regs) {
+                /* XXX: is String really necessary for this? */
 		String str = _stp_string_init (0);
 		_stp_stack_sprint (str, CONTEXT->regs, 0);	
 		strlcpy (THIS->__retvalue, _stp_string_ptr(str), MAXSTRINGLEN);
@@ -79,6 +80,7 @@ function euid:long () %{
 function print_stack(stk:string) %{
 	char *ptr = THIS->stk;
 	char *tok = strsep(&ptr, " ");
+	/* XXX: is this header really necessary & accurate? */
 	_stp_printf ("trace for %d (%s)\n", current->pid, current->comm);
 	while (tok && *tok) {
 		_stp_print_cstr(" ");
diff --git a/tapset/conversions.stp b/tapset/conversions.stp
index 901f26a4..50dbfeb0 100644
--- a/tapset/conversions.stp
+++ b/tapset/conversions.stp
@@ -6,3 +6,31 @@ function string:string (num:long) %{
     sprintf (THIS->__retvalue, "%lld", (long long) THIS->num);
 %}
 
+
+function kernel_string:string (addr:long) %{
+  char *destination = THIS->__retvalue;
+  deref_string (destination, THIS->addr, MAXSTRINGLEN);
+  goto success;
+deref_fault: /* branched to from deref() */
+  {
+    static char errmsg[40];
+    snprintf (errmsg, 40, "kernel string copy fault at 0x%p",
+	 (void *) (uintptr_t) THIS->addr);
+    CONTEXT->last_error = errmsg;
+  }
+success: ;
+%}
+
+# NB: accessing user space is hazardous from certain kernel contexts.
+function user_string:string (addr:long) %{
+  long rc = _stp_strncpy_from_user (THIS->__retvalue, 
+		 	            (const char __user*) (uintptr_t) THIS->addr,
+				    MAXSTRINGLEN);
+  if (rc < 0)
+  {
+    static char errmsg[40];
+    snprintf (errmsg, 40, "user string copy fault at 0x%p",
+	 (void *) (uintptr_t) THIS->addr);
+    CONTEXT->last_error = errmsg;
+  }
+%}
diff --git a/tapsets.cxx b/tapsets.cxx
index f2eeb5d1..14031308 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -957,42 +957,28 @@ dwflpp
 	  dwarf_formudata (dwarf_attr_integrate (pointee_typedie, DW_AT_encoding, &attr_mem), 
 			   &pointee_encoding);
 
-	  if (pointee_typetag == DW_TAG_base_type
-	      && (pointee_encoding == DW_ATE_signed_char
-		  || pointee_encoding == DW_ATE_unsigned_char
-		  || ((pointee_encoding == DW_ATE_signed 
-		       || pointee_encoding == DW_ATE_unsigned) && pointee_byte_size == 1)))
-	    {
-	      // We have an (un)signed char* or (un)signed char[], fetch it into 'tmpc' and
-	      // then copy to the return value via something strcpy-ish.
-	      ty = pe_string;
-	      prelude += "intptr_t tmpc = 0;\n";
-	      if (typetag == DW_TAG_array_type)
-		c_translate_array (&pool, 1, module_bias, typedie, &tail, NULL, 0);
-	      else
-		c_translate_pointer (&pool, 1, module_bias, typedie, &tail);
-	      c_translate_addressof (&pool, 1, module_bias, NULL, pointee_typedie, &tail, "tmpc");
-	      postlude += "deref_string (THIS->__retvalue, tmpc, MAXSTRINGLEN);\n";
-	    }
+	  // We have the pointer: cast it to an integral type via &(*(...))
+
+	  // NB: per bug #1187, at one point char*-like types were
+	  // automagically converted here to systemtap string values.
+	  // For several reasons, this was taken back out, leaving
+	  // pointer-to-string "conversion" (copying) to tapset functions.
+
+	  ty = pe_long;
+	  if (typetag == DW_TAG_array_type)
+	    c_translate_array (&pool, 1, module_bias, typedie, &tail, NULL, 0);
 	  else
-	    {
-	      // We have some other pointer: cast it to an integral type via &(*(...))
-	      ty = pe_long;
-	      if (typetag == DW_TAG_array_type)
-		c_translate_array (&pool, 1, module_bias, typedie, &tail, NULL, 0);
-	      else
-		c_translate_pointer (&pool, 1, module_bias, typedie, &tail);
-	      c_translate_addressof (&pool, 1, module_bias, NULL, pointee_typedie, &tail, 
-				     "THIS->__retvalue");
-	    }
+	    c_translate_pointer (&pool, 1, module_bias, typedie, &tail);
+	  c_translate_addressof (&pool, 1, module_bias, NULL, pointee_typedie, &tail, 
+				 "THIS->__retvalue");
 	}
 	break;	
       }
-
+    
     size_t bufsz = 1024;
     char *buf = static_cast<char*>(malloc(bufsz));
     assert(buf);
-
+    
     FILE *memstream = open_memstream (&buf, &bufsz);
     assert(memstream);