Correctly handle the LSB wraparound

[Didier Barvaux]

Section 4.5.1 entitled 'Least Significant Bits (LSB) encoding' says:

   Note that the values to be encoded have a finite range; for example,
   the RTP SN ranges from 0 to 0xFFFF. When the SN value is close to 0
   or 0xFFFF, the interpretation interval can straddle the wraparound
   boundary between 0 and 0xFFFF.

This is not correctly handled by the f() function in source code.

[Didier Barvaux (didier-barvaux)]

Fix committed on trunk. See http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/251

[Didier Barvaux (didier-barvaux)]

Confirmed on the 1.3.x branch.

[Didier Barvaux (didier-barvaux)]

Confirmed on the 1.2.x branch too.

[Didier Barvaux (didier-barvaux)]

On the 1.3.x branch, add non-regression tests.
http://bazaar.launchpad.net/~didier-barvaux/rohc/1.3.x/revision/169

[Didier Barvaux (didier-barvaux)]

Update the README file to note that bug #808475 entitled "Correctly handle the LSB wraparound" will not be fixed in the 1.3.x releases. The non-regression test that was added with the previous commit will fail forever.

Fixing the problem requires many changes in the LSB and WLSB functions (especially the f() and g() functions). Great care shall be taken with integer's sizes and signedness. This is too much risk on the 1.3.x branch.

See http://bazaar.launchpad.net/~didier-barvaux/rohc/1.3.x/revision/170

[Didier Barvaux (didier-barvaux)]

Will not be fixed on the 1.2.x branch too. Same reason as on the 1.3.x branch. The README file was updated in consequence.
See http://bazaar.launchpad.net/~didier-barvaux/rohc/1.2.x/revision/141

[Jon Andersson (jon-andersson)]

Still a problem in trunk.

I get an assert in d_lsb_decode16 () after 64K ping packets between two
 rohc-nodes (Fedora 15 and Fedora 16). We uses our own application code
 that uses rohc-lib rev. 293 (trunk).

 The test performs normal pings (with TOS=0x28 and TC-script that
 redirect this TOS-traffic to our rohc application). The assert happens
 on the decompressor node.

 After starting our app, I attach gdb and break in assert() on the
 decompressor node:

 (gdb) break assert
 Function "assert" not defined.
 Make breakpoint pending on future shared library load? (y or [n]) y
 Breakpoint 1 (assert) pending.
 (gdb) c
 Continuing.

 Program received signal SIGABRT, Aborted.
 [Switching to Thread 0xb6417b70 (LWP 8444)]
 0x00a2b424 in __kernel_vsyscall ()
 (gdb) up
 #1 0x4cf6d26f in __GI_raise (sig=6) at
 ../nptl/sysdeps/unix/sysv/linux/raise.c:64
 64 return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
 (gdb) up
 #2 0x4cf6eba5 in __GI_abort () at abort.c:93
 93 raise (SIGABRT);
 (gdb) up
 #3 0x4cf65cc5 in __assert_fail_base (fmt=0x4d08ca98 "%s%s%s:%u:
 %s%sAssertion `%s' failed.\n%n",
    assertion=0xa085a8 "(decoded32 & 0xffff) == decoded32",
 file=0xa086ff "lsb.c", line=142,
    function=0xa085d9 "d_lsb_decode16") at assert.c:96
 96 abort ();
 (gdb) up
 #4 0x4cf65d77 in __GI___assert_fail (assertion=0xa085a8 "(decoded32 &
 0xffff) == decoded32",
    file=0xa086ff "lsb.c", line=142, function=0xa085d9 "d_lsb_decode16")
 at assert.c:105
 105 __assert_fail_base (_("%s%s%s:%u: %s%sAssertion `%s' failed.\n%n"),
 (gdb) up
 #5 0x00a05b55 in d_lsb_decode16 (lsb=0xb5b020ac, m=0, k=4,
 decoded=0xb64160ba) at lsb.c:142
 142 assert((decoded32 & 0xffff) == decoded32);

 ### This is the assert that is asserted ###

 (gdb) list
 137 m32 = ((uint32_t) m) & 0xffff;
 138
 139 is_success = d_lsb_decode32(lsb, m32, k, &decoded32);
 140 if(is_success)
 141 {
 142 assert((decoded32 & 0xffff) == decoded32);
 143 *decoded = (uint16_t) (decoded32 & 0xffff);
 144 }
 145
 146 return is_success;
 (gdb) p (decoded32 & 0xffff)
 $1 = 0
 (gdb) p decoded32
 $2 = 65536
 (gdb) p *lsb
 $3 = {v_ref_d = 65535, old_v_ref_d = 65534, p = 3}
 (gdb) p *decoded
 $4 = 0
 (gdb) up
 #6 0x00ba3113 in decode_uo0 (decomp=0xb5b01aa8, context=0xb5b02040,
 rohc_packet=0x9f9828e "\002", rohc_length=67,
    second_byte=1, uncomp_packet=0xb6416af4 "E(") at d_generic.c:2977
 2977 ret = d_lsb_decode16(&g_context->sn, sn_bits, sn_bits_nr,
 &sn_decoded);
 (gdb) p *context
 $5 = {profile = 0xbacae0, specific = 0xb5b02098, mode = O_MODE, state =
 FULL_CONTEXT, latest_used = 3257851250,
  first_used = 3257814454, curval = 0, total_uncompressed_size =
 5504940, total_compressed_size = 4390928,
  header_uncompressed_size = 1310700, header_compressed_size = 196683,
 num_recv_packets = 65535, num_recv_ir = 3,
  num_recv_ir_dyn = 3, num_sent_feedbacks = 1, num_decomp_failures = 0,
 num_decomp_repairs = 0,
  total_16_uncompressed = 0xb5b02928, total_...

[Didier Barvaux (didier-barvaux)]

The assert() statement is too strict.

[Didier Barvaux (didier-barvaux)]

The assert() statement was removed on trunk. See http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/298.

A unit test for LSB encoding/decoding at wraparound was also added. See http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/297.