re-work unit test mocking (#400)

Instead of trying to use the linker's --wrap, which just led to more problems
when we want to call the real function, we'll add two defines, MOCK_DEFINE() and
MOCK_DECLARE(), that behave differently when building the unit tests, such that
all wrapped functions are picked up from test/mocks.c instead, regardless of
compilation unit.

Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>

9 files changed, 65 insertions, 84 deletions

diff --git a/lib/common.h b/lib/common.h
index 7ac865e..d15017a 100644
--- a/lib/common.h
+++ b/lib/common.h
@@ -55,21 +55,22 @@
 #define ROUND_DOWN(x, a)    ((x) & ~((a)-1))
 #define ROUND_UP(x,a)       ROUND_DOWN((x)+(a)-1, a)
 
-#define UNIT_TEST_SYMBOL(x) \
-    typeof(x) __wrap_##x __attribute__((weak, alias(#x)))
-
 #ifdef UNIT_TEST
 
-#define MOCKED(r, f, ...) \
+#define MOCK_DEFINE(f) \
+    (__real_ ## f)
+
+#define MOCK_DECLARE(r, f, ...) \
     r f(__VA_ARGS__); \
     r __real_ ## f(__VA_ARGS__); \
     r __wrap_ ## f(__VA_ARGS__);
 
 #else /* UNIT_TEST */
 
-#define MOCKED(r, f, ...) \
-    r f(__VA_ARGS__); \
-    r __wrap_ ## f(__VA_ARGS__)
+#define MOCK_DEFINE(f) (f)
+
+#define MOCK_DECLARE(r, f, ...) \
+    r f(__VA_ARGS__);
 
 #endif /* UNIT_TEST */
 
diff --git a/lib/dma.c b/lib/dma.c
index 60e1ae4..c06d666 100644
--- a/lib/dma.c
+++ b/lib/dma.c
@@ -92,8 +92,8 @@ dma_controller_create(vfu_ctx_t *vfu_ctx, int max_regions)
 }
 
 void
-_dma_controller_do_remove_region(dma_controller_t *dma,
-                                 dma_memory_region_t *region)
+MOCK_DEFINE(_dma_controller_do_remove_region)(dma_controller_t *dma,
+                                              dma_memory_region_t *region)
 {
     int err;
 
@@ -113,13 +113,6 @@ _dma_controller_do_remove_region(dma_controller_t *dma,
         }
     }
 }
-UNIT_TEST_SYMBOL(_dma_controller_do_remove_region);
-/*
- * FIXME super ugly, but without this functions within the same compilation
- * unit don't call the wrapped version, making unit testing impossible.
- * Ideally we'd like the UNIT_TEST_SYMBOL macro to solve this.
- */
-#define _dma_controller_do_remove_region __wrap__dma_controller_do_remove_region
 
 /*
  * FIXME no longer used. Also, it doesn't work for addresses that span two
@@ -144,9 +137,10 @@ dma_controller_region_valid(dma_controller_t *dma, dma_addr_t dma_addr,
 
 /* FIXME not thread safe */
 int
-dma_controller_remove_region(dma_controller_t *dma,
-                             dma_addr_t dma_addr, size_t size,
-                             vfu_unmap_dma_cb_t *unmap_dma, void *data)
+MOCK_DEFINE(dma_controller_remove_region)(dma_controller_t *dma,
+                                          dma_addr_t dma_addr, size_t size,
+                                          vfu_unmap_dma_cb_t *unmap_dma,
+                                          void *data)
 {
     int idx;
     dma_memory_region_t *region;
@@ -183,7 +177,6 @@ dma_controller_remove_region(dma_controller_t *dma,
     }
     return -ENOENT;
 }
-UNIT_TEST_SYMBOL(dma_controller_remove_region);
 
 static inline void
 dma_controller_remove_regions(dma_controller_t *dma)
@@ -214,9 +207,9 @@ dma_controller_destroy(dma_controller_t *dma)
 }
 
 int
-dma_controller_add_region(dma_controller_t *dma,
-                          dma_addr_t dma_addr, size_t size,
-                          int fd, off_t offset, uint32_t prot)
+MOCK_DEFINE(dma_controller_add_region)(dma_controller_t *dma,
+                                       dma_addr_t dma_addr, size_t size,
+                                       int fd, off_t offset, uint32_t prot)
 {
     int idx;
     dma_memory_region_t *region;
@@ -320,7 +313,6 @@ dma_controller_add_region(dma_controller_t *dma,
 err:
     return -idx - 1;
 }
-UNIT_TEST_SYMBOL(dma_controller_add_region);
 
 static inline void
 mmap_round(size_t *offset, size_t *size, size_t page_size)
@@ -331,7 +323,8 @@ mmap_round(size_t *offset, size_t *size, size_t page_size)
 }
 
 void *
