Skip to content

Commit

Permalink
memcheck - also tidy up the ctx impl
Browse files Browse the repository at this point in the history
  • Loading branch information
@jeremylt
jeremylt committed Sep 20, 2024
1 parent 31020a9 commit 91b5d27
Show file tree
Hide file tree
Showing 3 changed files with 150 additions and 44 deletions.
175 changes: 136 additions & 39 deletions backends/memcheck/ceed-memcheck-qfunctioncontext.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ static int CeedQFunctionContextHasValidData_Memcheck(CeedQFunctionContext ctx, b
CeedQFunctionContext_Memcheck *impl;

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
*has_valid_data = impl->data;
*has_valid_data = !!impl->data_allocated;
return CEED_ERROR_SUCCESS;
}

Expand All @@ -30,9 +30,10 @@ static int CeedQFunctionContextHasValidData_Memcheck(CeedQFunctionContext ctx, b
static int CeedQFunctionContextHasBorrowedDataOfType_Memcheck(CeedQFunctionContext ctx, CeedMemType mem_type, bool *has_borrowed_data_of_type) {
CeedQFunctionContext_Memcheck *impl;

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only set HOST memory for this backend");
*has_borrowed_data_of_type = impl->data_borrowed;

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
*has_borrowed_data_of_type = !!impl->data_borrowed;
return CEED_ERROR_SUCCESS;
}

Expand All @@ -43,52 +44,97 @@ static int CeedQFunctionContextSetData_Memcheck(CeedQFunctionContext ctx, CeedMe
size_t ctx_size;
CeedQFunctionContext_Memcheck *impl;

CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only set HOST memory for this backend");

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCallBackend(CeedQFunctionContextGetContextSize(ctx, &ctx_size));

CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only set HOST memory for this backend");

// Clear previous owned data buffers
if (impl->data_allocated) {
memset(impl->data_allocated, -42, ctx_size);
VALGRIND_DISCARD(impl->allocated_block_id);
}
CeedCallBackend(CeedFree(&impl->data_allocated));
if (impl->data_owned) {
memset(impl->data_owned, -42, ctx_size);
VALGRIND_DISCARD(impl->owned_block_id);
}
CeedCallBackend(CeedFree(&impl->data_owned));

// Clear borrowed block id, if present
if (impl->data_borrowed) VALGRIND_DISCARD(impl->borrowed_block_id);

// Set internal pointers to external buffers
switch (copy_mode) {
case CEED_COPY_VALUES:
CeedCallBackend(CeedMallocArray(1, ctx_size, &impl->data_owned));
impl->data_owned = NULL;
impl->data_borrowed = NULL;
impl->data = impl->data_owned;
memcpy(impl->data, data, ctx_size);
break;
case CEED_OWN_POINTER:
impl->data_owned = data;
impl->data_borrowed = NULL;
impl->data = data;
impl->data_owned = data;
impl->data_borrowed = NULL;
impl->owned_block_id = VALGRIND_CREATE_BLOCK(impl->data_owned, ctx_size, "Owned external data buffer");
break;
case CEED_USE_POINTER:
impl->data_borrowed = data;
impl->data = data;
impl->data_owned = NULL;
impl->data_borrowed = data;
impl->owned_block_id = VALGRIND_CREATE_BLOCK(impl->data_borrowed, ctx_size, "Borrowed external data buffer");
}
// Copy data to check ctx_size bounds

// Create internal data buffer
CeedCallBackend(CeedMallocArray(1, ctx_size, &impl->data_allocated));
memcpy(impl->data_allocated, impl->data, ctx_size);
impl->data = impl->data_allocated;
VALGRIND_DISCARD(impl->mem_block_id);
impl->mem_block_id = VALGRIND_CREATE_BLOCK(impl->data, ctx_size, "'QFunction backend context data copy'");
impl->allocated_block_id = VALGRIND_CREATE_BLOCK(impl->data_allocated, ctx_size, "'Allocated internal context data buffer");
memcpy(impl->data_allocated, data, ctx_size);
return CEED_ERROR_SUCCESS;
}

//------------------------------------------------------------------------------
// Sync data
//------------------------------------------------------------------------------
static int CeedQFunctionContextSyncData_Memcheck(CeedQFunctionContext ctx, CeedMemType mem_type) {
size_t ctx_size;
CeedQFunctionContext_Memcheck *impl;

CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only set HOST memory for this backend");

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCallBackend(CeedQFunctionContextGetContextSize(ctx, &ctx_size));

// Copy internal buffer back to owned or borrowed data buffer
if (impl->data_owned) {
memcpy(impl->data_owned, impl->data_allocated, ctx_size);
}
if (impl->data_borrowed) {
memcpy(impl->data_borrowed, impl->data_allocated, ctx_size);
}
return CEED_ERROR_SUCCESS;
}

//------------------------------------------------------------------------------
// QFunctionContext Take Data
//------------------------------------------------------------------------------
static int CeedQFunctionContextTakeData_Memcheck(CeedQFunctionContext ctx, CeedMemType mem_type, void *data) {
size_t ctx_size;
CeedQFunctionContext_Memcheck *impl;

CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only provide HOST memory for this backend");

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCallBackend(CeedQFunctionContextGetContextSize(ctx, &ctx_size));

CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only provide HOST memory for this backend");
// Synchronize memory
CeedCallBackend(CeedQFunctionContextSyncData_Memcheck(ctx, CEED_MEM_HOST));

// Return borrowed buffer
*(void **)data = impl->data_borrowed;
impl->data_borrowed = NULL;
impl->data = NULL;
VALGRIND_DISCARD(impl->mem_block_id);
VALGRIND_DISCARD(impl->borrowed_block_id);

// De-allocate internal memory
if (impl->data_allocated) {
memset(impl->data_allocated, -42, ctx_size);
VALGRIND_DISCARD(impl->allocated_block_id);
}
CeedCallBackend(CeedFree(&impl->data_allocated));
return CEED_ERROR_SUCCESS;
}
Expand All @@ -97,13 +143,19 @@ static int CeedQFunctionContextTakeData_Memcheck(CeedQFunctionContext ctx, CeedM
// QFunctionContext Get Data
//------------------------------------------------------------------------------
static int CeedQFunctionContextGetData_Memcheck(CeedQFunctionContext ctx, CeedMemType mem_type, void *data) {
size_t ctx_size;
CeedQFunctionContext_Memcheck *impl;

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only set HOST memory for this backend");

CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only provide HOST memory for this backend");
CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCallBackend(CeedQFunctionContextGetContextSize(ctx, &ctx_size));

*(void **)data = impl->data;
// Create and return writable buffer
CeedCallBackend(CeedMallocArray(1, ctx_size, &impl->data_writable_copy));
impl->writable_block_id = VALGRIND_CREATE_BLOCK(impl->data_writable_copy, ctx_size, "Allocated writeable data buffer copy");
memcpy(impl->data_writable_copy, impl->data_allocated, ctx_size);
*(void **)data = impl->data_writable_copy;
return CEED_ERROR_SUCCESS;
}

Expand All @@ -114,13 +166,18 @@ static int CeedQFunctionContextGetDataRead_Memcheck(CeedQFunctionContext ctx, Ce
size_t ctx_size;
CeedQFunctionContext_Memcheck *impl;

CeedCheck(mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND, "Can only set HOST memory for this backend");

CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCallBackend(CeedQFunctionContextGetContextSize(ctx, &ctx_size));
CeedCallBackend(CeedQFunctionContextGetData_Memcheck(ctx, mem_type, data));

// Make copy to verify no write occurred
CeedCallBackend(CeedMallocArray(1, ctx_size, &impl->data_read_only_copy));
memcpy(impl->data_read_only_copy, *(void **)data, ctx_size);
// Create and return read-only buffer
if (!impl->data_read_only_copy) {
CeedCallBackend(CeedMallocArray(1, ctx_size, &impl->data_read_only_copy));
impl->writable_block_id = VALGRIND_CREATE_BLOCK(impl->data_read_only_copy, ctx_size, "Allocated read-only data buffer copy");
memcpy(impl->data_read_only_copy, impl->data_allocated, ctx_size);
}
*(void **)data = impl->data_read_only_copy;
return CEED_ERROR_SUCCESS;
}

Expand All @@ -134,46 +191,76 @@ static int CeedQFunctionContextRestoreData_Memcheck(CeedQFunctionContext ctx) {
CeedCallBackend(CeedQFunctionContextGetContextSize(ctx, &ctx_size));
CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));

if (impl->data_borrowed) memcpy(impl->data_borrowed, impl->data, ctx_size);
if (impl->data_owned) memcpy(impl->data_owned, impl->data, ctx_size);
// Copy back to internal buffer and sync
memcpy(impl->data_allocated, impl->data_writable_copy, ctx_size);
CeedCallBackend(CeedQFunctionContextSyncData_Memcheck(ctx, CEED_MEM_HOST));

// Invalidate writable buffer
memset(impl->data_writable_copy, -42, ctx_size);
CeedCallBackend(CeedFree(&impl->data_writable_copy));
VALGRIND_DISCARD(impl->writable_block_id);
return CEED_ERROR_SUCCESS;
}

