Debugger crashes while trying to debug xusb

[pulkomandy]

This happened to me while trying to debug xusb, one of the test programs from the libusb. To reproduce:

• Get libusb at ​http://github.com/pulkomandy/libusbx/
• Compile it (usual ./configure and make)
• Plug some USB mass storage device
• Run examples/xusb -k 090c:1000 (replace with vid:pid of your device)

The program crashes, but attempting to debug it crashes Debugger.

This executable is unfortunately not easy to extract from libusb build tree. It's run with a shell script wrapper generated by libtool, so running "gdb xusb" won't work...

[anevilyak]

I'm unfortunately not able to reproduce this crash over here, it may be device-specific. All I get is:

/Data/devel/libusbx> examples/xusb -k 1058:0748
Using libusbx v1.0.17.10853

Opening device 1058:0748...
libusbx: info [haiku_open_device] Device open for access

Reading device descriptor:
            length: 18
      device class: 0
               S/N: 5
           VID:PID: 1058:0748
         bcdDevice: 1022
   iMan:iProd:iSer: 1:2:5
          nb confs: 1

Reading BOS descriptor: 2 caps
    USB 2.0 extension:
      attributes             : 02
    USB 3.0 capabilities:
      attributes             : 00
      supported speeds       : 000E
      supported functionality: 01

Reading first configuration descriptor:
libusbx: error [libusb_get_config_descriptor] short config descriptor read 0/9
   Input/Output Error
libusbx: warning [libusb_exit] application left some devices open

[anevilyak]

Never mind, found another device around the apartment that it does crash with, investigating...

[anevilyak]

The problem relates to DWARF4 implicit values, seemingly the build scripts for libusbx force the use of that version of the standard, as that isn't yet gcc's default. In any case, what occurs here specifically is that copies of the ValuePieceLocation are made via operator= when adding it to an Array. As this only does a shallow copy by default, both pieces wind up having a pointer to the same heap allocated memory block, and consequently when the original goes off the stack, it frees said pointer, leaving the other in the array pointing to already freed memory.

Attached is a patch that overrides the = operator to force a malloc + memcpy in that circumstance, but I'm not entirely happy with that solution, since it doesn't readily allow detection of out of memory conditions without introducing exception handling. My initial thought was to instead introduce a new BReferenceable subclass specifically for storing implicit value data, and then have ValuePieceLocation simply store a reference to such an object rather than a void pointer, so that the assignment operator would simply add a reference and deal with it from there. However, this is problematic due to ValueLocation's use of the Array class for storing its pieces. The latter uses malloc/memcpy/memmove to manage its internal array, and consequently doesn't actually wind up calling object constructors for its templated type. This in turn leads to a crash here, since this means it calls our overridden operator= on an uninitialized block of memory, and in turn, its member BReferenceable has an invalid pointer that it thinks it must release before assigning the new reference. If the Array class is generally intended to be used with C++ objects, that behavior seems suboptimal, but I'm uncertain as to the best way to fix it. Any thoughts/alternative suggested approaches?

[bonefish]

I wrote Array mainly to store PODs (or more generally: relocatable structures without destructor) with the guarantee that they are laid out as an array (IIRC std::vector didn't guarantee that back then (it does now)). I considered adding a template argument to indicate whether the element type is a POD, so proper construction and destruction could be done in those cases, but didn't need it back then.

Long story short, since ValuePieceLocation has a destructor, a different container should be used. Copy constructor and assignment operator should either throw std::bad_alloc or they should be made private and an explicit copy method (as in your patch) should be used. In the first case std::vector would a suitable container, since it throws anyway. Furthermore all other uses of copy constructor/assignment operator would need to catch the exception as well.

[pulkomandy]

FYI: I found the bug I had on libusb side, and fixed it. The version that crashes is ​https://github.com/pulkomandy/libusbx/commit/62f035573dced8c93c96e491ac254b2ed2fc65a9 .

[anevilyak]

Debugger problem fixed in hrev46585.