[Openmp-commits] [openmp] r292349 - Fix memory error in case of reinit using kmp_set_defaults() for lock code.

[Churbanov, Andrey via Openmp-commits]

I support merging this one.

- Andrey

-----Original Message-----
From: hwennborg at google.com [mailto:hwennborg at google.com] On Behalf Of Hans Wennborg
Sent: Thursday, January 19, 2017 9:40 PM
To: Hahnfeld, Jonas <Hahnfeld at itc.rwth-aachen.de>
Cc: Peyton, Jonathan L <jonathan.l.peyton at intel.com>; openmp-commits at lists.llvm.org; Churbanov, Andrey <Andrey.Churbanov at intel.com>
Subject: Re: [Openmp-commits] [openmp] r292349 - Fix memory error in case of reinit using kmp_set_defaults() for lock code.

Sounds reasonable to me. Jonathan, what do you think?

On Wed, Jan 18, 2017 at 12:57 AM, Hahnfeld, Jonas <Hahnfeld at itc.rwth-aachen.de> wrote:
> Is this one a candidate for release_40?
>
> Thanks,
> Jonas
>
>> -----Original Message-----
>> From: Openmp-commits [mailto:openmp-commits-bounces at lists.llvm.org]
>> On Behalf Of Jonathan Peyton via Openmp-commits
>> Sent: Wednesday, January 18, 2017 8:02 AM
>> To: openmp-commits at lists.llvm.org
>> Subject: [Openmp-commits] [openmp] r292349 - Fix memory error in case 
>> of reinit using kmp_set_defaults() for lock code.
>>
>> Author: jlpeyton
>> Date: Wed Jan 18 01:02:21 2017
>> New Revision: 292349
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=292349&view=rev
>> Log:
>> Fix memory error in case of reinit using kmp_set_defaults() for lock code.
>>
>> The lock tables were being reallocated if kmp_set_defaults() was called.
>> In the env_init code it says that the user should be able to switch 
>> between different KMP_CONSISTENCY_CHECK values which is what this 
>> change enables.
>>
>> Added:
>>     openmp/trunk/runtime/test/api/kmp_set_defaults_lock_bug.c
>> Modified:
>>     openmp/trunk/runtime/src/kmp_lock.cpp
>>
>> Modified: openmp/trunk/runtime/src/kmp_lock.cpp
>> URL: http://llvm.org/viewvc/llvm-
>> project/openmp/trunk/runtime/src/kmp_lock.cpp?rev=292349&r1=292348&
>> r2=292349&view=diff
>> ==========================================================
>> ====================
>> --- openmp/trunk/runtime/src/kmp_lock.cpp (original)
>> +++ openmp/trunk/runtime/src/kmp_lock.cpp Wed Jan 18 01:02:21 2017
>> @@ -3573,6 +3573,12 @@ __kmp_init_dynamic_user_locks()
>>          __kmp_indirect_unset = indirect_unset;
>>          __kmp_indirect_test  = indirect_test;
>>      }
>> +    // If the user locks have already been initialized, then return.
>> +    // Allow the switch between different KMP_CONSISTENCY_CHECK
>> values,
>> +    // but do not allocate new lock tables if they have already been
>> +    // allocated.
>> +    if (__kmp_init_user_locks)
>> +        return;
>>
>>      // Initialize lock index table
>>      __kmp_i_lock_table.size = KMP_I_LOCK_CHUNK;
>>
>> Added: openmp/trunk/runtime/test/api/kmp_set_defaults_lock_bug.c
>> URL: http://llvm.org/viewvc/llvm-
>> project/openmp/trunk/runtime/test/api/kmp_set_defaults_lock_bug.c?rev
>> =292349&view=auto
>> ==========================================================
>> ====================
>> --- openmp/trunk/runtime/test/api/kmp_set_defaults_lock_bug.c (added)
>> +++ openmp/trunk/runtime/test/api/kmp_set_defaults_lock_bug.c Wed
>> Jan 18
>> +++ 01:02:21 2017
>> @@ -0,0 +1,53 @@
>> +// RUN: %libomp-compile-and-run
>> +#include <stdio.h>
>> +#include "omp_testsuite.h"
>> +/* The bug occurs if the lock table is reallocated after
>> +   kmp_set_defaults() is called.  If the table is reallocated,
>> +   then the lock will not point to a valid lock object after the
>> +   kmp_set_defaults() call.*/
>> +omp_lock_t lock;
>> +
>> +int test_kmp_set_defaults_lock_bug() {
>> +  /* checks that omp_get_num_threads is equal to the number of
>> +     threads */
>> +  int nthreads_lib;
>> +  int nthreads = 0;
>> +
>> +  nthreads_lib = -1;
>> +
>> +  #pragma omp parallel
>> +  {
>> +    omp_set_lock(&lock);
>> +    nthreads++;
>> +    omp_unset_lock(&lock);
>> +    #pragma omp single
>> +    {
>> +      nthreads_lib = omp_get_num_threads ();
>> +    }  /* end of single */
>> +  } /* end of parallel */
>> +  kmp_set_defaults("OMP_NUM_THREADS");
>> +  #pragma omp parallel
>> +  {
>> +    omp_set_lock(&lock);
>> +    nthreads++;
>> +    omp_unset_lock(&lock);
>> +  } /* end of parallel */
>> +
>> +  return (nthreads == 2*nthreads_lib); }
>> +
>> +int main()
>> +{
>> +  int i;
>> +  int num_failed=0;
>> +  omp_init_lock(&lock);
>> +
>> +  for(i = 0; i < REPETITIONS; i++) {
>> +    if(!test_kmp_set_defaults_lock_bug()) {
>> +      num_failed++;
>> +    }
>> +  }
>> +  omp_destroy_lock(&lock);
>> +  return num_failed;
>> +}
>>
>>
>> _______________________________________________
>> Openmp-commits mailing list
>> Openmp-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/openmp-commits

--------------------------------------------------------------------
Joint Stock Company Intel A/O
Registered legal address: Krylatsky Hills Business Park,
17 Krylatskaya Str., Bldg 4, Moscow 121614,
Russian Federation

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.