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

[Hans Wennborg via Openmp-commits]

Thanks!

On Thu, Jan 19, 2017 at 3:04 PM, Peyton, Jonathan L
<jonathan.l.peyton at intel.com> wrote:
> Seems we are all in agreement.  I will merge both r292348 and 292349 into release_40 branch.
>
> -- Johnny
>
> -----Original Message-----
> From: Churbanov, Andrey
> Sent: Thursday, January 19, 2017 12:45 PM
> To: Hans Wennborg <hans at chromium.org>; Hahnfeld, Jonas <Hahnfeld at itc.rwth-aachen.de>
> Cc: Peyton, Jonathan L <jonathan.l.peyton at intel.com>; openmp-commits at lists.llvm.org
> Subject: RE: [Openmp-commits] [openmp] r292349 - Fix memory error in case of reinit using kmp_set_defaults() for lock code.
>
> 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