//------------------------------------------------------------------------------
// QFunctionContext Restore Data Read-Only
//------------------------------------------------------------------------------
static int CeedQFunctionContextRestoreDataRead_Memcheck(CeedQFunctionContext ctx) {
Ceed ceed;
size_t ctx_size;
CeedQFunctionContext_Memcheck *impl;

CeedCallBackend(CeedQFunctionContextGetCeed(ctx, &ceed));
CeedCallBackend(CeedQFunctionContextGetContextSize(ctx, &ctx_size));
CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));

CeedCheck(!memcmp(impl->data, impl->data_read_only_copy, ctx_size), CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND,
"Context data changed while accessed in read-only mode");
// Verify no changes made during read-only access
bool is_changed = memcmp(impl->data_allocated, impl->data_read_only_copy, ctx_size);

CeedCheck(!is_changed, ceed, CEED_ERROR_BACKEND, "Context data changed while accessed in read-only mode");

// Invalidate read-only buffer
memset(impl->data_read_only_copy, -42, ctx_size);
CeedCallBackend(CeedFree(&impl->data_read_only_copy));
VALGRIND_DISCARD(impl->read_only_block_id);
return CEED_ERROR_SUCCESS;
}

//------------------------------------------------------------------------------
// QFunctionContext destroy user data
//------------------------------------------------------------------------------
static int CeedQFunctionContextDataDestroy_Memcheck(CeedQFunctionContext ctx) {
Ceed ceed;
CeedMemType data_destroy_mem_type;
CeedQFunctionContextDataDestroyUser data_destroy_function;
CeedQFunctionContext_Memcheck *impl;

CeedCallBackend(CeedQFunctionContextGetCeed(ctx, &ceed));
CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCallBackend(CeedQFunctionContextGetDataDestroy(ctx, &data_destroy_mem_type, &data_destroy_function));

CeedCheck(data_destroy_mem_type == CEED_MEM_HOST, CeedQFunctionContextReturnCeed(ctx), CEED_ERROR_BACKEND,
"Can only destroy HOST memory for this backend");
CeedCallBackend(CeedQFunctionContextGetDataDestroy(ctx, &data_destroy_mem_type, &data_destroy_function));
CeedCheck(data_destroy_mem_type == CEED_MEM_HOST, ceed, CEED_ERROR_BACKEND, "Can only destroy HOST memory for this backend");

// Run user destroy routine
if (data_destroy_function) {
CeedCallBackend(data_destroy_function(impl->data_borrowed ? impl->data_borrowed : impl->data_owned));
bool is_borrowed = !!impl->data_borrowed;

CeedCallBackend(data_destroy_function(is_borrowed ? impl->data_borrowed : impl->data_owned));
if (is_borrowed) VALGRIND_DISCARD(impl->borrowed_block_id);
else VALGRIND_DISCARD(impl->owned_block_id);
}
// Free allocations and discard block ids
if (impl->data_allocated) {
CeedCallBackend(CeedFree(&impl->data_allocated));
VALGRIND_DISCARD(impl->allocated_block_id);
}
if (impl->data_owned) {
CeedCallBackend(CeedFree(&impl->data_owned));
VALGRIND_DISCARD(impl->owned_block_id);
}
if (impl->data_borrowed) {
VALGRIND_DISCARD(impl->borrowed_block_id);
}
CeedCallBackend(CeedFree(&impl->data_allocated));
return CEED_ERROR_SUCCESS;
}

