Re: ASN.1 integers and types

[Pascal Quantin <pascal.quantin () gmail com>]

2012/7/12 Pascal Quantin <pascal.quantin () gmail com>

> 2012/7/10 Pascal Quantin <pascal.quantin () gmail com>
>
> > 2012/7/10 Anders Broman <a.broman () bredband net>
> >
> > > Guy Harris skrev 2012-07-10 01:34:
> > >
> > >  On Jul 9, 2012, at 2:41 PM, buildbot-no-reply@wireshark.**org<buildbot-no-reply () wireshark org>wrote:
> > > >  The Buildbot has detected a new failure on builder OSX-10.5-x86 while
> > > > > building Wireshark (development).
> > > > > Full details are available at:
> > > > > http://buildbot.wireshark.org/**trunk/builders/OSX-10.5-x86/**
> > > > > builds/1821<http://buildbot.wireshark.org/trunk/builders/OSX-10.5-x86/builds/1821>
> > > > >
> > > > > Buildbot URL: http://buildbot.wireshark.org/**trunk/<http://buildbot.wireshark.org/trunk/>
> > > > >
> > > > > Buildslave for this Build: osx-10.5-x86
> > > > >
> > > > > Build Reason: scheduler
> > > > > Build Source Stamp: 43629
> > > > > Blamelist: pascal
> > > > >
> > > > > BUILD FAILED: failed compile
> > > > /bin/sh ../../libtool --tag=CC   --mode=compile ccache gcc
> > > > -DHAVE_CONFIG_H -I. -I../.. -I./../.. -I./.. -I/usr/local/include
> > > > -I/usr/local/include  -DINET6 -DG_DISABLE_DEPRECATED
> > > > -DG_DISABLE_SINGLE_INCLUDES -DGTK_DISABLE_DEPRECATED -DGTK_DISAB
> > > > LE_SINGLE_INCLUDES -D_U_="__attribute__((unused))**"
> > > >  -D_FORTIFY_SOURCE=2 -I/usr/local/include  '-DPLUGIN_DIR="/usr/local/lib/
> > > > **wireshark/plugins/1.9.0-SVN-**43629"' -Werror -no-cpp-precomp -g -O2
> > > > -Wall -W -Wextra -Wdeclaration-after-statement -Wendif-labels
> > > > -Wpointer-arith -Wno-pointer-sign -Wcast-align -Wformat-security
> > > > -Wold-style-definition -I/usr/X11/include/freetype2 -I/usr/X11/include
> > > > -I/usr/X11/include/libpng12 -I/usr/X11/include/pixman-1
> > > > -I/usr/local/include/gtk-2.0 -I/usr/local/lib/gtk-2.0/**include
> > > > -I/usr/local/include/atk-1.0 -I/usr/local/include/cairo
> > > > -I/usr/local/include/pango-1.0 -I/usr/local/include/glib-2.0
> > > > -I/usr/local/lib/glib-2.0/**include   -MT libdissectors_la-packet-mpeg-
> > > > **audio.lo -MD -MP -MF .deps/libdissectors_la-packet-**mpeg-audio.Tpo
> > > > -c -o libdissectors_la-packet-mpeg-**audio.lo `test -f
> > > > 'packet-mpeg-audio.c' || echo './'`packet-mpeg-audio.c
> > > > cc1: warnings being treated as errors
> > > > ../../asn1/lpp/lpp.cnf: In function 'dissect_lpp_INTEGER_**M2147483648_
> > > > 2147483647':
> > > > ../../asn1/lpp/lpp.cnf:1328: warning: this decimal constant is unsigned
> > > > only in ISO C90
> > > >
> > > > The offending code is
> > > >
> > > > static int
> > > > dissect_lpp_INTEGER_**M2147483648_2147483647(tvbuff_t *tvb _U_, int
> > > > offset _U_, asn1_ctx_t *actx _U_, proto_tree *tree _U_, int hf_index _U_) {
> > > >    offset = dissect_per_constrained_**integer(tvb, offset, actx, tree,
> > > > hf_index,
> > > >                                                              -
> > > > 2147483648, 2147483647U, NULL, FALSE);
> > > >
> > > >    return offset;
> > > > }
> > > >
> > > > The two integral constants in that call correspond to guint32
> > > > arguments; the first of those is, obviously, negative.
> > > >
> > > > ITU-T Recommendation X.680 says
> > > >
> > > >         3.8.40  integer type: A simple type with distinguished values
> > > > which are the positive and negative whole numbers, including zero (as a
> > > > single value).
> > > >
> > > >                 NOTE – Particular encoding rules limit the range of an
> > > > integer, but such limitations are chosen so as not to affect any user of
> > > > ASN.1.
> > > >
> > > > and I don't see anything about a type that includes only the positive
> > > > whole numbers, including zero - i.e., no unsigned type - so perhaps, if
> > > > we're to strictly interpret ASN.1, either all integral types should be
> > > > signed, and any integral type with values>  2147483647 should be
> > > > 64-bit (or we should introduce bignums so that we don't have to bail out on
> > > > *any* ASN.1 number) or, at least, all *unconstrained* integral types should
> > > > be signed and 64-bit (or, if we introduce bignums, bignum), with only
> > > > integral types constrained to a minimum value>= 0 unsigned, and with the
> > > > width of the type chosen based on the constraints.
> > > >
> > > > The right short-term fix might be to make the arguments to
> > > > dissect_per_constrained_**integer() be gint64 rather than guint32 -
> > > > and perhaps have the pointer for the returned value be a gint64 * rather
> > > > than a guint32 *.
> > > Yes I vote for that.
> >
> >
> > Hi Guy and Anders,
> >
> > sorry for the breakage, it was compiling fine with both my Ubuntu and
> > Windows boxes :/
> >
> > By modifying a bit asn2wrs.py so as to add G_GINT64_CONSTANT for -2^31
> > also fixes the compilation but I'm not sure this is the best idea...
> > Index: tools/asn2wrs.py
> > ===================================================================
> > --- tools/asn2wrs.py    (revision 43636)
> > +++ tools/asn2wrs.py    (working copy)
> > @@ -3225,7 +3225,7 @@
> >      if str(minv).isdigit():
> >        minv += 'U'
> >      elif (str(minv)[0] == "-") and str(minv)[1:].isdigit():
> > -      if (long(minv) < -(2**31)):
> > +      if (long(minv) <= -(2**31)):
> >          minv = "G_GINT64_CONSTANT(%s)" % (str(minv))
> >      if str(maxv).isdigit():
> >        if (long(maxv) >= 2**32):
>
> Hi,
>
> According to
> http://stackoverflow.com/questions/9941261/warning-this-decimal-constant-is-unsigned-only-in-iso-c90the warning we 
> see is expected (that's why INT32_MIN is defined as
> (-INT32_MAX - 1L) for example).
> Does the use of the G_GINT64_CONSTANT macro for this specific value (as
> done in the patch above) seems an acceptable workaround for everybody?
> Alternatively we could change asn2wrs.py so as to replace -2^31 by
> G_MININT32 (that also removes the warning).

Unless anyone objects in the next few days, I intend to replace -2^31 by
G_MININT32 and move the impacted ASN.1 dissectors from dirty to clean
library.

Regards,
Pascal.

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe