Vector35 / binaryninja-api

Public API, examples, documentation and issues for Binary Ninja
https://binary.ninja/
MIT License
946 stars 214 forks source link

Do not show nums or:ed with 0 if any bits are set #6216

Open RevPanda opened 19 hours ago

RevPanda commented 19 hours ago

Version and Platform (required):

Bug Description: BN displays enums as or:ed bitfields when used, and when 0 is named in the enum that is always or:ed in. This is arguably technically correct but also reduces readability for no good reason.

Steps To Reproduce: Define enum with 0 and something else. Use it with the variable set to anything but 0.

enum flags : uint32_t
{
    WRITABLE = 0x1,
    READABLE = 0x2,
    WARPABLE = 0x4,
    NONE     = 0x0
};
...
subspace_open(fd: 42, flags: READABLE | WARPABLE | NONE)

Another more obviously unhelpful example from Slack:

enum Bool : uint8_t
{
    YES = 0x1,
    NO  = 0x0
};
...
featuresHaveBeenInitialized = YES | NO

Expected Behavior: If any bits are set, do not show the 0:

subspace_open(fd: 42, flags: READABLE | WARPABLE)

Additional Information: This could be made a display option, but before going that route, try to figure out a real world example where having 0 or:ed in would be useful. At the very least it should not be the default.

RevPanda commented 18 hours ago

When trying to make a minimized binary, this did not reproduce. Turns out if the 0 is defined at the top of the enum it will not be displayed. If it's at the bottom it will.

RevPanda commented 18 hours ago

open-example.tar.gz

Small test that just opens a file and reads 99 bytes from it to make sure nothing is optimized away. Linux-x64 binary and source included. Define an enum and set the type of oflag in open():

enum oflags : uint32_t
{
    O_RDONLY = 0x0,
    O_WRONLY = 0x1
};
=>
read(fd: open(file: "/tmp/foo", oflag: O_WRONLY), buf: malloc(bytes: 0x64), nbytes: 0x63)

Redefine the enum and reanalyze the function to see the bug:

enum oflags : uint32_t
{
    O_WRONLY = 0x1,
    O_RDONLY = 0x0
};
=>
read(fd: open(file: "/tmp/foo", oflag: O_WRONLY | O_RDONLY), buf: malloc(bytes: 0x64), nbytes: 0x63)