[ppc64le] test-idmap unit test failure

[Tony Espy]

We made a change recently to the ofono ( 1.12+bzr6851-0ubuntu2 ) in debian/rules to enable -Wall.

This had the unfortunate side-effect of breaking the build on powerpc ( which isn't run by CI, it's only triggered by a release ) as the unit tests for the low-level grilrequest.c, grilreply.c, and grilunsol.c ( used by rilmodem ) include #if statements to prevent compilation if the BYTE_ORDER of the platform is not LITTLE_ENDIAN.

This is due to the fact that the unit tests include byte arrays that represent Binder parcels in wire-format, and unfortunately the Binder wire-format differs slightly depending on endian-ness of the platform.

The short-term fix was to extend the #if BYTE_ORDER check to cover the entire unit test files, so that errors are not thrown for the unused functions and variables.

To fix correctly will require fixes to the parcel data bytes arrays via further #if statements.

[Tony Espy (awe)]

A couple of comments...

1. The idmap code implements a collection used to trace the use of generic ids ( unsigned int ). It's only usage within ofono is within the gprs code for the purpose of tracking context ids.

2. The code uses bit arrays overlaid onto unsigned longs. The bits are set/cleared using a bit shit operator ( << ). From what I can tell this operation only supports 32-bits, and wraps when given a value that exceeds 32 bits.

3. On standard 64-bit architectures, the bit-shift operator wraps as described above. On ppc64le, it returns 0 past 32-bits. This is what causes the failure of the idmap_alloc_next() operation when it's expected to wrap from the maximum idmap value to the minimum ( if available ).

4. I'm working on creating a pull-request which will adjust the unit test to use a smaller idmap, and to exclude the wrap case ( ie. from 256->1 ).

[Tony Espy (awe)]

Pull request created:

https://github.com/rilmodem/ofono/pull/49

[Tony Espy (awe)]

Here's a test program that demonstrates the left bitwise shit operator and it's return values with the right operand incremented from 1-64.

On amd64, the return value wraps back to 1 when the right operand hits 32, and the sequence of return values follows the original sequence.

On ppc64le, once the right operand hits 32, the return value is 0, and stays that way through values up to 64.

[Matthias Klose (doko)]

this is undefined behaviour.

C99 6.5.7.3: "If the value of the right operand is negative or is greater than or equal to the width of the promoted left operand, the behavior is undefined."

[Launchpad Janitor (janitor)]

This bug was fixed in the package ofono - 1.12+bzr6856-0ubuntu1

---------------
ofono (1.12+bzr6856-0ubuntu1) trusty; urgency=low

  [ Tony Espy ]
  * unit: fix fail-to-build on powerpc
    The previously released version enabled -Wall,
    which triggered build failures due to unused
    functions on powerpc. This is due to the fact
    that the unit tests only run on LITTLE_ENDIAN
    systems via an ifdef, which leaves unused functions
    and data. This change exends the ifdef to cover
    the unused functions and test data.
  * idmap: use UL for bitshift literals (LP: #1271284)
    This change fixes a failing unit test on ppc64le
    due to the undefined behavior when the left bitshift
    operator is given a value that excedes the size of
    the value being shifted.
  * gril, rilmodem/sim, unit: fix SIM IO crash (LP: #1268743)
    - cleanup gril_reply_parse_sim_io() to add malformed parcel
      check and fix memory leak.
    - add check for null hex_response in ril_file_io_cb(), as
      the emulator can return such responses.
    - add additional unit tests to cover crash scenarios.
  * ril, src: enable message-waiting-interface
    - register message_waiting atom in ril plugin
    - register message_waiting atom in ril plugin
    - fix sms_mwi_dcs_decode bug which prevented incoming message
      waiting indications from being set.
  * rilmodem/voicecall: fix call-decline bug (LP: #1260988)
    Send a RIL_REQUEST_HANGUP_WAITING_OR_BACKGROUND instead of
    was used correctly for displaying operator name, however
    some modems failed to handle roaming correctly for MVNOs.
    This fix is transparent to modems that do the right thing.
  * gril/gril.c, plugins/ril.c: API changes for OEMS
    This change introduces a socket-path to the g_ril_new()
    function, and also adds a new disconnect function.
  * gril/grilunsol.c: add support for v5 signal strength message
  * debian/control: adjust ofono-scripts dependencies for Python 3
  * test/rilmodem: add copyright/license headers

  [ Martin Pitt ]
  * test: convert tests scripts to Python 3 (LP: #1283571)
 -- Ricardo Salveti de Araujo <email address hidden> Fri, 28 Feb 2014 17:50:54 -0300