Skip to content

Commit

Permalink
Disable sanitizer checks where known behaviors occur (#299)
Browse files Browse the repository at this point in the history
When running with sanitizers enabled some of the functionality in
decNumber and a few places within ion-c itself, triggers runtime
sanitizer checks. I've disabled specific sanitizer checks on the
functions that exhibit the behaviors in order to quiet the
sanitizers, with the expectation that a sanitizer error should
cause unit test failure. I have looked at each of the instances
and verified that despite the sanitizer being correct, the code
is behaving as expected.

That said, there are certainly ways to implement the code without
changing functionality, that would not exhibit these behaviors.
It would be ideal to revisit each case and eliminate the need for
disabling the sanitizer checks.
  • Loading branch information
nirosys authored Jul 5, 2022
1 parent 5bad60b commit 312503f
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 0 deletions.
7 changes: 7 additions & 0 deletions decNumber/decNumber.c
Original file line number Diff line number Diff line change
Expand Up @@ -3347,6 +3347,9 @@ const char *decNumberClassToString(enum decClass eclass) {
/* All fields are updated as required. This is a utility operation, */
/* so special values are unchanged and no error is possible. */
/* ------------------------------------------------------------------ */
// Disable sanitizer bounds checking, since this function relies on
// occasionally walking beyond the lsu definition.
NOSAN_BOUNDS
decNumber * decNumberCopy(decNumber *dest, const decNumber *src) {

#if DECCHECK
Expand Down Expand Up @@ -3616,6 +3619,10 @@ decNumber * decNumberZero(decNumber *dn) {
/* ------------------------------------------------------------------ */
// If DECCHECK is enabled the string "?" is returned if a number is
// invalid.
// This function disables bounds checking for for the Undefined Behavior
// sanitizer since it relies on occasionally walking past the struct
// defined boundaries for decNumber's lsu.
NOSAN_BOUNDS
static void decToString(const decNumber *dn, char *string, Flag eng) {
Int exp=dn->exponent; // local copy
Int e; // E-part value
Expand Down
6 changes: 6 additions & 0 deletions decNumber/include/decNumber/decNumber.h
Original file line number Diff line number Diff line change
Expand Up @@ -179,4 +179,10 @@
&& (((dn)->bits&DECSPECIAL)==0))
#define decNumberRadix(dn) (10)

# if defined(__clang__) || defined(__GNUC__)
# define NOSAN_BOUNDS __attribute__((no_sanitize("bounds")))
# else
# define NOSAN_BOUNDS
# endif

#endif
3 changes: 3 additions & 0 deletions ionc/ion_int.c
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,9 @@ iERR ion_int_from_decimal(ION_INT *iint, const decQuad *p_value, decContext *con
iRETURN;
}

// Disable sanitizer bounds checking since we know this function requires this behavior when
// indexing beyond lsu struct bounds on larger numbers.
NOSAN_BOUNDS
iERR _ion_int_from_decimal_number(ION_INT *iint, const decNumber *p_value, decContext *context)
{
iENTER;
Expand Down
8 changes: 8 additions & 0 deletions ionc/ion_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,14 @@ GLOBAL int TID_NIBBLE_TO_ION_TYPE[]
#endif
;

#if defined(__clang__) || defined(__GNUC__)
# define NOSAN_SHIFT __attribute__((no_sanitize("shift")))
# define NOSAN_SINT_OVERFLOW __attribute__((no_sanitize("signed-integer-overflow")))
#else
# define NOSAN_SHIFT
# define NOSAN_SINT_OVERFLOW
#endif


#ifdef __cplusplus
}
Expand Down
2 changes: 2 additions & 0 deletions ionc/ion_scanner.c
Original file line number Diff line number Diff line change
Expand Up @@ -1981,6 +1981,8 @@ iERR _ion_scanner_read_lob_closing_braces(ION_SCANNER *scanner)
iRETURN;
}

// Disabled sanitizer shift checks, due to unsigned int shifting.
NOSAN_SHIFT
iERR _ion_scanner_read_as_base64(ION_SCANNER *scanner, BYTE *buf, SIZE len, SIZE *p_bytes_written, BOOL *p_eos_encountered)
{
iENTER;
Expand Down
3 changes: 3 additions & 0 deletions ionc/ion_symbol_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -2155,6 +2155,9 @@ int_fast8_t _ion_symbol_table_compare_fn(void *key1, void *key2, void *context)
return cmp;
}

// Disable the unexpected behavior sanitizer's shift, and signed int overflow checks since
// we know _ion_symbol_table_hash_fn uses these behaviors.
NOSAN_SHIFT NOSAN_SINT_OVERFLOW
int_fast32_t _ion_symbol_table_hash_fn(void *key, void *context)
{
ION_SYMBOL *sym = (ION_SYMBOL *)key;
Expand Down

0 comments on commit 312503f

Please sign in to comment.