Skip to content

atomic_add impl report error by ThreadSanitizer #28

@moonlightsh

Description

@moonlightsh

hello,
I learn a lot from you project,
When I write a multi-thread program and test.
I use tsan to check data race, reports error:

0x7b0c00000328
==================
WARNING: ThreadSanitizer: data race (pid=48322)
  Read of size 8 at 0x7b0c00000318 by thread T1:
    #0 sref mman.c:91 (a.out:x86_64+0x100002c47)
    #1 thr main.c:16 (a.out:x86_64+0x100002655)
old_count: 2
  Previous atomic write of size 8 at 0x7b0c00000318 by main thread:
    #0 sref mman.c:91 (a.out:x86_64+0x100002c95)
    #1 main main.c:31 (a.out:x86_64+0x1000027a6)
old_count: 2
  Location is heap block of size 48 at 0x7b0c00000300 allocated by main thread:
    #0 malloc <null>:188732607 (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x55a7c)
    #1 smalloc_impl mman.c:163 (a.out:x86_64+0x10000349d)
    #2 smalloc mman.c:212 (a.out:x86_64+0x100003341)
    #3 main main.c:26 (a.out:x86_64+0x1000026ed)

  Thread T1 (tid=486984, running) created by main thread at:
    #0 pthread_create <null>:188732607 (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x3155f)
    #1 main main.c:30 (a.out:x86_64+0x100002794)

there is a simple demo just for test:

#include <stdio.h>
#include <pthread.h>
#include <assert.h>
#include <csptr/smart_ptr.h>

struct T {
    int foo;
};

static void cleanupfunc(void *ptr, void *meta) {
    printf("cleanupfunc\n");
}

void *thr(void *p)
{
    sref(p);
    sfree(p);
    return NULL;
}

int main()
{
    pthread_t t1;
    pthread_t t2;
    pthread_t t3;
    smart struct T *p = shared_ptr(struct T, {}, cleanupfunc);
    
    sref(p);
    pthread_create(&t1, NULL, thr, p);
    sref(p);
    pthread_create(&t2, NULL, thr, p);
    sref(p);
    pthread_create(&t3, NULL, thr, p);
    pthread_join(t1, NULL);
    pthread_join(t2, NULL);
    pthread_join(t3, NULL);
    sfree(p);
    sfree(p);
    sfree(p);
}

I check the code, found maybe the non-sync access for count ,

libcsptr/src/mman.c

Lines 43 to 52 in ac73451

static CSPTR_INLINE size_t atomic_add(volatile size_t *count, const size_t limit, const size_t val) {
size_t old_count, new_count;
do {
old_count = *count;
if (old_count == limit)
abort();
new_count = old_count + val;
} while (!__sync_bool_compare_and_swap(count, old_count, new_count));
return new_count;
}

But in think it not a real issue ,unless in x64. I got a way to avoid from abort of tsan, is it just make it happier?

static CSPTR_INLINE size_t atomic_add(volatile size_t *count, const size_t limit, const size_t val) {
    size_t old_count, new_count;
    do {
      old_count = __sync_fetch_and_add(count, 0);
      if (old_count == limit)
          abort();
      new_count = old_count + val;
    } while (!__sync_bool_compare_and_swap(count, old_count, new_count));
    return new_count;
}

also commit :
983932

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions