From ed0b19b94bf07056b5e0cfe64d4d05c3ebae801a Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Wed, 17 Sep 2014 18:39:22 +0200 Subject: [PATCH] Make boolean bitmask type 64-bit wide There are protocols out there that have 64-bit wide bit mask fields, so make the internal representation and bitfield decoders 64-bit aware. For this, the ws_ctz() fallback and bits_count_ones() have to be tweaked slightly. Change-Id: I19237b954a69c9e6c55864f281993c1e8731a233 Reviewed-on: https://code.wireshark.org/review/4158 Petri-Dish: Alexis La Goutte Tested-by: Petri Dish Buildbot Reviewed-by: Alexis La Goutte Reviewed-by: Michael Mann --- doc/README.dissector | 4 +-- epan/ftypes/ftype-integer.c | 8 ++--- epan/proto.c | 70 ++++++++++++++++++++++++++----------- epan/proto.h | 2 +- epan/to_str.c | 12 +++---- epan/to_str.h | 6 ++-- ui/gtk/bytes_view.c | 2 +- ui/gtk/bytes_view.h | 2 +- ui/gtk/packet_panes.c | 25 +++++++------ ui/gtk/packet_win.c | 2 +- wsutil/bits_count_ones.h | 12 +++---- wsutil/bits_ctz.h | 28 +++++++++++---- 12 files changed, 110 insertions(+), 63 deletions(-) diff --git a/doc/README.dissector b/doc/README.dissector index 2495c918de..af9a15e781 100644 --- a/doc/README.dissector +++ b/doc/README.dissector @@ -639,7 +639,7 @@ struct header_field_info { enum ftenum type; int display; const void *strings; - guint32 bitmask; + guint64 bitmask; const char *blurb; ..... }; @@ -1774,7 +1774,7 @@ proto_tree_add_bitmask() et al. These functions provide easy to use and convenient dissection of many types of common bitmasks into individual fields. -header is an integer type and must be of type FT_[U]INT{8|16|24|32} and +header is an integer type and must be of type FT_[U]INT{8|16|24|32|64} and represents the entire dissectable width of the bitmask. 'header' and 'ett' are the hf fields and ett field respectively to create an diff --git a/epan/ftypes/ftype-integer.c b/epan/ftypes/ftype-integer.c index 2bf81d6781..e603a9bf4a 100644 --- a/epan/ftypes/ftype-integer.c +++ b/epan/ftypes/ftype-integer.c @@ -1077,7 +1077,7 @@ ftype_register_integers(void) 0, /* wire_size */ boolean_fvalue_new, /* new_value */ NULL, /* free_value */ - uint32_from_unparsed, /* val_from_unparsed */ + uint64_from_unparsed, /* val_from_unparsed */ NULL, /* val_from_string */ boolean_to_repr, /* val_to_string_repr */ boolean_repr_len, /* len_string_repr */ @@ -1090,13 +1090,13 @@ ftype_register_integers(void) NULL, /* set_value_tvbuff */ set_uinteger, /* set_value_uinteger */ NULL, /* set_value_sinteger */ - NULL, /* set_value_integer64 */ + set_integer64, /* set_value_integer64 */ NULL, /* set_value_floating */ NULL, /* get_value */ - get_uinteger, /* get_value_uinteger */ + NULL, /* get_value_uinteger */ NULL, /* get_value_sinteger */ - NULL, /* get_value_integer64 */ + get_integer64, /* get_value_integer64 */ NULL, /* get_value_floating */ bool_eq, /* cmp_eq */ diff --git a/epan/proto.c b/epan/proto.c index 88c3e33f09..3d7c2e5860 100644 --- a/epan/proto.c +++ b/epan/proto.c @@ -239,7 +239,7 @@ proto_tree_set_system_id(field_info *fi, const guint8* value_ptr, gint length); static void proto_tree_set_system_id_tvb(field_info *fi, tvbuff_t *tvb, gint start, gint length); static void -proto_tree_set_boolean(field_info *fi, guint32 value); +proto_tree_set_boolean(field_info *fi, guint64 value); static void proto_tree_set_float(field_info *fi, float value); static void @@ -1645,7 +1645,7 @@ proto_tree_new_item(field_info *new_fi, proto_tree *tree, if (encoding) encoding = ENC_LITTLE_ENDIAN; proto_tree_set_boolean(new_fi, - get_uint_value(tree, tvb, start, length, encoding)); + get_uint64_value(tree, tvb, start, length, encoding)); break; /* XXX - make these just FT_UINT? */ @@ -2974,7 +2974,25 @@ proto_tree_set_system_id_tvb(field_info *fi, tvbuff_t *tvb, gint start, gint len static void proto_tree_set_uint64(field_info *fi, guint64 value) { - fvalue_set_integer64(&fi->value, value); + header_field_info *hfinfo; + guint64 integer; + gint no_of_bits; + + hfinfo = fi->hfinfo; + integer = value; + + if (hfinfo->bitmask) { + /* Mask out irrelevant portions */ + integer &= hfinfo->bitmask; + + /* Shift bits */ + integer >>= hfinfo_bitshift(hfinfo); + + no_of_bits = ws_count_ones(hfinfo->bitmask); + integer = ws_sign_ext64(integer, no_of_bits); + } + + fvalue_set_integer64(&fi->value, integer); } /* Add a FT_STRING, FT_STRINGZ, or FT_STRINGZPAD to a proto_tree. Creates @@ -3281,9 +3299,9 @@ proto_tree_add_boolean_format(proto_tree *tree, int hfindex, tvbuff_t *tvb, /* Set the FT_BOOLEAN value */ static void -proto_tree_set_boolean(field_info *fi, guint32 value) +proto_tree_set_boolean(field_info *fi, guint64 value) { - proto_tree_set_uint(fi, value); + proto_tree_set_uint64(fi, value); } /* Add a FT_FLOAT to a proto_tree */ @@ -4011,10 +4029,14 @@ proto_tree_set_representation_value(proto_item *pi, const char *format, va_list ITEM_LABEL_NEW(PNODE_POOL(pi), fi->rep); if (hf->bitmask && (hf->type == FT_BOOLEAN || IS_FT_UINT(hf->type))) { - guint32 val; + guint64 val; char *p; - val = fvalue_get_uinteger(&fi->value); + if (IS_FT_UINT(hf->type)) + val = fvalue_get_uinteger(&fi->value); + else + val = fvalue_get_integer64(&fi->value); + val <<= hfinfo_bitshift(hf); p = decode_bitfield_value(fi->rep->representation, val, hf->bitmask, hfinfo_bitwidth(hf)); @@ -6004,8 +6026,8 @@ fill_label_boolean(field_info *fi, gchar *label_str) { char *p = label_str; int bitfield_byte_length = 0, bitwidth; - guint32 unshifted_value; - guint32 value; + guint64 unshifted_value; + guint64 value; header_field_info *hfinfo = fi->hfinfo; const true_false_string *tfstring = (const true_false_string *)&tfs_true_false; @@ -6014,7 +6036,7 @@ fill_label_boolean(field_info *fi, gchar *label_str) tfstring = (const struct true_false_string*) hfinfo->strings; } - value = fvalue_get_uinteger(&fi->value); + value = fvalue_get_integer64(&fi->value); if (hfinfo->bitmask) { /* Figure out the bit width */ bitwidth = hfinfo_bitwidth(hfinfo); @@ -7006,9 +7028,10 @@ proto_registrar_dump_fields(void) else if (strlen(blurb) == 0) blurb = "\"\""; - printf("F\t%s\t%s\t%s\t%s\t%s\t0x%x\t%s\n", + printf("F\t%s\t%s\t%s\t%s\t%s\t0x%llx\t%s\n", hfinfo->name, hfinfo->abbrev, enum_name, - parent_hfinfo->abbrev, base_name, hfinfo->bitmask, blurb); + parent_hfinfo->abbrev, base_name, + (unsigned long long) hfinfo->bitmask, blurb); } } } @@ -7344,9 +7367,9 @@ proto_item_add_bitmask_tree(proto_item *item, tvbuff_t *tvb, const int offset, const guint encoding, const int flags, gboolean first) { - guint32 value = 0; - guint32 available_bits = 0; - guint32 tmpval; + guint64 value = 0; + guint64 available_bits = 0; + guint64 tmpval; proto_tree *tree = NULL; header_field_info *hf; @@ -7370,13 +7393,18 @@ proto_item_add_bitmask_tree(proto_item *item, tvbuff_t *tvb, const int offset, tvb_get_ntohl(tvb, offset); available_bits = 0xFFFFFFFF; break; + case 8: + value = encoding ? tvb_get_letoh64(tvb, offset) : + tvb_get_ntoh64(tvb, offset); + available_bits = 0xFFFFFFFFFFFFFFFF; + break; default: g_assert_not_reached(); } tree = proto_item_add_subtree(item, ett); while (*fields) { - guint32 present_bits; + guint64 present_bits; PROTO_REGISTRAR_GET_NTH(**fields,hf); DISSECTOR_ASSERT(hf->bitmask != 0); @@ -7408,14 +7436,14 @@ proto_item_add_bitmask_tree(proto_item *item, tvbuff_t *tvb, const int offset, const custom_fmt_func_t fmtfunc = (const custom_fmt_func_t)hf->strings; DISSECTOR_ASSERT(fmtfunc); - fmtfunc(lbl, tmpval); + fmtfunc(lbl, (guint32) tmpval); proto_item_append_text(item, "%s%s: %s", first ? "" : ", ", hf->name, lbl); first = FALSE; } else if (hf->strings) { proto_item_append_text(item, "%s%s: %s", first ? "" : ", ", - hf->name, hf_try_val_to_str_const(tmpval, hf, "Unknown")); + hf->name, hf_try_val_to_str_const((guint32) tmpval, hf, "Unknown")); first = FALSE; } else if (!(flags & BMT_NO_INT)) { @@ -7426,7 +7454,7 @@ proto_item_add_bitmask_tree(proto_item *item, tvbuff_t *tvb, const int offset, proto_item_append_text(item, ", "); } - out = hfinfo_number_value_format(hf, buf, tmpval); + out = hfinfo_number_value_format(hf, buf, (guint32) tmpval); proto_item_append_text(item, "%s: %s", hf->name, out); first = FALSE; } @@ -7691,7 +7719,7 @@ _proto_tree_add_bits_ret_val(proto_tree *tree, const int hfindex, tvbuff_t *tvb, return proto_tree_add_boolean_format(tree, hfindex, tvb, offset, length, (guint32)value, "%s = %s: %s", bf_str, hf_field->name, - (guint32)value ? tfstring->true_string : tfstring->false_string); + (guint64)value ? tfstring->true_string : tfstring->false_string); break; case FT_UINT8: @@ -7852,7 +7880,7 @@ proto_tree_add_split_bits_item_ret_val(proto_tree *tree, const int hfindex, tvbu tvb, octet_offset, octet_length, (guint32)value, "%s = %s: %s", bf_str, hf_field->name, - (guint32)value ? tfstring->true_string : tfstring->false_string); + (guint64)value ? tfstring->true_string : tfstring->false_string); break; case FT_UINT8: diff --git a/epan/proto.h b/epan/proto.h index 65a1ca2d35..eb788561c6 100644 --- a/epan/proto.h +++ b/epan/proto.h @@ -477,7 +477,7 @@ struct _header_field_info { typically converted by VALS(), RVALS() or TFS(). If this is an FT_PROTOCOL then it points to the associated protocol_t structure */ - guint32 bitmask; /**< [BITMASK] bitmask of interesting bits */ + guint64 bitmask; /**< [BITMASK] bitmask of interesting bits */ const char *blurb; /**< [FIELDDESCR] Brief description of field */ /* ------- set by proto routines (prefilled by HFILL macro, see below) ------ */ diff --git a/epan/to_str.c b/epan/to_str.c index 5b42179001..f9b876a0e0 100644 --- a/epan/to_str.c +++ b/epan/to_str.c @@ -996,15 +996,15 @@ decode_bits_in_field(const guint bit_offset, const gint no_of_bits, const guint6 Return a pointer to the character after that string. */ /*XXX this needs a buf_len check */ char * -other_decode_bitfield_value(char *buf, const guint32 val, const guint32 mask, const int width) +other_decode_bitfield_value(char *buf, const guint64 val, const guint64 mask, const int width) { int i; - guint32 bit; + guint64 bit; char *p; i = 0; p = buf; - bit = 1 << (width - 1); + bit = 1ULL << (width - 1); for (;;) { if (mask & bit) { /* This bit is part of the field. Show its value. */ @@ -1028,7 +1028,7 @@ other_decode_bitfield_value(char *buf, const guint32 val, const guint32 mask, co } char * -decode_bitfield_value(char *buf, const guint32 val, const guint32 mask, const int width) +decode_bitfield_value(char *buf, const guint64 val, const guint64 mask, const int width) { char *p; @@ -1041,7 +1041,7 @@ decode_bitfield_value(char *buf, const guint32 val, const guint32 mask, const in /* Generate a string describing a numeric bitfield (an N-bit field whose value is just a number). */ const char * -decode_numeric_bitfield(const guint32 val, const guint32 mask, const int width, +decode_numeric_bitfield(const guint64 val, const guint64 mask, const int width, const char *fmt) { char *buf; @@ -1051,7 +1051,7 @@ decode_numeric_bitfield(const guint32 val, const guint32 mask, const int width, buf=(char *)ep_alloc(1025); /* isn't this a bit overkill? */ /* Compute the number of bits we have to shift the bitfield right to extract its value. */ - while ((mask & (1<start[0] = start; bv->end[0] = end; diff --git a/ui/gtk/bytes_view.h b/ui/gtk/bytes_view.h index 78b1d54524..2df3a6a9c4 100644 --- a/ui/gtk/bytes_view.h +++ b/ui/gtk/bytes_view.h @@ -41,7 +41,7 @@ void bytes_view_set_encoding(BytesView *bv, int enc); void bytes_view_set_format(BytesView *bv, int format); void bytes_view_set_highlight_style(BytesView *bv, gboolean bold); -void bytes_view_set_highlight(BytesView *bv, int start, int end, guint32 mask, int maskle); +void bytes_view_set_highlight(BytesView *bv, int start, int end, guint64 mask, int maskle); void bytes_view_set_highlight_extra(BytesView *bv, int id, int start, int end); void bytes_view_refresh(BytesView *bv); diff --git a/ui/gtk/packet_panes.c b/ui/gtk/packet_panes.c index dcd7926d2b..36888ab61a 100644 --- a/ui/gtk/packet_panes.c +++ b/ui/gtk/packet_panes.c @@ -85,7 +85,8 @@ #define E_BYTE_VIEW_TVBUFF_KEY "byte_view_tvbuff" #define E_BYTE_VIEW_START_KEY "byte_view_start" #define E_BYTE_VIEW_END_KEY "byte_view_end" -#define E_BYTE_VIEW_MASK_KEY "byte_view_mask" +#define E_BYTE_VIEW_MASK_LO_KEY "byte_view_mask_lo" +#define E_BYTE_VIEW_MASK_HI_KEY "byte_view_mask_hi" #define E_BYTE_VIEW_MASKLE_KEY "byte_view_mask_le" #define E_BYTE_VIEW_APP_START_KEY "byte_view_app_start" #define E_BYTE_VIEW_APP_END_KEY "byte_view_app_end" @@ -838,7 +839,7 @@ savehex_cb(GtkWidget * w _U_, gpointer data _U_) static void packet_hex_update(GtkWidget *bv, const guint8 *pd, int len, int bstart, - int bend, guint32 bmask, int bmask_le, + int bend, guint64 bmask, int bmask_le, int astart, int aend, int pstart, int pend, int encoding) @@ -899,7 +900,7 @@ packet_hex_print(GtkWidget *bv, const guint8 *pd, frame_data *fd, /* to redraw the display if preferences change. */ int bstart = -1, bend = -1, blen = -1; - guint32 bmask = 0x00; int bmask_le = 0; + guint64 bmask = 0x00; int bmask_le = 0; int astart = -1, aend = -1, alen = -1; int pstart = -1, pend = -1, plen = -1; @@ -960,7 +961,7 @@ packet_hex_print(GtkWidget *bv, const guint8 *pd, frame_data *fd, /* XXX, mask has only 32 bit, later we can store bito&bitc, and use them (which should be faster) */ if (bitt > 0 && bitt < 32) { - bmask = ((1 << bitc) - 1) << ((8-bitt) & 7); + bmask = ((1ULL << bitc) - 1) << ((8-bitt) & 7); bmask_le = 0; /* ? */ } } @@ -991,7 +992,8 @@ packet_hex_print(GtkWidget *bv, const guint8 *pd, frame_data *fd, /* should we save the fd & finfo pointers instead ?? */ g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_START_KEY, GINT_TO_POINTER(bstart)); g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_END_KEY, GINT_TO_POINTER(bend)); - g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_MASK_KEY, GINT_TO_POINTER(bmask)); + g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_MASK_LO_KEY, GINT_TO_POINTER((guint32) bmask)); + g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_MASK_HI_KEY, GINT_TO_POINTER(bmask >> 32)); g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_MASKLE_KEY, GINT_TO_POINTER(bmask_le)); g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_APP_START_KEY, GINT_TO_POINTER(astart)); g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_APP_END_KEY, GINT_TO_POINTER(aend)); @@ -1013,7 +1015,7 @@ packet_hex_editor_print(GtkWidget *bv, const guint8 *pd, frame_data *fd, int off /* to redraw the display if preferences change. */ int bstart = offset, bend = (bstart != -1) ? offset+1 : -1; - guint32 bmask=0; int bmask_le = 0; + guint64 bmask=0; int bmask_le = 0; int astart = -1, aend = -1; int pstart = -1, pend = -1; @@ -1023,7 +1025,7 @@ packet_hex_editor_print(GtkWidget *bv, const guint8 *pd, frame_data *fd, int off break; case BYTES_BITS: - bmask = (1 << (7-bitoffset)); + bmask = (1ULL << (7-bitoffset)); break; default: @@ -1034,7 +1036,8 @@ packet_hex_editor_print(GtkWidget *bv, const guint8 *pd, frame_data *fd, int off /* save the information needed to redraw the text */ g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_START_KEY, GINT_TO_POINTER(bstart)); g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_END_KEY, GINT_TO_POINTER(bend)); - g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_MASK_KEY, GINT_TO_POINTER(bmask)); + g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_MASK_LO_KEY, GINT_TO_POINTER((guint32) bmask)); + g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_MASK_HI_KEY, GINT_TO_POINTER(bmask >> 32)); g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_MASKLE_KEY, GINT_TO_POINTER(bmask_le)); g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_APP_START_KEY, GINT_TO_POINTER(astart)); g_object_set_data(G_OBJECT(bv), E_BYTE_VIEW_APP_END_KEY, GINT_TO_POINTER(aend)); @@ -1053,15 +1056,17 @@ packet_hex_editor_print(GtkWidget *bv, const guint8 *pd, frame_data *fd, int off void packet_hex_reprint(GtkWidget *bv) { - int start, end, mask, mask_le, encoding; + int start, end, mask_le, encoding; int astart, aend; int pstart, pend; + guint64 mask; const guint8 *data; guint len = 0; start = GPOINTER_TO_INT(g_object_get_data(G_OBJECT(bv), E_BYTE_VIEW_START_KEY)); end = GPOINTER_TO_INT(g_object_get_data(G_OBJECT(bv), E_BYTE_VIEW_END_KEY)); - mask = GPOINTER_TO_INT(g_object_get_data(G_OBJECT(bv), E_BYTE_VIEW_MASK_KEY)); + mask = (guint64) GPOINTER_TO_INT(g_object_get_data(G_OBJECT(bv), E_BYTE_VIEW_MASK_HI_KEY)) << 32 | + GPOINTER_TO_INT(g_object_get_data(G_OBJECT(bv), E_BYTE_VIEW_MASK_LO_KEY)); mask_le = GPOINTER_TO_INT(g_object_get_data(G_OBJECT(bv), E_BYTE_VIEW_MASKLE_KEY)); astart = GPOINTER_TO_INT(g_object_get_data(G_OBJECT(bv), E_BYTE_VIEW_APP_START_KEY)); aend = GPOINTER_TO_INT(g_object_get_data(G_OBJECT(bv), E_BYTE_VIEW_APP_END_KEY)); diff --git a/ui/gtk/packet_win.c b/ui/gtk/packet_win.c index dd0e324cab..5be2aa707b 100644 --- a/ui/gtk/packet_win.c +++ b/ui/gtk/packet_win.c @@ -302,7 +302,7 @@ finfo_integer_common(struct FieldinfoWinData *DataPtr, guint64 u_val) int finfo_length = finfo->length; if (finfo_offset <= DataPtr->frame->cap_len && finfo_offset + finfo_length <= DataPtr->frame->cap_len) { - guint32 u_mask = hfinfo->bitmask; + guint64 u_mask = hfinfo->bitmask; while (finfo_length--) { guint8 *ptr = (FI_GET_FLAG(finfo, FI_LITTLE_ENDIAN)) ? diff --git a/wsutil/bits_count_ones.h b/wsutil/bits_count_ones.h index e2c2f94f3f..6a8fa1a555 100644 --- a/wsutil/bits_count_ones.h +++ b/wsutil/bits_count_ones.h @@ -37,15 +37,15 @@ */ static inline int -ws_count_ones(const guint32 x) +ws_count_ones(const guint64 x) { - int bits = x; + unsigned long long bits = x; - bits = bits - ((bits >> 1) & 0x55555555); - bits = (bits & 0x33333333) + ((bits >> 2) & 0x33333333); - bits = (bits + (bits >> 4)) & 0x0F0F0F0F; + bits = bits - ((bits >> 1) & 0x5555555555555555ULL); + bits = (bits & 0x3333333333333333ULL) + ((bits >> 2) & 0x3333333333333333ULL); + bits = (bits + (bits >> 4)) & 0x0F0F0F0F0F0F0F0F; - return (bits * 0x01010101) >> 24; + return (bits * 0x0101010101010101ULL) >> 56; } #endif /* __WSUTIL_BITS_COUNT_ONES_H__ */ diff --git a/wsutil/bits_ctz.h b/wsutil/bits_ctz.h index 9b05882bfa..dfdf24bf93 100644 --- a/wsutil/bits_ctz.h +++ b/wsutil/bits_ctz.h @@ -25,14 +25,16 @@ #include -static inline int -ws_ctz(guint32 x) -{ #if defined(__GNUC__) && ((__GNUC__ > 3) || (__GNUC__ == 3 && __GNUC_MINOR__ >= 4)) - g_assert(x != 0); - - return __builtin_ctz(x); +static inline int +ws_ctz(guint64 x) +{ + return __builtin_ctzll(x); +} #else +static inline int +__ws_ctz32(guint32 x) +{ /* From http://graphics.stanford.edu/~seander/bithacks.html#ZerosOnRightMultLookup */ static const int table[32] = { 0, 1, 28, 2, 29, 14, 24, 3, 30, 22, 20, 15, 25, 17, 4, 8, @@ -40,7 +42,19 @@ ws_ctz(guint32 x) }; return table[((guint32)((x & -(gint32)x) * 0x077CB531U)) >> 27]; -#endif } +static inline int +ws_ctz(guint64 x) +{ + guint32 hi = x >> 32; + guint32 lo = (guint32) x; + + if (lo == 0) + return 32 + __ws_ctz32(hi); + else + return __ws_ctz32(lo); +} +#endif + #endif /* __WSUTIL_BITS_CTZ_H__ */