-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Memory relocation #36
base: main
Are you sure you want to change the base?
Changes from all commits
3cbb748
c32247d
93ff635
871638a
768049f
683eca8
6982d8b
330c509
74d742c
2c5b7d6
d62a338
1ae32e5
44f91eb
9b83d6d
0bdf1e1
d193b3f
1f43986
49805ee
4353f64
1b22061
2f11a7e
65cb901
8093541
d026137
3ab79b3
d1b7492
27ad5c5
c3da197
f147768
1726141
e778bb9
31ffde5
8d16b3f
2dc7b1e
b9cbaf5
d3c7165
073b9c2
5ea00bc
b366829
59aa6cd
1641508
4a0fce9
364bb88
fd379ad
34d73ec
e9ebde7
91fc3af
276c7ed
87908c5
5ce16e1
27d58db
5d1cb15
ecc539f
5003cf4
156b6bc
0e06021
39fa31e
fb62c88
9339225
ad17642
d51f564
1215e12
e95a506
2868221
04f4bb3
9a908f0
ddd7327
b6eae99
46080d8
7a3eab0
7a9c388
7733f88
df1ba72
397258a
a6aa156
1914b4d
8011708
7b9d02f
1619478
5004cb5
37be285
3578b67
558c7c0
1758adc
c96fc24
1f60b89
04ba4a9
26fc93c
149d477
f9b910e
3554bb9
73f09d3
5f93b3d
a0b99da
fd931ed
d292af9
ee8d84a
0850b8e
4ee1bc8
337d4e1
4cfda73
0b4ccc3
52eeec2
3002695
ac4f34e
8cbe5a5
0debb94
ec7b4e5
c0e262f
719a954
c25e065
6de2333
be9f432
dc4d509
6ef4928
6ab52c5
50af070
983a436
5bd52ff
68085d2
ef18e4e
3b917ac
351a712
dae2cf1
711d8d3
265df47
14ffeda
cb671d2
7fa51a1
e39c562
42cfc8e
ce5495c
e567907
8311f66
f56cec6
f717da3
2e7826d
8d567ce
3c753b8
7be35cc
2f7d610
1c94c58
968dfbb
85c424a
8cdb836
d756c34
8bbf39a
2674e1b
f48ef0d
0b4a017
8f98213
fbe0f9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,35 @@ | ||
#include "cairo_runner.h" | ||
#include "collectc/cc_common.h" | ||
#include "collectc/cc_hashtable.h" | ||
#include "memory.h" | ||
#include "program.h" | ||
#include "relocatable.h" | ||
#include "vm.h" | ||
#include <assert.h> | ||
#include <collectc/cc_array.h> | ||
#include <stdio.h> | ||
#include <stdlib.h> | ||
|
||
cairo_runner runner_new(struct program program) { | ||
virtual_machine vm = vm_new(); | ||
cairo_runner runner = {program, vm, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}; | ||
CC_Array *relocated_memory; | ||
cc_array_new(&relocated_memory); | ||
CC_Array *relocation_table; | ||
cc_array_new(&relocation_table); | ||
cairo_runner runner = {program, vm, relocated_memory, relocation_table, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}; | ||
return runner; | ||
} | ||
|
||
void runner_free(cairo_runner *runner) { | ||
memory_free(&runner->vm.memory); | ||
program_free(&runner->program); | ||
cc_array_remove_all_free(runner->relocated_memory); | ||
cc_array_destroy(runner->relocated_memory); | ||
cc_array_remove_all_free(runner->relocation_table); | ||
cc_array_destroy(runner->relocation_table); | ||
} | ||
|
||
// Creates program, execution and builtin segments | ||
// Creates program, execution and builtin segments. | ||
void runner_initialize_segments(cairo_runner *runner) { | ||
// Program Segment | ||
runner->program_base = memory_add_segment(&runner->vm.memory); | ||
|
@@ -77,10 +90,110 @@ void runner_initialize_vm(cairo_runner *runner) { | |
// Apply validation rules to memory | ||
} | ||
|
||
void runner_get_segment_sizes(cairo_runner *runner, CC_Array *segment_sizes) { | ||
int num_segments = runner->vm.memory.num_segments; | ||
unsigned int *zero; | ||
for (int i = 0; i < num_segments; i++) { | ||
zero = malloc(sizeof(unsigned int)); | ||
*zero = 0; | ||
cc_array_add(segment_sizes, zero); | ||
} | ||
|
||
unsigned int *new_size; | ||
CC_HashTableIter memory_iter; | ||
cc_hashtable_iter_init(&memory_iter, runner->vm.memory.data); | ||
TableEntry *entry; | ||
while (cc_hashtable_iter_next(&memory_iter, &entry) != CC_ITER_END) { | ||
relocatable *ptr = entry->key; | ||
unsigned int *segment_size; | ||
assert(cc_array_get_at(segment_sizes, ptr->segment_index, (void **)&segment_size) == CC_OK); | ||
|
||
if (*segment_size < ptr->offset + 1) { | ||
new_size = malloc(sizeof(unsigned int)); | ||
*new_size = ptr->offset + 1; | ||
cc_array_replace_at(segment_sizes, new_size, ptr->segment_index, NULL); | ||
} | ||
} | ||
} | ||
|
||
// Fills relocation table with the first relocated address of each memory segment | ||
// Warning: Can fail if runner_initialize_main_entrypoint is not called before | ||
void runner_fill_relocation_table(cairo_runner *runner, CC_Array *segment_sizes) { | ||
int *first_addr = malloc(sizeof(int)); | ||
*first_addr = 1; | ||
cc_array_add(runner->relocation_table, first_addr); | ||
int *last_segment_size_added; | ||
assert(cc_array_get_at(runner->relocation_table, 0, (void **)&last_segment_size_added) == CC_OK); | ||
|
||
int relocation_table_size = runner->vm.memory.num_segments + 1; | ||
for (int i = 1; i < relocation_table_size; i++) { | ||
int segment_idx = i - 1; | ||
int *segment_size; | ||
assert(cc_array_get_at(segment_sizes, segment_idx, (void **)&segment_size) == CC_OK); | ||
int *address = malloc(sizeof(int)); | ||
*address = *last_segment_size_added + *segment_size; | ||
cc_array_add(runner->relocation_table, address); | ||
assert(cc_array_get_at(runner->relocation_table, i, (void **)&last_segment_size_added) == CC_OK); | ||
} | ||
} | ||
Comment on lines
+119
to
+138
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another batch of suggestions:
|
||
|
||
relocatable runner_initialize(cairo_runner *runner) { | ||
// runner_initialize_builtins | ||
runner_initialize_segments(runner); | ||
relocatable end = runner_initialize_main_entrypoint(runner); | ||
runner_initialize_vm(runner); | ||
return end; | ||
} | ||
|
||
int runner_get_relocated_address(cairo_runner *runner, relocatable *relocatable) { | ||
int *address; | ||
assert(cc_array_get_at(runner->relocation_table, relocatable->segment_index, (void **)&address) == CC_OK); | ||
return *address + relocatable->offset; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Issue: not checking |
||
} | ||
|
||
void runner_get_relocated_value(cairo_runner *runner, maybe_relocatable *value, felt_t out) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we're modifying |
||
if (value->is_felt) { | ||
zero(out); | ||
add(out, value->value.felt, out); | ||
} else { | ||
int out_number = runner_get_relocated_address(runner, &value->value.relocatable); | ||
from(out, out_number); | ||
} | ||
} | ||
|
||
void runner_relocate_memory(cairo_runner *runner) { | ||
assert(cc_array_size(runner->relocated_memory) == 0); | ||
relocated_memory_cell *first_cell = malloc(sizeof(relocated_memory_cell)); | ||
first_cell->is_some = false; | ||
first_cell->memory_value.none = 0; | ||
cc_array_add(runner->relocated_memory, first_cell); | ||
|
||
CC_Array *segment_sizes; | ||
cc_array_new(&segment_sizes); | ||
runner_get_segment_sizes(runner, segment_sizes); | ||
runner_fill_relocation_table(runner, segment_sizes); | ||
|
||
int num_segment_sizes = cc_array_size(segment_sizes); | ||
|
||
for (int i = 0; i < num_segment_sizes; i++) { | ||
int *segment_size; | ||
assert(cc_array_get_at(segment_sizes, i, (void **)&segment_size) == CC_OK); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Issue: not checking |
||
|
||
for (int j = 0; j < *segment_size; j++) { | ||
relocatable ptr = {i, j}; | ||
maybe_relocatable *cell; | ||
if (cc_hashtable_get(runner->vm.memory.data, &ptr, (void **)&cell) == CC_OK) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same for |
||
int relocated_addr = runner_get_relocated_address(runner, &ptr); | ||
relocated_memory_cell *relocated_value = malloc(sizeof(relocated_memory_cell)); | ||
relocated_value->is_some = true; | ||
runner_get_relocated_value(runner, cell, relocated_value->memory_value.value); | ||
cc_array_add_at(runner->relocated_memory, relocated_value, relocated_addr); | ||
} else { | ||
relocated_memory_cell *none = malloc(sizeof(relocated_memory_cell)); | ||
none->is_some = false; | ||
none->memory_value.none = 0; | ||
cc_array_add(runner->relocated_memory, none); | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few issues and suggestions with this function.
uintptr_t
and thenvoid *
. Document it appropriately.cc_array_replace_at
is leaking the old value. With suggestion 1 this automatically gets fixed.cc_array_get_at
already gives you the pointer to the value and doesn't remove it, so if you choose to keep the heap allocated values you can overwrite the value pointer by it instead, also fixing the leaks.NULL
forsegment_size
before dereferencing it. If it's the first inserted value then you hit undefined behavior. You should check forNULL
and store unconditionally in that case, since it's the first value you see for that segment.