Expand All @@ -183,9 +270,19 @@ static int CeedQFunctionContextDataDestroy_Memcheck(CeedQFunctionContext ctx) {
static int CeedQFunctionContextDestroy_Memcheck(CeedQFunctionContext ctx) {
CeedQFunctionContext_Memcheck *impl;

// Free allocations and discard block ids
CeedCallBackend(CeedQFunctionContextGetBackendData(ctx, &impl));
CeedCallBackend(CeedFree(&impl->data_allocated));
CeedCallBackend(CeedFree(&impl->data_owned));
if (impl->data_allocated) {
CeedCallBackend(CeedFree(&impl->data_allocated));
VALGRIND_DISCARD(impl->allocated_block_id);
}
if (impl->data_owned) {
CeedCallBackend(CeedFree(&impl->data_owned));
VALGRIND_DISCARD(impl->owned_block_id);
}
if (impl->data_borrowed) {
VALGRIND_DISCARD(impl->borrowed_block_id);
}
CeedCallBackend(CeedFree(&impl));
return CEED_ERROR_SUCCESS;
}
Expand Down
6 changes: 3 additions & 3 deletions backends/memcheck/ceed-memcheck-vector.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,13 @@ static int CeedVectorSetArray_Memcheck(CeedVector vec, CeedMemType mem_type, Cee
// Clear previous owned arrays
if (impl->array_allocated) {
for (CeedSize i = 0; i < length; i++) impl->array_allocated[i] = NAN;
VALGRIND_DISCARD(impl->allocated_block_id);
}
CeedCallBackend(CeedFree(&impl->array_allocated));
VALGRIND_DISCARD(impl->allocated_block_id);
if (impl->array_owned) {
for (CeedSize i = 0; i < length; i++) impl->array_owned[i] = NAN;
VALGRIND_DISCARD(impl->owned_block_id);
}
VALGRIND_DISCARD(impl->owned_block_id);
CeedCallBackend(CeedFree(&impl->array_owned));

// Clear borrowed block id, if present
Expand Down Expand Up @@ -139,9 +139,9 @@ static int CeedVectorTakeArray_Memcheck(CeedVector vec, CeedMemType mem_type, Ce
// De-allocate internal memory
if (impl->array_allocated) {
for (CeedSize i = 0; i < length; i++) impl->array_allocated[i] = NAN;
VALGRIND_DISCARD(impl->allocated_block_id);
}
CeedCallBackend(CeedFree(&impl->array_allocated));
VALGRIND_DISCARD(impl->allocated_block_id);
return CEED_ERROR_SUCCESS;
}

Expand Down
13 changes: 11 additions & 2 deletions backends/memcheck/ceed-memcheck.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,21 @@ typedef struct {
} CeedQFunction_Memcheck;

typedef struct {
int mem_block_id;
void *data;
// Internal data buffer
int allocated_block_id;
void *data_allocated;
// Owned external data
int owned_block_id;
void *data_owned;
// Borrowed external data
int borrowed_block_id;
void *data_borrowed;
// Externally viewable read-only data
int read_only_block_id;
void *data_read_only_copy;
// Externally viewable writable data
int writable_block_id;
void *data_writable_copy;
} CeedQFunctionContext_Memcheck;

CEED_INTERN int CeedVectorCreate_Memcheck(CeedSize n, CeedVector vec);
Expand Down

0 comments on commit 91b5d27

Please sign in to comment.