Uploaded image for project: 'WiredTiger'
  1. WiredTiger
  2. WT-10234

Don't call functions with side effects inside of WT_ASSERTs

    • 5
    • StorEng - Defined Pipeline

      Summary
      This popped up during the implementation of WT-10043. We have a handful of WT_ASSERTs which are checking the results of functions that with side effects such as allocating memory or assigning values to structs.

      // This sets ref_ikey
      WT_ASSERT(session, __wt_atomic_cas_ptr(&ref->ref_ikey, (WT_IKEY *)oldv, ikey));
      
      // This allocates mem for &key
      WT_ASSERT(session, __wt_row_leaf_key(session, page, rip, &key, false) == 0); 
      
      // This allocated mem for &ctmp
      WT_ASSERT(session, __wt_scr_alloc(session, dsk->mem_size, &ctmp)); 
      
      // This loads data from buf into ctmp
      WT_ASSERT(session, btree->compressor->decompress(btree->compressor, &session->iface, (uint8_t *)buf->data + WT_BLOCK_COMPRESS_SKIP, buf->size - WT_BLOCK_COMPRESS_SKIP, (uint8_t *)ctmp->data + WT_BLOCK_COMPRESS_SKIP, ctmp->memsize - WT_BLOCK_COMPRESS_SKIP, &result_len) == 0); 

      The problem with this is that if asserts are disabled, these functions are not run and can potentially break WiredTiger. In the case of the first example it can lead to a null pointer dereference.

      Luckily this is currently not possible in the code base as all these examples are wrapped inside an #ifdef HAVE_DIAGNOSTIC which guarantees WT_ASSERTS are run, but for cleanliness we should move these functions outside of their WT_ASSERTs and instead only assert on their returned values.

            Assignee:
            backlog-server-storage-engines [DO NOT USE] Backlog - Storage Engines Team
            Reporter:
            andrew.morton@mongodb.com Andrew Morton
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: