You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by yl...@apache.org on 2021/12/16 17:19:33 UTC

svn commit: r1896068 - in /apr/apr/branches/1.7.x: ./ CHANGES atomic/win32/apr_atomic64.c test/testatomic.c

Author: ylavic
Date: Thu Dec 16 17:19:32 2021
New Revision: 1896068

URL: http://svn.apache.org/viewvc?rev=1896068&view=rev
Log:
Merge r1868129, r1868502 from trunk:


apr_atomic_read64(): Fix non-atomic read on 32-bit Windows.

* atomic/win32/apr_atomic64.c
  (apr_atomic_read64): Use InterlockedCompareExchange64() instead of direct
   memory read.

* test/testatomic.c
  (test_atomics_threaded_setread64): New test.
  (test_func_set64): Helepr for test_atomics_threaded_setread64 test.

* CHANGES: Add changelog entry.


* atomic/win32/apr_atomic64.c
  (apr_atomic_read64): Use direct memory read when compiled for x86_x64, since
   64-bit reads are atomic in 64-bit Windows [1].

Suggested by: Yann Ylavic

[1] https://docs.microsoft.com/en-us/windows/win32/sync/interlocked-variable-access


Submitted by: ivan
Reviewed by: ylavic

Modified:
    apr/apr/branches/1.7.x/   (props changed)
    apr/apr/branches/1.7.x/CHANGES
    apr/apr/branches/1.7.x/atomic/win32/apr_atomic64.c
    apr/apr/branches/1.7.x/test/testatomic.c

Propchange: apr/apr/branches/1.7.x/
------------------------------------------------------------------------------
  Merged /apr/apr/trunk:r1868129,1868502

Modified: apr/apr/branches/1.7.x/CHANGES
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/CHANGES?rev=1896068&r1=1896067&r2=1896068&view=diff
==============================================================================
--- apr/apr/branches/1.7.x/CHANGES [utf-8] (original)
+++ apr/apr/branches/1.7.x/CHANGES [utf-8] Thu Dec 16 17:19:32 2021
@@ -6,6 +6,8 @@ Changes for APR 1.7.1
      (This issue was addressed as CVE-2017-12613 in APR 1.6.3 and
      later 1.6.x releases, but was missing in 1.7.0.)  [Stefan Sperling]
 
+  *) apr_atomic_read64(): Fix non-atomic read on 32-bit Windows [Ivan Zhakov]
+
   *) configure: Add --disable-sctp argument to forcibly disable SCTP
      support, or --enable-sctp which fails if SCTP support is not
      detected.  [Lubos Uhliarik <luhliari redhat.com>, Joe Orton]

Modified: apr/apr/branches/1.7.x/atomic/win32/apr_atomic64.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/atomic/win32/apr_atomic64.c?rev=1896068&r1=1896067&r2=1896068&view=diff
==============================================================================
--- apr/apr/branches/1.7.x/atomic/win32/apr_atomic64.c (original)
+++ apr/apr/branches/1.7.x/atomic/win32/apr_atomic64.c Thu Dec 16 17:19:32 2021
@@ -51,7 +51,16 @@ APR_DECLARE(void) apr_atomic_set64(volat
 
 APR_DECLARE(apr_uint64_t) apr_atomic_read64(volatile apr_uint64_t *mem)
 {
+#if defined(_M_X64)
+    /* https://docs.microsoft.com/en-us/windows/win32/sync/interlocked-variable-access
+     * "Simple reads and writes to properly aligned 64-bit variables are atomic
+     * on 64-bit Windows."*/
     return *mem;
+#else
+    /* 64-bit read is not atomic on 32-bit platform: use InterlockedCompareExchange
+       to perform atomic read. */
+    return InterlockedCompareExchange64((volatile LONG64 *)mem, 0, 0);
+#endif
 }
 
 APR_DECLARE(apr_uint64_t) apr_atomic_cas64(volatile apr_uint64_t *mem, apr_uint64_t with,

Modified: apr/apr/branches/1.7.x/test/testatomic.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/test/testatomic.c?rev=1896068&r1=1896067&r2=1896068&view=diff
==============================================================================
--- apr/apr/branches/1.7.x/test/testatomic.c (original)
+++ apr/apr/branches/1.7.x/test/testatomic.c Thu Dec 16 17:19:32 2021
@@ -876,6 +876,43 @@ static void test_atomics_busyloop_thread
     ABTS_ASSERT(tc, "Failed creating threads", rv == APR_SUCCESS);
 }
 
+void *APR_THREAD_FUNC test_func_set64(apr_thread_t *thd, void *data)
+{
+    int i;
+
+    for (i = 0; i < 1000 * 1000; i++) {
+        apr_atomic_set64(&atomic_ops64, APR_UINT64_C(0x1111222233334444));
+        apr_atomic_set64(&atomic_ops64, APR_UINT64_C(0x4444555566667777));
+    }
+
+    apr_thread_exit(thd, APR_SUCCESS);
+    return NULL;
+}
+
+static void test_atomics_threaded_setread64(abts_case *tc, void *data)
+{
+    apr_status_t retval;
+    apr_thread_t *thread;
+    int i;
+
+    apr_atomic_set64(&atomic_ops64, APR_UINT64_C(0x1111222233334444));
+
+    apr_thread_create(&thread, NULL, test_func_set64, NULL, p);
+
+    for (i = 0; i < 1000 * 1000 * 2; i++) {
+        apr_uint64_t val = apr_atomic_read64(&atomic_ops64);
+
+        if (val != APR_UINT64_C(0x1111222233334444) &&
+            val != APR_UINT64_C(0x4444555566667777))
+        {
+            ABTS_FAIL(tc, "Unexpected value");
+            break;
+        }
+    }
+
+    apr_thread_join(&retval, thread);
+}
+
 #endif /* !APR_HAS_THREADS */
 
 abts_suite *testatomic(abts_suite *suite)
@@ -916,6 +953,7 @@ abts_suite *testatomic(abts_suite *suite
     abts_run_test(suite, test_atomics_threaded64, NULL);
     abts_run_test(suite, test_atomics_busyloop_threaded, NULL);
     abts_run_test(suite, test_atomics_busyloop_threaded64, NULL);
+    abts_run_test(suite, test_atomics_threaded_setread64, NULL);
 #endif
 
     return suite;