Page MenuHome
Differential D14798 Diff 52704 source/blender/makesrna/intern/rna_access.c

Changeset View

Standalone View

View Options

source/blender/makesrna/intern/rna_access.c

Context not available.
* \ingroup RNA * \ingroup RNA
*/ */
#include <assert.h>
#include <ctype.h> #include <ctype.h>
#include <stddef.h> #include <stddef.h>
#include <stdlib.h> #include <stdlib.h>
linux_drAuthorUnsubmitted
Done
Inline Actions

THIS IS THE SOURCE OF THE POSSIBLE MEMORY CORRUPTION:

This size can later be used for memcpy()s which can read off the end of memory, or worse, write over the end of memory.

linux_dr: **THIS IS THE SOURCE OF THE POSSIBLE MEMORY CORRUPTION**: This `size` can later be used for…
brechtUnsubmitted
Done
Inline Actions

MEM_mallocN_aligned should be unnecessary, we're not using that for every struct allocation that may contain arbitrary types, and this is no different as far as I can tell.

brecht: `MEM_mallocN_aligned` should be unnecessary, we're not using that for every struct allocation…
linux_drAuthorUnsubmitted
Done
Inline Actions

Since then I learned that all malloc Allocations default to alignof(std::max_align_t) (which is intended to be sizeof(void*)), which means explicitly aligned allocations should only be necessary for weird types used for things like SSE instructions. I will update this in my next revision

