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

[Peyton, Jonathan L via Openmp-commits]

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