disas/libvixl/vixl/invalset.h: possible dodgy code in binary search ?

Bug #1655700 reported by dcb
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
QEMU
Invalid
Undecided
Unassigned

Bug Description

[qemu/disas/libvixl/vixl/invalset.h:442]: (style) Array index 'low' is used before limits check.

Source code is

    while (!IsValid(elements[low]) && (low < high)) ++low;

Also:

qemu/disas/libvixl/vixl/invalset.h:450]: (style) Array index 'middle' is used before limits check.

The source code is

   while (!IsValid(elements[high]) && (low < high)) --high;

Mind you, these lines of code look similar but didn't get reported:

    while (!IsValid(elements[middle]) && (middle < high - 1)) ++middle;
    while (!IsValid(elements[middle]) && (low + 1 < middle)) --middle;

Given that binary search is notoriously tricky to get correct and a standard C library routine
I am puzzled as to why the standard library routine didn't get used, with of course a custom
comparison function.

Revision history for this message
Peter Maydell (pmaydell) wrote :

That doesn't look like a bounds check to me, so I think your checker is producing false positives.

libvixl is third-party code in any case, so stylistic questions are better directed to them upstream. But I think the difference between this code and a standard binary search is (as the comment says) that it ignores invalid elements in the array.

Changed in qemu:
status: New → Invalid
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.