2007-01-19 Josh Stone <joshua.i.stone@intel.com>

	PR 3079
runtime/
	* loc2c-runtime.h (kread, kwrite): New macros to safely read/write
	values from kernel pointers.  This includes a workaround for 64-bit
	numbers on i386 platforms.
testsuite/
	* systemtap.base/deref.stp: Use the new kread macro that should work
	fine with 64-bit numbers on i386 platforms.  Also expand the test to
	include writes with kwrite.
	* systemtap.base/deref.exp: Remove the setup_kfail.

5 files changed, 91 insertions, 11 deletions

diff --git a/runtime/ChangeLog b/runtime/ChangeLog
index 500dd25b..d14c5b8f 100644
--- a/runtime/ChangeLog
+++ b/runtime/ChangeLog
@@ -1,3 +1,10 @@
+2007-01-19  Josh Stone  <joshua.i.stone@intel.com>
+
+	PR 3079
+	* loc2c-runtime.h (kread, kwrite): New macros to safely read/write
+	values from kernel pointers.  This includes a workaround for 64-bit
+	numbers on i386 platforms.
+
 2007-01-10  Martin Hunt  <hunt@redhat.com>
 	PR 3708
 	* map.c (str_copy): Check for NULL pointers.
diff --git a/runtime/loc2c-runtime.h b/runtime/loc2c-runtime.h
index c51908a9..4617f697 100644
--- a/runtime/loc2c-runtime.h
+++ b/runtime/loc2c-runtime.h
@@ -1,6 +1,6 @@
 /* target operations
  * Copyright (C) 2005 Red Hat Inc.
- * Copyright (C) 2005,2006 Intel Corporation.
+ * Copyright (C) 2005, 2006, 2007 Intel Corporation.
  *
  * This file is part of systemtap, and is free software.  You can
  * redistribute it and/or modify it under the terms of the GNU General
@@ -461,3 +461,41 @@
     *_d = '\0';								      \
     (dst);								      \
   })
+
+
+#if defined __i386__
+
+/* x86 can't do 8-byte put/get_user_asm, so we have to split it */
+
+#define kread(ptr)						\
+  ({ 								\
+    typeof(*(ptr)) _r; 						\
+    if (sizeof(*(ptr)) == 8) { 					\
+      union { u64 q; u32 l[2]; } _q; 				\
+      _q.l[0] = (u32) deref(4, &((u32 *)(ptr))[0]); 		\
+      _q.l[1] = (u32) deref(4, &((u32 *)(ptr))[1]); 		\
+      _r = (typeof(*(ptr))) _q.q; 				\
+    } else 							\
+      _r = (typeof(*(ptr))) deref(sizeof(*(ptr)), (ptr)); 	\
+    _r; 							\
+  })
+
+#define kwrite(ptr, value)						\
+  ({ 									\
+    if (sizeof(*(ptr)) == 8) { 						\
+      union { u64 q; u32 l[2]; } _q; 					\
+      _q.q = (u64)(typeof(*(ptr)))(value);				\
+      store_deref(4, &((u32 *)(ptr))[0], _q.l[0]);			\
+      store_deref(4, &((u32 *)(ptr))[1], _q.l[1]);			\
+    } else 								\
+      store_deref(sizeof(*(ptr)), (ptr), (typeof(*(ptr)))(value));	\
+  })
+
+#else
+
+#define kread(ptr) \
+  ( (typeof(*(ptr))) deref(sizeof(*(ptr)), (ptr)) )
+#define kwrite(ptr, value) \
+  ( store_deref(sizeof(*(ptr)), (ptr), (typeof(*(ptr)))(value)) )
+
+#endif
diff --git a/testsuite/ChangeLog b/testsuite/ChangeLog
index 22b78b0b..92e9a0ab 100644
--- a/testsuite/ChangeLog
+++ b/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2007-01-19  Josh Stone  <joshua.i.stone@intel.com>
+
+	PR 3079
+	* systemtap.base/deref.stp: Use the new kread macro that should work
+	fine with 64-bit numbers on i386 platforms.  Also expand the test to
+	include writes with kwrite.
+	* systemtap.base/deref.exp: Remove the setup_kfail.
+
 2007-01-16  Mike Mason <mmlnx@us.ibm.com>
 
 	* buildok/eighteen.stp: changed to find __audit_getname or
diff --git a/testsuite/systemtap.base/deref.exp b/testsuite/systemtap.base/deref.exp
index 0a44bfac..1f00f00f 100644
--- a/testsuite/systemtap.base/deref.exp
+++ b/testsuite/systemtap.base/deref.exp
@@ -4,7 +4,4 @@ load_lib "stap_run.exp"
 
 set test "deref"
 
-# PR 3079: deref macro cannot read 64-bit values on 32-bit x86
-setup_kfail i?86-*-* 3079
-
 stap_run $srcdir/$subdir/$test.stp no_load $all_pass_string -g
diff --git a/testsuite/systemtap.base/deref.stp b/testsuite/systemtap.base/deref.stp
index 6fb59f2c..d17fcac8 100644
--- a/testsuite/systemtap.base/deref.stp
+++ b/testsuite/systemtap.base/deref.stp
@@ -8,19 +8,43 @@ probe begin {  log("systemtap starting probe")  }
 probe end   {  log("systemtap ending probe")    }
 
 
-function call_deref:long(val:long) %{
+function call_kread:long(val:long) %{
   if ((uint64_t)THIS->val < 0x100ULL) {
     uint8_t local8 = (uint8_t)THIS->val;
-    THIS->__retvalue = (uint8_t)deref(sizeof(local8), &local8);
+    THIS->__retvalue = kread(&local8);
   } else if ((uint64_t)THIS->val < 0x10000ULL) {
     uint16_t local16 = (uint16_t)THIS->val;
-    THIS->__retvalue = (uint16_t)deref(sizeof(local16), &local16);
+    THIS->__retvalue = kread(&local16);
   } else if ((uint64_t)THIS->val < 0x100000000ULL) {
     uint32_t local32 = (uint32_t)THIS->val;
-    THIS->__retvalue = (uint32_t)deref(sizeof(local32), &local32);
+    THIS->__retvalue = kread(&local32);
   } else {
     uint64_t local64 = (uint64_t)THIS->val;
-    THIS->__retvalue = (uint64_t)deref(sizeof(local64), &local64);
+    THIS->__retvalue = kread(&local64);
+  }
+  if (0) {
+deref_fault:
+    CONTEXT->last_error = "pointer dereference error";
+  }
+%}
+
+function call_kwrite:long(val:long) %{
+  if ((uint64_t)THIS->val < 0x100ULL) {
+    uint8_t local8;
+    kwrite(&local8, THIS->val);
+    THIS->__retvalue = local8;
+  } else if ((uint64_t)THIS->val < 0x10000ULL) {
+    uint16_t local16;
+    kwrite(&local16, THIS->val);
+    THIS->__retvalue = local16;
+  } else if ((uint64_t)THIS->val < 0x100000000ULL) {
+    uint32_t local32;
+    kwrite(&local32, THIS->val);
+    THIS->__retvalue = local32;
+  } else {
+    uint64_t local64;
+    kwrite(&local64, THIS->val);
+    THIS->__retvalue = local64;
   }
   if (0) {
 deref_fault:
@@ -29,11 +53,17 @@ deref_fault:
 %}
 
 function check_deref(val) {
-  deref = call_deref(val)
+  deref = call_kread(val)
+  if (deref == val)
+    log("systemtap test success")
+  else
+    printf("systemtap test failure - kread %#x != %#x\n", deref, val)
+
+  deref = call_kwrite(val)
   if (deref == val)
     log("systemtap test success")
   else
-    printf("systemtap test failure - %#x != %#x\n", deref, val)
+    printf("systemtap test failure - kwrite %#x != %#x\n", deref, val)
 }
 
 probe end(1) {