Skip to content

Commit

Permalink
Fail on alignment issues with ubsan (#616)
Browse files Browse the repository at this point in the history
Signed-off-by: Tristan Partin <[email protected]>
  • Loading branch information
tristan957 authored Feb 16, 2023
1 parent 7c1bc9d commit 07c4368
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 61 deletions.
3 changes: 3 additions & 0 deletions lib/cn/omf.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ struct wbt_lfe_omf {
uint16_t lfe_kmd;
} HSE_PACKED;

/* Size of an LE32 */
#define WBT_LFE_INLINE_KMD_OFF_SIZE 4

OMF_SETGET(struct wbt_lfe_omf, lfe_koff, 16)
OMF_SETGET(struct wbt_lfe_omf, lfe_kmd, 16)

Expand Down
8 changes: 7 additions & 1 deletion lib/cn/wbt_builder.c
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,15 @@ wbt_leaf_publish(struct wbb *wbb)
assert((void *)entry < sfxp);

if (key_extra) {
uint32_t kmd_off_omf;

static_assert(
sizeof(kmd_off_omf) == WBT_LFE_INLINE_KMD_OFF_SIZE, "An LE32 is a 32-bit integer");

/* kmdoff is too large for u16 */
omf_set_lfe_kmd(entry, UINT16_MAX);
*(uint32_t *)sfxp = cpu_to_omf32(kin->kmd_off);
kmd_off_omf = cpu_to_omf32(kin->kmd_off);
memcpy(sfxp, &kmd_off_omf, WBT_LFE_INLINE_KMD_OFF_SIZE);
} else {
omf_set_lfe_kmd(entry, (uint16_t)(kin->kmd_off));
}
Expand Down
5 changes: 2 additions & 3 deletions lib/cn/wbt_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,8 @@ wbt_lfe_kmd(const struct wbt_node_hdr_omf *node, const struct wbt_lfe_omf *lfe)

/* Offset stored in front of key when its too large for 16-bits */
if (kmd_off == UINT16_MAX) {
const uint32_t *p = (void *)node + omf_lfe_koff(lfe);

kmd_off = omf32_to_cpu(*p);
memcpy(&kmd_off, (void *)node + omf_lfe_koff(lfe), WBT_LFE_INLINE_KMD_OFF_SIZE);
kmd_off = omf32_to_cpu(kmd_off);
}

return omf_wbn_kmd(node) + kmd_off;
Expand Down
19 changes: 8 additions & 11 deletions meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,16 @@ relative_dir = run_command(
# Strip relative path prefixes from the code if possible, otherwise hide them.
# The following snippet is inspired by the SwayWM Project under the MIT license.
add_project_arguments(
cc.first_supported_argument([
cc.first_supported_argument(
'-fmacro-prefix-map=@0@='.format(relative_dir),
'-DHSE_REL_SRC_DIR="@0@"'.format(relative_dir),
]),
'-DHSE_REL_SRC_DIR="@0@"'.format(relative_dir)
),
language: 'c'
)

if get_option('b_sanitize').contains('undefined')
add_project_arguments(
cc.get_supported_arguments([
'-fno-sanitize-recover=undefined',
'-fsanitize-recover=alignment',
]),
cc.get_supported_arguments('-fno-sanitize-recover=undefined'),
language: 'c'
)
endif
Expand All @@ -92,10 +89,10 @@ if get_option('debug')

if get_option('buildtype') == 'debug'
add_project_arguments(
cc.get_supported_arguments([
cc.get_supported_arguments(
'-DDEBUG_RCU',
'-fstack-protector-all',
]),
'-fstack-protector-all'
),
language: 'c'
)
endif
Expand Down Expand Up @@ -152,7 +149,7 @@ add_project_arguments(
'-Wno-unused-parameter',
'-DLOG_DEFAULT=@0@'.format(log_pri_default),
'-DLEVEL1_DCACHE_LINESIZE=@0@'.format(level1_dcache_linesize),
'-DURCU_INLINE_SMALL_FUNCTIONS',
'-DURCU_INLINE_SMALL_FUNCTIONS'
),
language: 'c'
)
Expand Down
8 changes: 0 additions & 8 deletions suppressions/hse.undefined.supp

This file was deleted.

1 change: 0 additions & 1 deletion tests/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ if get_option('b_sanitize') != 'none'
additional_suite_exclusions += 'hse-python'

if get_option('b_sanitize').contains('undefined')
run_env.append('UBSAN_OPTIONS', 'suppressions=@0@/suppressions/hse.undefined.supp'.format(meson.project_source_root()))
run_env.append('UBSAN_OPTIONS', 'print_stacktrace=1')
endif
endif
Expand Down
4 changes: 2 additions & 2 deletions tests/support/include/hse/test/support/random_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* Write pseudo-random data to a buffer, based on a specified seed
*/
void
randomize_buffer(void *buf, size_t len, unsigned int seed);
randomize_buffer(char *buf, size_t len, unsigned int seed);

/*
* validate_random_buffer
Expand All @@ -23,7 +23,7 @@ randomize_buffer(void *buf, size_t len, unsigned int seed);
* the same pseudo-random data, for an easy way to validate a buffer
*/
int
validate_random_buffer(void *buf, size_t len, unsigned int seed);
validate_random_buffer(char *buf, size_t len, unsigned int seed);

/* generate_random_u32
*
Expand Down
59 changes: 26 additions & 33 deletions tests/support/lib/random_buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,65 +7,58 @@
#include <stdint.h>
#include <string.h>

#include <sys/param.h>
#include <sys/types.h>

#include <hse/util/xrand.h>

#include <hse/test/support/random_buffer.h>

void
randomize_buffer(void *buf, size_t len, unsigned int seed)
randomize_buffer(char *buf, size_t len, unsigned int seed)
{
unsigned int *tmp = (unsigned int *)buf;
uint last;
long int remain = len;
int i;
size_t remain;
struct xrand xr;
uint64_t rand_val;

if (len == 0)
return;

xrand_init(&xr, seed);
for (i = 0; remain > 0; i++, remain -= sizeof(*tmp)) {
if (remain > sizeof(*tmp)) { /* likely */
tmp[i] = xrand64(&xr);
} else { /* unlikely */
last = xrand64(&xr);
memcpy(&tmp[i], &last, remain);
}
for (remain = len; remain >= sizeof(rand_val); remain -= sizeof(rand_val)) {
rand_val = xrand64(&xr);
memcpy(buf + len - remain, &rand_val, sizeof(rand_val));
}

