RE: More PTHREAD_PROCESS_SHARED(Storing pointers into shared memory?)

I've submitted a patch to ngpt-2.2.0 that enhances ngptc to allow a size
and address to be specified when

it runs, which should address this. It did introduce some sticky
problems with the pth_shared_segment as 

there now is a new layout for it which may break a concurrent old
runtime that expects the old layout but 

doesn't check.

 

Apply the attached patches as follows:

            # tar xvf ngpt-2.2.0.tar

            # cd ngpt-2.2.0

            # patch -p1 < patch-ngpt-2.2.0-pshared_2

            # patch -p1 < patch-ngpt-2.2.0-ngptc_2

and rerun configure and build ngpt-2.2.0 as normal. See RELEASE for
utility interface info to ngptinit/ngptc

to use the new functionality.

 

These are my opinions and not official opinions of Intel Corp.

 

David Howell

Intel Corporation

Telco Server Development

Server Products Division

Voice: (803) 461-6112  Fax: (803) 461-6292

 

Intel Corporation

Columbia Design Center, CBA-2

250 Berryhill Road, Suite 100

Columbia, SC 29210

 

[email protected]

 

-----Original Message-----
From: Chris McFarlen [mailto:[email protected]] 
Sent: Sunday, January 26, 2003 1:24 PM
To: Howell, David P; [email protected];
[email protected]
Subject: RE: [pthreads-devel] More PTHREAD_PROCESS_SHARED(Storing
pointers into shared memory?)

 

I submit this patch to you to show what I did to fix the problem.  I
hesitate to recommend the patch, because it creates invalid pointers
that must be checked before they are used.  Basically, when allocating a
shared mutex, return array index of the mutex + 1(so you can still have
NULL pointers).  Then, before actually de-referencing the pointer, check
to see if the mutex pointer is really an index and fix the pointer.  I
have been running this patch for a couple of weeks without any
problems...

Thanks for the reply, 
Chris McFarlen 
T-NETIX, Inc. 

diff -urNp ngpt-2.2.0/pth_sync.c ngpt-2.2.0-my/pth_sync.c 
--- ngpt-2.2.0/pth_sync.c       2002-12-03 10:21:56.000000000 -0600 
+++ ngpt-2.2.0-my/pth_sync.c    2003-01-16 13:32:57.000000000 -0600 
@@ -82,6 +82,24 @@ intern int               pth_shared_fd = 0; 
 intern pth_shared_area_t    *pth_shared_area; 
 intern int                 PTH_SHARED_SIZE = (PTH_MAX_SHARED_OBJECTS *
sizeof(struct pth_shared_sync_st) + sizeof(struct pth_qlock_st));

  
+pth_mutex_t *pth_fix_mutex_ptr(pth_mutex_t *mtx) 
+{ 
+   if (((int)mtx) <= PTH_MAX_SHARED_OBJECTS) { 
+      //fprintf(stderr, "fixup mutex: ptr: %p indx: %i\n",
&pth_shared_area->o[((int)mtx)-1].u.mx, mtx-1); 
+      return &pth_shared_area->o[((int)mtx)-1].u.mx; 
+   } 
+   return mtx; 
+} 
+ 
+pth_cond_t *pth_fix_cond_ptr(pth_cond_t *cn) 
+{ 
+   if (((int)cn) <= PTH_MAX_SHARED_OBJECTS) { 
+      return &pth_shared_area->o[((int)cn)-1].u.cn; 
+   } 
+   return cn; 
+} 
+ 
+ 
 /* 
 **  Mutual Exclusion Locks 
 */ 
@@ -115,6 +133,8 @@ intern int pth_find_shared_mutex(pth_mut 
 { 
     int indx = 0; 
  
+    mutex = pth_fix_mutex_ptr(mutex); 
+ 
     pth_acquire_lock(&(pth_shared_area->lock)); 
     while (pth_shared_area->o[indx].used == TRUE) { 
        if (mutex == &pth_shared_area->o[indx].u.mx) { 
@@ -152,7 +172,8 @@ intern pth_mutex_t *pth_alloc_shared_mut 
  
     pth_release_lock(&(pth_shared_area->lock)); 
  
-    return rmx; 
+    //fprintf(stderr, "alloc shared: ptr: %p index: %i\n", rmx, indx); 
+    return (pth_mutex_t *)indx+1; 
 } 
  
 intern pth_cond_t *pth_alloc_shared_cond(void) 
@@ -180,11 +201,13 @@ intern pth_cond_t *pth_alloc_shared_cond 
  
     pth_release_lock(&(pth_shared_area->lock)); 
  
-    return rcn; 
+    return (pth_cond_t *)indx+1; 
 } 
  
 int pth_mutex_init(pth_mutex_t *mutex, pth_mutexattr_t *pattr) 
 { 
+    mutex = pth_fix_mutex_ptr(mutex); 
+ 
     int pshared = (pattr != NULL) ? pattr->pshared : FALSE; 
     if (mutex == NULL) 
        return FALSE; 
@@ -231,6 +254,8 @@ int pth_mutex_acquire(pth_mutex_t *mutex 
     pth_event_t ev; 
     int retcode = 0; 
  
+    mutex = pth_fix_mutex_ptr(mutex); 
+ 
     if (mutex->mx_state & PTH_MUTEX_INTERNAL_LOCKED) { 
        _pth_acquire_lock(&(mutex->mx_lock), 0); 
        return 0; 
@@ -561,6 +586,8 @@ int pth_mutex_release(pth_mutex_t *mutex 
     pth_descr_t descr = pth_get_native_descr(); 
     pth_list_t *mq; 
  
+    mutex = pth_fix_mutex_ptr(mutex); 
+ 
     if (!(mutex->mx_state & PTH_MUTEX_LOCKED)) { 
        if (mutex->mx_state & PTH_MUTEX_INTERNAL_LOCKED) { 
            _pth_release_lock(&(mutex->mx_lock), 0); 
@@ -643,7 +670,7 @@ intern void pth_mutex_releaseall(pth_t t 
        pth_mutex_release(thread->mutex_owned); 
     rn = rnf = pth_ring_first(&(thread->mutexring)); 
     while (rn != NULL) { 
-       m = (pth_mutex_t *)((char *)rn - (int)&((pth_mutex_t
*)0)->mx_node); 
+       m = pth_fix_mutex_ptr((pth_mutex_t *)((char *)rn -
(int)&((pth_mutex_t *)0)->mx_node)); 
         pth_mutex_release(m); 
  
        /* 
@@ -664,6 +691,8 @@ intern void pth_mutex_releaseall(pth_t t 
  
 int pth_mutex_destroy(pth_mutex_t *mutex) 
 { 
+    mutex = pth_fix_mutex_ptr(mutex); 
+ 
     /* 
      * Sanity check... 
      * This only valid for shared mutexes. 
@@ -810,6 +839,8 @@ int pth_rwlock_release(pth_rwlock_t *rwl 
  
 int _pth_cond_init(pth_cond_t *cond, int pshared) 
 { 
+    cond = pth_fix_cond_ptr(cond); 
+ 
     rfutex_init(&cond->cn_shared, cond, pshared); 
     if (pshared != TRUE) 
        cond->cn_index = -1; 
@@ -825,6 +856,8 @@ int _pth_cond_init(pth_cond_t *cond, int 
  
 int pth_cond_destroy(pth_cond_t *cond) 
 { 
+    cond = pth_fix_cond_ptr(cond); 
+ 
     /* 
      * Sanity check... 
      * This only valid for shared cond variables. 
@@ -884,6 +917,8 @@ int pth_cond_await(pth_cond_t *cond, pth 
     pth_t current= descr->current; 
     int futx_fd = 0; 
  
+    cond = pth_fix_cond_ptr(cond); 
+ 
     /* consistency checks */ 
     if ((!(cond->cn_state & PTH_COND_INITIALIZED)) || (mutex->mx_owner
!= current)) 
         return FALSE; 
@@ -1010,6 +1045,8 @@ static int pth_check_waiters(void *arg) 
 { 
     pth_cond_t *cond = (pth_cond_t *)arg; 
  
+    cond = pth_fix_cond_ptr(cond); 
+ 
     if (!cond->cn_waiters) 
        return TRUE; 
     else 
@@ -1024,6 +1061,8 @@ int pth_cond_notify(pth_cond_t *cond, in 
     char c = (int)1; 
     pth_descr_t ds = NULL; 
     
+    cond = pth_fix_cond_ptr(cond); 
+ 
     /* consistency checks */ 
     if (!(cond->cn_state & PTH_COND_INITIALIZED)) 
         return FALSE; 

 

> -----Original Message----- 
> From: Howell, David P [mailto:[email protected]] 
> Sent: Friday, January 24, 2003 5:08 PM 
> To: [email protected]; [email protected] 
> Subject: RE: [pthreads-devel] More 
> PTHREAD_PROCESS_SHARED(Storing pointers into shared memory?) 
> 
> 
> This is clearly a bug, seems like we have to nail the NGPT 
> shared area to a 
> known virtual address that all processes using shared mutexes 
> would use. It 
> is unfortunate but necessary to add restrictions like this 
> with the current implementation, we should investigate other 
> ways to implement this. 
> 
> I just modified pth_sync.c in the call to mmap to use a start 
> address of a 
> constant 0x48000000, this should keep it at the same virtual 
> address for all processes that use the ngpt runtime. Is there 
> a better place to put this to keep it out of the way of other 
> mmaps, or just to place it better for efficiency? Also, the 
> man page says that 'start' is a hint only, that's scary for a 
> solution like this. 
> 
> But this may help folks who are stuck on this to make 
> progress. We could for the short term do this and make it a 
> configuration time tuneable. 
> 
> Any thoughts? 
>  
> Dave Howell 
> 
> -----Original Message----- 
> From: [email protected] [mailto:[email protected]] 
> Sent: Tuesday, January 07, 2003 3:34 PM 
> To: [email protected] 
> Subject: Re: [pthreads-devel] More 
> PTHREAD_PROCESS_SHARED(Storing pointers into shared memory?) 
> 
> 
> 
> I have written code that demonstrates the problem.  This 
> scenario involves two programs(proga, progb) executing the 
> same code that manipulates some shared data protected with a 
> PTHREAD_PROCESS_SHARED mutex. Both programs use the same 
> initialization code to map its shared memory and, if 
> necessary, initialize the mutex.  The only difference between 
> the two is that progb has another library linked in 
> (libbreak). This library doesn't do anything but allocate 
> some memory, but takes up address space causing the internal 
> NGPT shared memory to be mapped at a different address than 
> proga.  The source to everything is in the attached tarball. 
> 
> After initialization, each program acquires the mutex, prints 
> out the shared data, then updates it to be the programs name 
> (argv[0]).  Before each step it asks the user to press enter 
> (this way you can get the other program started). 
> 
> If you run two instances of proga, or two instances of progb, 
> everything works as expected.  Running proga with progb 
> causes the locks not to work (seemingly ignored). 
> 
> I found it useful to add this code to the bottom of 
> pth_initialize_shared(): 
> 
>     fprintf(stderr, "NGPT: shared area pointer: %p\n", 
> pth_shared_area); 
> 
> You can see then, that the memory is mapped to a different 
> address in progb then proga. 
> 
> to test: 
>    untar ngptproblem.tar.gz 
>    ./configure --with-ngptdir=/your/ngpt/path 
>    make 
>    ./proga 
>    (on another tty) 
>    ./progb (or proga if you want to see it work) 
> 
>    The first <enter> locks the mutex, prints out the data, 
> then updates it.  The second <enter> unlocks the mutex.  So 
> hit enter on proga, then hit enter on progb.  The lock wont 
> be honored and the critical section executes(because the 
> pointer in the shared mutex is wrong from progb's perspective). 
> 
> 
> Here is the output from proga: 
> 
> NGPT: shared area pointer: 0x40195000 
> internal mutex pointer: 0x40198200 
> press a key to obtain the lock and update data 
> 
> locking mutex... 
> locked 
> shared data is '' 
> press a key to unlock the mutex 
> 
> unlocking mutex... 
> unlocked 
> 
> And here from progb: 
> 
> NGPT: shared area pointer: 0x40197000 
> Initializing library 
> press a key to obtain the lock and update data 
> 
> locking mutex... 
> locked 
> shared data is './proga' 
> press a key to unlock the mutex 
> 
> unlocking mutex... 
> unlocked 
> 
> Thanks for taking a look, 
> Chris McFarlen 
> _______________________________________________ 
> pthreads-devel mailing list 
> [email protected] 
> http://www-124.ibm.com/developerworks/oss/mailman/listinfo/pth 
reads-devel