Suspected clang problem with x86 build

[Davy Wentzler]

Background:

I'm using libusb to access USB audio devices on Android. I recently moved from NDKr10e to NDKr16b and after release I received a couple of bug reports that USB devices were not recognized anymore. The users who reported this used a x86 system (Dell Venue, x86 on Android, etc.).

After digging for hours into libusb, I found that parsing the USB descriptors was going wrong. The issue is in descriptor.c of libusb, but I've tried to reduce the code as much as possible. However, it's not very simple: adding a debug statement or removing a memcpy() in a code path that is not executed will make it work again! I've spent a whole day trying to reduce the code more, but everything I change something, it starts working. Stack/register problem??

The bug is that it enters an if statement that is false...

Notes are in the comments, but most importantly, using NDK_TOOLCHAIN_VERSION := 4.9 makes it work again on x86. On ARM devices, it works regardless of NDK or gcc/clang.

Any help appreciated since this blocks me from moving to clang on a x86 build at least.

struct test_header
{
   uint8_t  bLength;
   uint8_t  bDescriptorType;
} __attribute__((packed)); // packed or not, it does not matter


void parse_descriptor(const unsigned char *source, const char *descriptor, void *dest)
{
   const unsigned char *sp = source;
   unsigned char *dp = (unsigned char*) dest;
   const char *cp = descriptor; // same problem when hardcoded to 'bb'
   const int size = strlen(descriptor); // checked, it's 2

    for (int i = 0; i < size; i++)
   {
      switch (*cp)
      {
         case 'b':  /* 8-bit byte, the initial call should enter this case statement twice, since descriptor is 'bb' */
         {
            *dp++ = *sp++;
            break;
         }
         case 'd':  /* 32-bit word, convert from little endian to CPU */
         {
            break;
         }
         case 'u':  /* 16 byte UUID */
         {
            memcpy(dp, sp, 16); // commenting this line makes it work!!!!!!!
            break;
         }
      }

      cp++;
   }
}


// This code originates from the libusb library, and was modified and shortened as much as possible to
// reproduce the problem.
// the idea of this function, which originally is much longer, is to parse raw USB descriptors
// and map them to structs. It first tries to fill struct 'test_header' which consists of two uint8_t's.
// The code assumes that these two uint8_t's are aligned as 16 bits in total
static void parse_interface(unsigned char *buffer, int size)
{
   int parsed = 0;
   struct test_header header;

    // the first two bytes of buffer are 9 and 33. They should map to test_header, such that bLength = 9 and bDescriptorType = 33;

    int outer = 0;
   while (size >= INTERFACE_DESC_LENGTH && outer < 4)
    {
      /* Skip over any interface, class or vendor descriptors */
      while (size >= DESC_HEADER_LENGTH)
      {
         parse_descriptor(buffer, "bb", &header);

            // uncommenting this __android_log_print makes it work!!!
            //__android_log_print(ANDROID_LOG_DEBUG, "Main", "header len = %u", header.bLength);

         if (header.bLength < DESC_HEADER_LENGTH)
         {
            __android_log_print(ANDROID_LOG_DEBUG, "Main", "invalid extra intf desc len (%d)", header.bLength);
            return;
         }
         else if (header.bLength > size)
         {
            __android_log_print(ANDROID_LOG_DEBUG, "Main", "short extra intf desc read %d/%d", size, header.bLength);
            return;
         }

            // here, the bug shows: the code tests for 4, 5, 2 or 1, but the IF is entered!
         if ((header.bDescriptorType == 4) ||
                (header.bDescriptorType == 5) ||
                (header.bDescriptorType == 2) ||
                (header.bDescriptorType == 1))
         {
             // using clang, NDKr16b, compiled for x86, this prints on a Dell Venu 8 (Intel Atom):
             // bDescriptorType = 33, outer = 0
                // Done, size = 115, parsed = 0, header.bLength = 0, header.bDescriptorType = 33
                // but 33 isn't part of the IF statement!
                // When built with NDKr16b, but with NDK_TOOLCHAIN_VERSION := 4.9 in Application.mk or building with NDKr10e, all is fine (which means gcc is used)
                // On ARM devices, all is fine, regardless of gcc or clang or NDK version. Correct/expected output would be:
                // bDescriptorType = 5, outer = 0
                // Done, size = 106, parsed = 9, header.bLength = 7, header.bDescriptorType = 5
             __android_log_print(ANDROID_LOG_DEBUG, "Main", "bDescriptorType = %u, outer = %d", header.bDescriptorType, outer);
             __android_log_print(ANDROID_LOG_DEBUG, "Main", "Done, size = %d, parsed = %d, header.bLength = %u, header.bDescriptorType = %u", size, parsed, header.bLength, header.bDescriptorType);
            break;
         }

         buffer += header.bLength;
         parsed += header.bLength;
         size -= header.bLength;
      }

      __android_log_print(ANDROID_LOG_DEBUG, "Main", "outer = %d", outer);
      outer++;
   }

    __android_log_print(ANDROID_LOG_DEBUG, "Main", "END outer = %d", outer);
}


const unsigned char buf[115] =
{
 9,
 33,
 17,
 1,
 0,
 1,
 34,
 52,
 0,
 7,
 5,
 129,
 3,
 32,
 0,
 1,
 7,
 5,
 2,
 3,
 32,
 0,
 1,
 9,
 4,
 1,
 0,
 0,
 1,
 1,
 0,
 0,
 9,
 36,
 1,
 0,
 1,
 9,
 0,
 1,
 2,

 9,
 4,
 2,
 0,
 2,
 1,
 3,
 0,
 0,
 7,
 36,
 1,
 0,
 1,
 65,
 0,
 6,
 36,
 2,
 1,
 1,
 0,
 6,
 36,
 2,
 2,
 2,
 0,
 9,
 36,
 3,
 1,
 3,
 1,
 2,
 1,
 0,
 9,
 36,
 3,
 2,
 4,
 1,
 1,
 1,
 0,
 9,
 5,
 3,
 2,

 64,
  0,
  0,
  0,
  0,
  5,
  37,
  1,
  1,
  1,
  9,
  5,
  132,
  2,
  64,
  0,
  0,
  0,
  0,
  5,
  37,
  1,
  1,
  3
};


static void runTest()
{
    wxLogDebugMain("TR");

    parse_interface((unsigned char *) buf, 115);
    wxLogDebugMain("TR done");
}

[Davy Wentzler]

sorry guys, moving this to the issues section