if (remain != 0) {
rand_val = xrand64(&xr);
memcpy(buf + len - remain, &rand_val, remain);
}
}

int
validate_random_buffer(void *buf, size_t len, unsigned int seed)
validate_random_buffer(char *buf, size_t len, unsigned int seed)
{
unsigned int *tmp = (unsigned int *)buf;
unsigned int val;
char *expect = (char *)&val;
char *found;
long int remain = len;
int i;
size_t remain;
struct xrand xr;
uint64_t rand_val;

if (len == 0)
return -1; /* success... */

xrand_init(&xr, seed);
for (i = 0; remain > 0; i++, remain -= sizeof(*tmp)) {
val = xrand64(&xr);
if ((remain >= sizeof(*tmp)) && (val != tmp[i])) { /* Likely */
return ((int)(len - remain));
} else if (remain < sizeof(*tmp)) { /* Unlikely */
found = (char *)&tmp[i];
if (memcmp(expect, found, remain)) {
/*
* [HSE_REVISIT]
* Miscompare offset might be off here
*/
return ((int)(len - remain));
}
}
for (remain = len; remain > sizeof(rand_val); remain -= sizeof(rand_val)) {
rand_val = xrand64(&xr);
if (memcmp(&rand_val, buf + len - remain, sizeof(rand_val)) != 0)
return (int)(len - remain);
}

if (remain != 0) {
rand_val = xrand64(&xr);
if (memcmp(&rand_val, buf + len - remain, remain) != 0)
return (int)(len - remain);
}

/* -1 is success, because 0..n are valid offsets for an error */
return -1;
}
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/cn/cn_api_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ MTF_DEFINE_UTEST_PRE(cn_api, basic, pre)
struct kvs_cparams cp = { 0 };
struct mpool *ds = (void *)-1;
struct cndb *cndb = (void *)-1;
uint64_t dummy_ikvdb[32] = { 0 };

enum key_lookup_res res;
struct kvs_rparams *rp_out;
Expand All @@ -61,12 +60,13 @@ MTF_DEFINE_UTEST_PRE(cn_api, basic, pre)
kt.kt_data = "123";
kt.kt_len = 3;

kk.kk_parent = (void *)&dummy_ikvdb;
kk.kk_cparams = &cp;

mapi_inject(mapi_idx_ikvdb_get_csched, 0);
mapi_inject(mapi_idx_mpool_props_get, 0);
mapi_inject(mapi_idx_mpool_mclass_props_get, ENOENT);
mapi_inject_ptr(mapi_idx_ikvdb_kvdb_handle, NULL);
mapi_inject_ptr(mapi_idx_kvdb_kvs_parent, NULL);

err = cn_kvdb_create(4, 4, &cn_kvdb);
ASSERT_EQ(0, err);
Expand Down

0 comments on commit 07c4368

Please sign in to comment.