-dma_map_region(dma_memory_region_t *region, int prot, size_t offset, size_t len)
+MOCK_DEFINE(dma_map_region)(dma_memory_region_t *region, int prot,
+                            size_t offset, size_t len)
 {
     size_t mmap_offset, mmap_size = len;
     char *mmap_base;
@@ -357,7 +350,6 @@ dma_map_region(dma_memory_region_t *region, int prot, size_t offset, size_t len)
 
     return mmap_base + (offset - mmap_offset);
 }
-UNIT_TEST_SYMBOL(dma_map_region);
 
 int
 dma_unmap_region(dma_memory_region_t *region, void *virt_addr, size_t len)
diff --git a/lib/dma.h b/lib/dma.h
index 3f94945..0eb7ac3 100644
--- a/lib/dma.h
+++ b/lib/dma.h
@@ -116,15 +116,16 @@ dma_controller_destroy(dma_controller_t *dma);
  *   where this region would have been mapped to if the call could succeed
  *   (e.g. due to conflict with existing region).
  */
-MOCKED(int, dma_controller_add_region, dma_controller_t *dma,
-       dma_addr_t dma_addr, size_t size, int fd, off_t offset, uint32_t prot);
+MOCK_DECLARE(int, dma_controller_add_region, dma_controller_t *dma,
+             dma_addr_t dma_addr, size_t size, int fd, off_t offset,
+             uint32_t prot);
 
-MOCKED(void, _dma_controller_do_remove_region, dma_controller_t *dma,
-       dma_memory_region_t *region);
+MOCK_DECLARE(void, _dma_controller_do_remove_region, dma_controller_t *dma,
+             dma_memory_region_t *region);
 
-MOCKED(int, dma_controller_remove_region, dma_controller_t *dma,
-       dma_addr_t dma_addr, size_t size, vfu_unmap_dma_cb_t *unmap_dma,
-       void *data);
+MOCK_DECLARE(int, dma_controller_remove_region, dma_controller_t *dma,
+             dma_addr_t dma_addr, size_t size, vfu_unmap_dma_cb_t *unmap_dma,
+             void *data);
 
 // Helper for dma_addr_to_sg() slow path.
 int
@@ -250,8 +251,8 @@ dma_addr_to_sg(const dma_controller_t *dma,
     return cnt;
 }
 
-MOCKED(void *, dma_map_region, dma_memory_region_t *region, int prot,
-       size_t offset, size_t len);
+MOCK_DECLARE(void *, dma_map_region, dma_memory_region_t *region, int prot,
+             size_t offset, size_t len);
 
 int
 dma_unmap_region(dma_memory_region_t *region, void *virt_addr, size_t len);
diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c
index 3140a75..40a23b8 100644
--- a/lib/libvfio-user.c
+++ b/lib/libvfio-user.c
@@ -618,9 +618,9 @@ out:
 }
 
 int
-handle_dirty_pages(vfu_ctx_t *vfu_ctx, uint32_t size,
-                   struct iovec **iovecs, size_t *nr_iovecs,
-                   struct vfio_iommu_type1_dirty_bitmap *dirty_bitmap)
+MOCK_DEFINE(handle_dirty_pages)(vfu_ctx_t *vfu_ctx, uint32_t size,
+                                struct iovec **iovecs, size_t *nr_iovecs,
+                                struct vfio_iommu_type1_dirty_bitmap *dirty_bitmap)
 {
     int ret;
 
@@ -650,8 +650,6 @@ handle_dirty_pages(vfu_ctx_t *vfu_ctx, uint32_t size,
 
     return ret;
 }
-UNIT_TEST_SYMBOL(handle_dirty_pages);
-#define handle_dirty_pages __wrap_handle_dirty_pages
 
 /*
  * FIXME return value is messed up, sometimes we return -1 and set errno while
@@ -693,8 +691,8 @@ validate_header(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size)
  * the number of bytes read.
  */
 int
-get_next_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, int *fds,
-                 size_t *nr_fds)
+MOCK_DEFINE(get_next_command)(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr,
+                              int *fds, size_t *nr_fds)
 {
     int ret;
 
@@ -723,21 +721,17 @@ get_next_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, int *fds,
     }
     return ret;
 }
