Check results of all runtime function calls (#6389)

* Check results of all runtime function calls

This cherry-picks just the changes to callsites internal to Halide (and tests) from #6388. (It doesn't attempt to annotate runtime functions to enforce checking the results.)

* Update write_debug_image.cpp

* Add checks + comment to buffer_copy_aottest

* Add comment to gpu_object_lifetime_aottest

* Update memory_profiler_mandelbrot_aottest.cpp

* Update user_context_insanity_aottest.cpp

* Update process.cpp

src/runtime/HalideBuffer.h

@@ -240,15 +240,19 @@ class Buffer {
                        "Call device_free explicitly if you want to drop dirty device-side data. "
                        "Call copy_to_host explicitly if you want the data copied to the host allocation "
                        "before the device allocation is freed.");
+                int result = 0;
                 if (dev_ref_count && dev_ref_count->ownership == BufferDeviceOwnership::WrappedNative) {
-                    buf.device_interface->detach_native(nullptr, &buf);
+                    result = buf.device_interface->detach_native(nullptr, &buf);
                 } else if (dev_ref_count && dev_ref_count->ownership == BufferDeviceOwnership::AllocatedDeviceAndHost) {
-                    buf.device_interface->device_and_host_free(nullptr, &buf);
+                    result = buf.device_interface->device_and_host_free(nullptr, &buf);
                 } else if (dev_ref_count && dev_ref_count->ownership == BufferDeviceOwnership::Cropped) {
-                    buf.device_interface->device_release_crop(nullptr, &buf);
+                    result = buf.device_interface->device_release_crop(nullptr, &buf);
                 } else if (dev_ref_count == nullptr || dev_ref_count->ownership == BufferDeviceOwnership::Allocated) {
-                    buf.device_interface->device_free(nullptr, &buf);
+                    result = buf.device_interface->device_free(nullptr, &buf);
                 }
+                // No reasonable way to return the error, but we can at least assert-fail in debug builds.
+                assert((result == 0) && "device_interface call returned a nonzero result in Buffer::decref()");
+                (void)result;
             }
             if (dev_ref_count) {
                 if (dev_ref_count->ownership == BufferDeviceOwnership::Cropped) {

src/runtime/cache.cpp

@@ -179,7 +179,10 @@ WEAK bool CacheEntry::init(const uint8_t *cache_key, size_t cache_key_size,
 
 WEAK void CacheEntry::destroy() {
     for (uint32_t i = 0; i < tuple_count; i++) {
-        halide_device_free(nullptr, &buf[i]);
+        if (halide_device_free(nullptr, &buf[i]) != 0) {
+            // Just log a debug message, there's not much we can do in response here
+            debug(nullptr) << "CacheEntry::destroy: halide_device_free failed\n";
+        }
         halide_free(nullptr, get_pointer_to_header(buf[i].host));
     }
     halide_free(nullptr, metadata_storage);

src/runtime/device_interface.cpp

@@ -48,7 +48,7 @@ WEAK int copy_to_host_already_locked(void *user_context, struct halide_buffer_t
         return halide_error_code_copy_to_host_failed;
     }
     buf->set_device_dirty(false);
-    halide_msan_annotate_buffer_is_initialized(user_context, buf);
+    (void)halide_msan_annotate_buffer_is_initialized(user_context, buf);  // ignore errors
 
     return result;
 }
@@ -264,7 +264,7 @@ WEAK int halide_device_free(void *user_context, struct halide_buffer_t *buf) {
  * error. Used when freeing as a destructor on an error. */
 WEAK void halide_device_free_as_destructor(void *user_context, void *obj) {
     struct halide_buffer_t *buf = (struct halide_buffer_t *)obj;
-    halide_device_free(user_context, buf);
+    (void)halide_device_free(user_context, buf);  // ignore errors
 }
 
 /** Allocate host and device memory to back a halide_buffer_t. Ideally this

src/runtime/halide_buffer_t.cpp

@@ -156,7 +156,10 @@ halide_buffer_t *_halide_buffer_crop(void *user_context,
     dst->device_interface = nullptr;
     dst->device = 0;
     if (src->device_interface) {
-        src->device_interface->device_crop(user_context, src, dst);
+        if (src->device_interface->device_crop(user_context, src, dst) != 0) {
+            // No way to report it: just ignore it
+            // debug(user_context) << "_halide_buffer_crop: device_crop failed\n";
+        }
     }
     return dst;
 }
@@ -168,6 +171,7 @@ halide_buffer_t *_halide_buffer_crop(void *user_context,
 HALIDE_BUFFER_HELPER_ATTRS
 int _halide_buffer_retire_crop_after_extern_stage(void *user_context,
                                                   void *obj) {
+    int result;
     halide_buffer_t **buffers = (halide_buffer_t **)obj;
     halide_buffer_t *crop = buffers[0];
     halide_buffer_t *parent = buffers[1];
@@ -178,15 +182,24 @@ int _halide_buffer_retire_crop_after_extern_stage(void *user_context,
             // stage. It only represents the cropped region, so we
             // can't just give it to the parent.
             if (crop->device_dirty()) {
-                crop->device_interface->copy_to_host(user_context, crop);
+                result = crop->device_interface->copy_to_host(user_context, crop);
+                if (result != 0) {
+                    return result;
+                }
+            }
+            result = crop->device_interface->device_free(user_context, crop);
+            if (result != 0) {
+                return result;
             }
-            crop->device_interface->device_free(user_context, crop);
         } else {
             // We are a crop of an existing device allocation.
             if (crop->device_dirty()) {
                 parent->set_device_dirty();
             }
-            crop->device_interface->device_release_crop(user_context, crop);
+            result = crop->device_interface->device_release_crop(user_context, crop);
+            if (result != 0) {
+                return result;
+            }
         }
     }
     if (crop->host_dirty()) {

src/runtime/matlab.cpp

@@ -507,12 +507,18 @@ WEAK int halide_matlab_call_pipeline(void *user_context,
 
         if (arg_metadata->kind == halide_argument_kind_output_buffer) {
             halide_buffer_t *buf = (halide_buffer_t *)args[i];
-            halide_copy_to_host(user_context, buf);
+            if ((result = halide_copy_to_host(user_context, buf)) != 0) {
+                error(user_context) << "halide_matlab_call_pipeline: halide_copy_to_host failed.\n";
+                return result;
+            }
         }
         if (arg_metadata->kind == halide_argument_kind_input_buffer ||
             arg_metadata->kind == halide_argument_kind_output_buffer) {
             halide_buffer_t *buf = (halide_buffer_t *)args[i];
-            halide_device_free(user_context, buf);
+            if ((result = halide_device_free(user_context, buf)) != 0) {
+                error(user_context) << "halide_matlab_call_pipeline: halide_device_free failed.\n";
+                return result;
+            }
         }
     }
 

src/runtime/msan.cpp

@@ -34,7 +34,7 @@ WEAK void annotate_helper(void *uc, const device_copy &c, int d, int64_t off) {
 
     if (d == -1) {
         const void *from = (void *)(c.src + off);
-        halide_msan_annotate_memory_is_initialized(uc, from, c.chunk_size);
+        (void)halide_msan_annotate_memory_is_initialized(uc, from, c.chunk_size);  // ignore errors
     } else {
         for (uint64_t i = 0; i < c.extent[d]; i++) {
             annotate_helper(uc, c, d - 1, off);
@@ -50,7 +50,7 @@ WEAK void check_helper(void *uc, const device_copy &c, int d, int64_t off, const
 
     if (d == -1) {
         const void *from = (void *)(c.src + off);
-        halide_msan_check_memory_is_initialized(uc, from, c.chunk_size, buf_name);
+        (void)halide_msan_check_memory_is_initialized(uc, from, c.chunk_size, buf_name);  // ignore errors
     } else {
         for (uint64_t i = 0; i < c.extent[d]; i++) {
             check_helper(uc, c, d - 1, off, buf_name);
@@ -97,8 +97,8 @@ WEAK int halide_msan_check_buffer_is_initialized(void *user_context, halide_buff
         return 0;
     }
 
-    halide_msan_check_memory_is_initialized(user_context, (void *)b, sizeof(*b), buf_name);
-    halide_msan_check_memory_is_initialized(user_context, (void *)b->dim, b->dimensions * sizeof(b->dim[0]), buf_name);
+    (void)halide_msan_check_memory_is_initialized(user_context, (void *)b, sizeof(*b), buf_name);                              // ignore errors
+    (void)halide_msan_check_memory_is_initialized(user_context, (void *)b->dim, b->dimensions * sizeof(b->dim[0]), buf_name);  // ignore errors
 
     Halide::Runtime::Internal::device_copy c = Halide::Runtime::Internal::make_host_to_device_copy(b);
     if (c.chunk_size == 0) {

src/runtime/tracing.cpp

@@ -385,7 +385,7 @@ WEAK int halide_shutdown_trace() {
 
 namespace {
 WEAK __attribute__((destructor)) void halide_trace_cleanup() {
-    halide_shutdown_trace();
+    (void)halide_shutdown_trace();  // ignore errors
 }
 }  // namespace
 }

src/runtime/write_debug_image.cpp

@@ -142,7 +142,10 @@ WEAK extern "C" int32_t halide_debug_to_file(void *user_context, const char *fil
         return -1;
     }
 
-    halide_copy_to_host(user_context, buf);
+    int result = halide_copy_to_host(user_context, buf);
+    if (result != 0) {
+        return result;
+    }
 
     ScopedFile f(filename, "wb");
     if (!f.open()) {

test/generator/buffer_copy_aottest.cpp

@@ -10,13 +10,22 @@ using namespace Halide::Runtime;
 int main(int argc, char **argv) {
     // Test simple host to host buffer copy.
 
+    // Note that  only way halide_buffer_copy() could possibly fail is if
+    // the allocation of the images failed (which would simply crash us),
+    // so checking the return code is arguably redundant in this context
+    // (but testing to verify that is part of a good test).
+
     {
         Buffer<int> input(128, 128);
         input.fill([&](int x, int y) { return x + 10 * y; });
         Buffer<int> out(64, 64);
         out.set_min(32, 32);
 
-        halide_buffer_copy(nullptr, input, nullptr, out);
+        int result = halide_buffer_copy(nullptr, input, nullptr, out);
+        if (result != 0) {
+            printf("halide_buffer_copy() failed\n");
+            exit(-1);
+        }
 
         Buffer<int> in_crop = input.cropped(0, 32, 64).cropped(1, 32, 64);
         out.for_each_value([&](int a, int b) {
@@ -44,7 +53,11 @@ int main(int argc, char **argv) {
         out.set_min(32, 32);
         Buffer<int> in_crop = input.cropped(0, 32, 64).cropped(1, 32, 64);
 
-        halide_buffer_copy(nullptr, in_crop, dev, out);
+        int result = halide_buffer_copy(nullptr, in_crop, dev, out);
+        if (result != 0) {
+            printf("halide_buffer_copy() failed\n");
+            exit(-1);
+        }
 
         out.copy_to_host();
 
@@ -72,7 +85,11 @@ int main(int argc, char **argv) {
         in_crop.set_host_dirty(false);
         in_crop.set_device_dirty();
 
-        halide_buffer_copy(nullptr, in_crop, nullptr, out);
+        int result = halide_buffer_copy(nullptr, in_crop, nullptr, out);
+        if (result != 0) {
+            printf("halide_buffer_copy() failed\n");
+            exit(-1);
+        }
 
         in_crop.copy_to_host();
 

test/generator/gpu_object_lifetime_aottest.cpp

@@ -162,7 +162,12 @@ int main(int argc, char **argv) {
             {
                 Buffer<int> copy(raw_buf);
             }
-            halide_device_free(nullptr, &raw_buf);
+            // Note that a nonzero result should be impossible here (in theory)
+            int result = halide_device_free(nullptr, &raw_buf);
+            if (result != 0) {
+                printf("Error! halide_device_free() returned: %d\n", result);
+                return -1;
+            }
         }
 
         // Test coverage for Halide::Runtime::Buffer construction from halide_buffer_t, taking ownership

test/generator/memory_profiler_mandelbrot_aottest.cpp

@@ -82,7 +82,11 @@ int main(int argc, char **argv) {
     printf("argmin expected value\n  stack peak: %d\n", argmin_stack_peak);
     printf("\n");
 
-    halide_do_par_for(nullptr, launcher_task, 0, num_launcher_tasks, nullptr);
+    // Note that launcher_task() always returns zero, thus halide_do_par_for()
+    // should always return zero, but since this is a test, let's verify that.
+    int result = halide_do_par_for(nullptr, launcher_task, 0, num_launcher_tasks, nullptr);
+    assert(result == 0);
+    (void)result;
 
     halide_profiler_state *state = halide_profiler_get_state();
     assert(state != nullptr);

test/generator/user_context_insanity_aottest.cpp

@@ -39,10 +39,15 @@ int main(int argc, char **argv) {
 
     // Hijack halide's runtime to run a bunch of instances of this function
     // in parallel.
-    halide_do_par_for(nullptr, launcher_task, 0, num_launcher_tasks, nullptr);
+    // Note that launcher_task() always returns zero, thus halide_do_par_for()
+    // should always return zero, but since this is a test, let's verify that.
+    int result = halide_do_par_for(nullptr, launcher_task, 0, num_launcher_tasks, nullptr);
+    assert(result == 0);
+    (void)result;
 
-    for (int i = 0; i < num_launcher_tasks; ++i)
+    for (int i = 0; i < num_launcher_tasks; ++i) {
         assert(got_context[i] == true);
+    }
 
     printf("Success!\n");
     return 0;

tools/RunGenMain.cpp

@@ -520,7 +520,10 @@ int main(int argc, char **argv) {
 
     // This is a single-purpose binary to benchmark this filter, so we
     // shouldn't be eagerly returning device memory.
-    halide_reuse_device_allocations(nullptr, true);
+    int result = halide_reuse_device_allocations(nullptr, true);
+    if (result != 0) {
+        std::cerr << "halide_reuse_device_allocations() returned an error: " << result << "\n";
+    }
 
     if (benchmark) {
         if (benchmarks_flag_value.empty()) {