linux_dr: Since then I learned that all `malloc` Allocations default to `alignof(std::max_align_t)`…
Context not available.
PropertyRNA *parm; PropertyRNA *parm;
PointerRNA null_ptr = PointerRNA_NULL; PointerRNA null_ptr = PointerRNA_NULL;
void *data; void *data;
int alloc_size = 0, size; void *data_start;
size_t total_size = 0, data_offset = 0, overall_alignment = 1, parm_padding;
struct ParameterInfo current_parm_info, next_parm_info;
parms->arg_count = 0; parms->arg_count = 0;
parms->ret_count = 0; parms->ret_count = 0;
/* allocate data */ /* allocate data */
next_parm_info = rna_parameter_info(func->cont.properties.first);
for (parm = func->cont.properties.first; parm; parm = parm->next) { for (parm = func->cont.properties.first; parm; parm = parm->next) {
alloc_size += rna_parameter_size(parm); current_parm_info = next_parm_info;
next_parm_info = rna_parameter_info(parm->next);
if (current_parm_info.alignment > overall_alignment) {
overall_alignment = current_parm_info.alignment;
}
parm_padding = rna_compute_parameter_padding(
total_size, current_parm_info.size, next_parm_info.alignment);
total_size += current_parm_info.size + parm_padding;
if (parm->flag_parameter & PARM_OUTPUT) { if (parm->flag_parameter & PARM_OUTPUT) {
parms->ret_count++; parms->ret_count++;
Context not available.
} }
} }
parms->data = MEM_callocN(alloc_size, "RNA_parameter_list_create"); /* MEM_mallocN_aligned should not be needed with RNA parameters because
* `overall_alignment <= alignof(MAX_ALIGN_T)` should always be true. */
static_assert(RNA_PARAMETER_INFO__ALIGNMENT__MAXIMUM <= alignof(MAX_ALIGN_T),
"RNA params should not need extended alignment.");
/* This should be enforced by compile time post-condition in `rna_parameter_info()`: */
assert(overall_alignment <= RNA_PARAMETER_INFO__ALIGNMENT__MAXIMUM);
parms->data = MEM_callocN(total_size, "RNA_parameter_list_create");
parms->func = func; parms->func = func;
parms->alloc_size = alloc_size; parms->alloc_size = total_size;
/* set default values */ /* set default values */
data = parms->data; data_start = parms->data;
data = data_start;
data_offset = 0;
next_parm_info = rna_parameter_info(func->cont.properties.first);
for (parm = func->cont.properties.first; parm; parm = parm->next) { for (parm = func->cont.properties.first; parm; parm = parm->next) {
size = rna_parameter_size(parm); current_parm_info = next_parm_info;
next_parm_info = rna_parameter_info(parm->next);
parm_padding = rna_compute_parameter_padding(
data_offset, current_parm_info.size, next_parm_info.alignment);
linux_drAuthorUnsubmitted
Done
Inline Actions

(memory corruption)...and here

linux_dr: (**memory corruption**)...and here
brechtUnsubmitted
Done
Inline Actions

I don't think changing the size here makes sense. It should be modifying only the offset.

brecht: I don't think changing the size here makes sense. It should be modifying only the offset.
/* set length to 0, these need to be set later, see bpy_array.c's py_to_array */ /* set length to 0, these need to be set later, see bpy_array.c's py_to_array */
if (parm->flag & PROP_DYNAMIC) { if (parm->flag & PROP_DYNAMIC) {
linux_drAuthorUnsubmitted
Done
Inline Actions

(memory corruption)...and here

linux_dr: (**memory corruption**)...and here
brechtUnsubmitted
Done
Inline Actions

Same comment.

brecht: Same comment.
Context not available.
&null_ptr, (BoolPropertyRNA *)parm, data); &null_ptr, (BoolPropertyRNA *)parm, data);
} }
else { else {
memcpy(data, &((BoolPropertyRNA *)parm)->defaultvalue, size); memcpy(data, &((BoolPropertyRNA *)parm)->defaultvalue, current_parm_info.size);
} }
break; break;
case PROP_INT: case PROP_INT:
Context not available.
rna_property_int_get_default_array_values(&null_ptr, (IntPropertyRNA *)parm, data); rna_property_int_get_default_array_values(&null_ptr, (IntPropertyRNA *)parm, data);
} }
else { else {
memcpy(data, &((IntPropertyRNA *)parm)->defaultvalue, size); memcpy(data, &((IntPropertyRNA *)parm)->defaultvalue, current_parm_info.size);
} }
break; break;
case PROP_FLOAT: case PROP_FLOAT:
Context not available.
rna_property_float_get_default_array_values(&null_ptr, (FloatPropertyRNA *)parm, data); rna_property_float_get_default_array_values(&null_ptr, (FloatPropertyRNA *)parm, data);
} }
else { else {
memcpy(data, &((FloatPropertyRNA *)parm)->defaultvalue, size); memcpy(data, &((FloatPropertyRNA *)parm)->defaultvalue, current_parm_info.size);
} }
break; break;
case PROP_ENUM: case PROP_ENUM:
memcpy(data, &((EnumPropertyRNA *)parm)->defaultvalue, size); memcpy(data, &((EnumPropertyRNA *)parm)->defaultvalue, current_parm_info.size);
break; break;
case PROP_STRING: { case PROP_STRING: {
const char *defvalue = ((StringPropertyRNA *)parm)->defaultvalue; const char *defvalue = ((StringPropertyRNA *)parm)->defaultvalue;
Context not available.
/* causes bug T29988, possibly this is only correct for thick wrapped /* causes bug T29988, possibly this is only correct for thick wrapped
* need to look further into it - campbell */ * need to look further into it - campbell */
#if 0 #if 0
BLI_strncpy(data, defvalue, size); BLI_strncpy(data, defvalue, current_parm_info.size);
#else #else
memcpy(data, &defvalue, size); memcpy(data, &defvalue, current_parm_info.size);
#endif #endif
} }
break; break;
Context not available.
} }
} }
data = ((char *)data) + rna_parameter_size(parm); data_offset += (current_parm_info.size + parm_padding);
data = ((char *)data_start) + data_offset;
} }
return parms; return parms;
Context not available.
void RNA_parameter_list_free(ParameterList *parms) void RNA_parameter_list_free(ParameterList *parms)
{ {
PropertyRNA *parm; PropertyRNA *parm;
int tot; size_t tot, padding;
struct ParameterInfo current_parm_info, next_parm_info;
parm = parms->func->cont.properties.first; parm = parms->func->cont.properties.first;
next_parm_info = rna_parameter_info(parm);
for (tot = 0; parm; parm = parm->next) { for (tot = 0; parm; parm = parm->next) {
current_parm_info = next_parm_info;
next_parm_info = rna_parameter_info(parm->next);
padding = rna_compute_parameter_padding(tot, current_parm_info.size, next_parm_info.alignment);
if (parm->type == PROP_COLLECTION) { if (parm->type == PROP_COLLECTION) {
BLI_freelistN((ListBase *)((char *)parms->data + tot)); BLI_freelistN((ListBase *)((char *)parms->data + tot));
} }
Context not available.
} }
} }
tot += rna_parameter_size(parm); tot += (current_parm_info.size + padding);
} }
MEM_freeN(parms->data); MEM_freeN(parms->data);
Context not available.
// RNA_pointer_create(NULL, &RNA_Function, parms->func, &iter->funcptr); /* UNUSED */ // RNA_pointer_create(NULL, &RNA_Function, parms->func, &iter->funcptr); /* UNUSED */
iter->parms = parms; iter->parms = parms;
iter->parm = parms->func->cont.properties.first;
iter->valid = iter->parm != NULL;
iter->offset = 0; iter->offset = 0;
iter->size = 0;
iter->padding = 0;
if (iter->valid) { /* We use this fake pre-property only so we can bootstrap the iter->parm in
iter->size = rna_parameter_size(iter->parm); * `RNA_parameter_list_next()` */
iter->data = (((char *)iter->parms->data)); /* +iter->offset, always 0 */ PropertyRNA fake_pre_property;
} fake_pre_property.next = parms->func->cont.properties.first;
iter->parm = &fake_pre_property;
iter->valid = true;
RNA_parameter_list_next(iter);
} }
void RNA_parameter_list_next(ParameterIterator *iter) void RNA_parameter_list_next(ParameterIterator *iter)
{ {
iter->offset += iter->size; iter->offset += iter->size + iter->padding;
iter->parm = iter->parm->next; iter->parm = iter->parm->next;
iter->valid = iter->parm != NULL; iter->valid = iter->parm != NULL;
if (iter->valid) { if (iter->valid) {
iter->size = rna_parameter_size(iter->parm); struct ParameterInfo current_parm_info = rna_parameter_info(iter->parm);
struct ParameterInfo next_parm_info = rna_parameter_info(iter->parm->next);
iter->size = current_parm_info.size;
iter->padding = rna_compute_parameter_padding(
iter->offset, current_parm_info.size, next_parm_info.alignment);
iter->data = (((char *)iter->parms->data) + iter->offset); iter->data = (((char *)iter->parms->data) + iter->offset);
} }
} }
Context not available.