-UNIT_TEST_SYMBOL(get_next_command);
-#define get_next_command __wrap_get_next_command
 
 bool
-cmd_allowed_when_stopped_and_copying(uint16_t cmd)
+MOCK_DEFINE(cmd_allowed_when_stopped_and_copying)(uint16_t cmd)
 {
     return cmd == VFIO_USER_REGION_READ ||
            cmd == VFIO_USER_REGION_WRITE ||
            cmd == VFIO_USER_DIRTY_PAGES;
 }
-UNIT_TEST_SYMBOL(cmd_allowed_when_stopped_and_copying);
-#define cmd_allowed_when_stopped_and_copying __wrap_cmd_allowed_when_stopped_and_copying
 
 bool
-should_exec_command(vfu_ctx_t *vfu_ctx, uint16_t cmd)
+MOCK_DEFINE(should_exec_command)(vfu_ctx_t *vfu_ctx, uint16_t cmd)
 {
     assert(vfu_ctx != NULL);
 
@@ -755,14 +749,13 @@ should_exec_command(vfu_ctx_t *vfu_ctx, uint16_t cmd)
     }
     return true;
 }
-UNIT_TEST_SYMBOL(should_exec_command);
-#define should_exec_command __wrap_should_exec_command
 
 int
-exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size,
-             int *fds, size_t nr_fds, int **fds_out, size_t *nr_fds_out,
-             struct iovec *_iovecs, struct iovec **iovecs, size_t *nr_iovecs,
-             bool *free_iovec_data)
+MOCK_DEFINE(exec_command)(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr,
+                          size_t size, int *fds, size_t nr_fds, int **fds_out,
+                          size_t *nr_fds_out, struct iovec *_iovecs,
+                          struct iovec **iovecs, size_t *nr_iovecs,
+                          bool *free_iovec_data)
 {
     int ret;
     struct vfio_irq_info *irq_info;
@@ -898,11 +891,9 @@ exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size,
     free(cmd_data);
     return ret;
 }
-UNIT_TEST_SYMBOL(exec_command);
-#define exec_command __wrap_exec_command
 
 int
-process_request(vfu_ctx_t *vfu_ctx)
+MOCK_DEFINE(process_request)(vfu_ctx_t *vfu_ctx)
 {
     struct vfio_user_header hdr = { 0, };
     int ret;
@@ -984,8 +975,6 @@ process_request(vfu_ctx_t *vfu_ctx)
     free(fds_out);
     return ret;
 }
-UNIT_TEST_SYMBOL(process_request);
-#define process_request __wrap_process_request
 
 int
 vfu_realize_ctx(vfu_ctx_t *vfu_ctx)
diff --git a/lib/migration.c b/lib/migration.c
index cc7abcf..93703a0 100644
--- a/lib/migration.c
+++ b/lib/migration.c
@@ -477,13 +477,13 @@ migration_region_access(vfu_ctx_t *vfu_ctx, char *buf, size_t count,
 }
 
 bool
-device_is_stopped_and_copying(struct migration *migr)
+MOCK_DEFINE(device_is_stopped_and_copying)(struct migration *migr)
 {
     return migr != NULL && migr->info.device_state == VFIO_DEVICE_STATE_SAVING;
 }
 
 bool
-device_is_stopped(struct migration *migr)
+MOCK_DEFINE(device_is_stopped)(struct migration *migr)
 {
     return migr != NULL && migr->info.device_state == VFIO_DEVICE_STATE_STOP;
 }
