areusch commented on a change in pull request #7785:
URL: https://github.com/apache/tvm/pull/7785#discussion_r619396774
##########
File path: src/runtime/crt/memory/stack_allocator.c
##########
@@ -16,17 +16,22 @@
* specific language governing permissions and limitations
* under the License.
*/
-
// LINT_C_FILE
-
#include <tvm/runtime/crt/stack_allocator.h>
+#ifdef TVM_CRT_DEBUG
+#include <tvm/runtime/crt/logging.h>
+#endif
void* StackMemoryManager_Allocate(tvm_workspace_t* tvm_runtime_workspace,
int32_t nbytes) {
uint32_t offset_bytes = (~nbytes + 1) & (TVM_RUNTIME_ALLOC_ALIGNMENT_BYTES -
1);
uint8_t* current_alloc = tvm_runtime_workspace->next_alloc;
uint8_t* next_alloc = tvm_runtime_workspace->next_alloc + nbytes +
offset_bytes;
uint8_t* workspace_end = tvm_runtime_workspace->workspace +
tvm_runtime_workspace->workspace_size;
-
+#ifdef TVM_CRT_DEBUG
Review comment:
for example, perhaps you are integrating this with an application which
operates a radio, but _that_ library accidentally overwrites `next_alloc`. in
this case, it's preferable to have our library fail predictably as soon as it
detects memory corruption. without the check here, we would hand out a corrupt
memory pointer on the following allocate call.
anyway--sorry, my comment here was mainly aimed at addressing @giuseros
question about unit testing. this my rationale for solving the problem of how
to unit test this by making it on-by-default. we should probably address
productionization in a different PR, so to move forward here, we can just find
another way to enable the check in unit tests--that would address my primary
concern here, which is to prove that the generated code uses the APIs correctly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