diff --git a/lib/migration.h b/lib/migration.h
index b0ea652..498cae0 100644
--- a/lib/migration.h
+++ b/lib/migration.h
@@ -55,9 +55,9 @@ migration_region_access(vfu_ctx_t *vfu_ctx, char *buf, size_t count,
 bool
 migration_available(vfu_ctx_t *vfu_ctx);
 
-MOCKED(bool, device_is_stopped, struct migration *migr);
+MOCK_DECLARE(bool, device_is_stopped, struct migration *migr);
 
-MOCKED(bool, device_is_stopped_and_copying, struct migration *migration);
+MOCK_DECLARE(bool, device_is_stopped_and_copying, struct migration *migration);
 
 size_t
 migration_get_pgsize(struct migration *migr);
diff --git a/lib/private.h b/lib/private.h
index 9f06304..c463fea 100644
--- a/lib/private.h
+++ b/lib/private.h
@@ -163,23 +163,23 @@ int
 handle_device_set_irqs(vfu_ctx_t *vfu_ctx, uint32_t size,
                        int *fds, size_t nr_fds, struct vfio_irq_set *irq_set);
 
-MOCKED(bool, should_exec_command, vfu_ctx_t *vfu_ctx, uint16_t cmd);
+MOCK_DECLARE(bool, should_exec_command, vfu_ctx_t *vfu_ctx, uint16_t cmd);
 
-MOCKED(bool, cmd_allowed_when_stopped_and_copying, uint16_t cmd);
+MOCK_DECLARE(bool, cmd_allowed_when_stopped_and_copying, uint16_t cmd);
 
-MOCKED(int, get_next_command, vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr,
-       int *fds, size_t *nr_fds);
+MOCK_DECLARE(int, get_next_command, vfu_ctx_t *vfu_ctx,
+             struct vfio_user_header *hdr, int *fds, size_t *nr_fds);
 
-MOCKED(int, exec_command, vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr,
-       size_t size, int *fds, size_t nr_fds, int **fds_out, size_t *nr_fds_out,
-       struct iovec *_iovecs, struct iovec **iovecs, size_t *nr_iovecs,
-       bool *free_iovec_data);
+MOCK_DECLARE(int, exec_command, vfu_ctx_t *vfu_ctx,
+             struct vfio_user_header *hdr, size_t size, int *fds, size_t nr_fds,
+             int **fds_out, size_t *nr_fds_out, struct iovec *_iovecs,
+             struct iovec **iovecs, size_t *nr_iovecs, bool *free_iovec_data);
 
-MOCKED(int, process_request, vfu_ctx_t *vfu_ctx);
+MOCK_DECLARE(int, process_request, vfu_ctx_t *vfu_ctx);
 
-MOCKED(int, handle_dirty_pages, vfu_ctx_t *vfu_ctx, uint32_t size,
-       struct iovec **iovecs, size_t *nr_iovecs,
-       struct vfio_iommu_type1_dirty_bitmap *dirty_bitmap);
+MOCK_DECLARE(int, handle_dirty_pages, vfu_ctx_t *vfu_ctx, uint32_t size,
+             struct iovec **iovecs, size_t *nr_iovecs,
+             struct vfio_iommu_type1_dirty_bitmap *dirty_bitmap);
 
 #endif /* LIB_VFIO_USER_PRIVATE_H */
 
diff --git a/lib/tran_sock.c b/lib/tran_sock.c
index 8919c3f..268b2b4 100644
--- a/lib/tran_sock.c
+++ b/lib/tran_sock.c
@@ -58,10 +58,10 @@ typedef struct {
 } tran_sock_t;
 
 int
-tran_sock_send_iovec(int sock, uint16_t msg_id, bool is_reply,
-                     enum vfio_user_command cmd,
-                     struct iovec *iovecs, size_t nr_iovecs,
-                     int *fds, int count, int err)
+MOCK_DEFINE(tran_sock_send_iovec)(int sock, uint16_t msg_id, bool is_reply,
+                                  enum vfio_user_command cmd,
+                                  struct iovec *iovecs, size_t nr_iovecs,
+                                  int *fds, int count, int err)
 {
     int ret;
     struct vfio_user_header hdr = {.msg_id = msg_id};
@@ -123,8 +123,6 @@ tran_sock_send_iovec(int sock, uint16_t msg_id, bool is_reply,
 
     return 0;
 }
-UNIT_TEST_SYMBOL(tran_sock_send_iovec);
-#define tran_sock_send_iovec __wrap_tran_sock_send_iovec
 
 int
 tran_sock_send(int sock, uint16_t msg_id, bool is_reply,
diff --git a/lib/tran_sock.h b/lib/tran_sock.h
index 754798f..eb7658b 100644
--- a/lib/tran_sock.h
+++ b/lib/tran_sock.h
@@ -60,9 +60,9 @@ tran_parse_version_json(const char *json_str, int *client_max_fdsp,
  * Send a message to the other end.  The iovecs array should leave the first
  * entry empty, as it will be used for the header.
  */
-MOCKED(int, tran_sock_send_iovec, int sock, uint16_t msg_id, bool is_reply,
-       enum vfio_user_command cmd, struct iovec *iovecs, size_t nr_iovecs,
-       int *fds, int count, int err);
+MOCK_DECLARE(int, tran_sock_send_iovec, int sock, uint16_t msg_id,
+             bool is_reply, enum vfio_user_command cmd, struct iovec *iovecs,
+             size_t nr_iovecs, int *fds, int count, int err);
 
 /*
  * Send a message to the other end with the given data.