9
\$\begingroup\$

Binary Visualization based on Christopher Domas talk: youtube link and this Russian programmer where I discovered this talk: youtube link.

Algorithm:

  • Scan the pairs of bytes of a file with a sliding window. (AA BB CC DD ... -> (AA, BB) (BB, CC) (CC, DD) ...);
  • Interpret the pairs of bytes as coordinates on a 256x256 2D plain;
  • Place a dot for each pair on the plain;
  • The more frequent the dot, the brighter it is;
  • Different patterns emerge depending on the type of the data of the file.

Sample outputs:

Executables: (gdb - Intel x86_64, binviz - ARM64, git-bash - Windows PE 32+, pandoc - Intel x86_64)

Intel x86_64, gdb ARM64, this program's executable Windows PE 32+, git-bash Intel x86_64, pandoc

Text Files: (war and peace, stb_image_write.h)

war_and_peace.txt stb_image_write.h

Audio File: (streets.wav)

streets.wav

Code:

This stb-library was used for the image generation.

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <string.h>
#include <errno.h>
#include <tgmath.h>

#define STB_IMAGE_WRITE_IMPLEMENTATION
#define STB_IMAGE_WRITE_STATIC
#include "stb_image_write.h"

#define MAP_SIZE            256
#define OUTPUT_FNAME_EXT    ".binviz.png"
#define OUTPUT_FPATH_SIZE   256
#define CHUNK_SIZE          1024 * 8

static size_t map[MAP_SIZE][MAP_SIZE];
static int32_t pixels[MAP_SIZE][MAP_SIZE];

static FILE *xfopen(const char path[static 1])
{
    errno = 0;
    FILE *const fp = fopen(path, "rb");

    if (!fp) {
        fprintf(stderr, "Error: could not open file %s: %s.\n", path,
            errno ? strerror(errno) : "");
        exit(EXIT_FAILURE);
    }
    return fp;
}

static char *read_next_chunk(FILE stream[static 1], char chunk[static CHUNK_SIZE], size_t *size)
{
    if (size) {
        *size = 0;
    }
    
    const size_t rcount = fread(chunk, 1, CHUNK_SIZE, stream);

    if (rcount < CHUNK_SIZE) {
        if (!feof(stream)) {
            /* A read error occured. */
            return NULL;
        }

        if (rcount == 0) {
            return NULL;
        }
    }
    
    chunk[rcount] = '\0';

    if (size) {
        *size = rcount;
    }

    return chunk;
}

static _Bool process_file(FILE stream[static 1])
{
    char content[CHUNK_SIZE];
    char *p = NULL;
    size_t nbytes = 0;

    while ((p = read_next_chunk(stream, content, &nbytes))) {
        for (size_t i = 0; i < nbytes - 1; ++i) {
            uint8_t x = (uint8_t)content[i];
            uint8_t y = (uint8_t)content[i + 1];

            map[y][x] += 1;
        }

        float max = 0;

        for (size_t y = 0; y < MAP_SIZE; ++y) {
            for (size_t x = 0; x < MAP_SIZE; ++x) {
                float f = 0.0f;

                if (map[y][x] > 0) {
                    f = log(map[y][x]);
                }

                if (f > max) {
                    max = f;
                }
            }
        }

        for (size_t y = 0; y < MAP_SIZE; ++y) {
            for (size_t x = 0; x < MAP_SIZE; ++x) {
                float t = log(map[y][x]) / max;
                uint32_t b = t * MAP_SIZE;

                pixels[y][x] = 0XFF000000 | b | (b << 8) | (b << 16);
            }
        }
    }

    return feof(stream);
}

int main(int argc, char **argv)
{
    /* Sanity check. POSIX requires the invoking process to pass a non-NULL 
     * argv[0]. 
     */
    if (!argv[0]) {
        fprintf(stderr,
            "A NULL argv[0] was passed through an exec system call.\n");
        return EXIT_FAILURE;
    }

    if (argc != 2) {
        fprintf(stderr, "Error: no file provided.\n"
            "Usage: %s <filename>.\n", argv[0]);
        return EXIT_FAILURE;
    }

    const char *const input = argv[1];
    FILE *const fp = xfopen(input);
    
    if (!process_file(fp)) {
        fprintf(stderr, "Error: an unexpected error occured while reading.\n");
        fclose(fp);
        return EXIT_FAILURE;
    }

    fclose(fp);

    char out_fpath[OUTPUT_FPATH_SIZE];

    sprintf(out_fpath, "%s" OUTPUT_FNAME_EXT, input);
    const int rv =
        stbi_write_png(out_fpath, MAP_SIZE, MAP_SIZE, 4, pixels,
        MAP_SIZE * sizeof pixels[0][0]);

    if (rv == 0) {
        fprintf(stderr, "Error: could not save image %s.\n", out_fpath);
        return EXIT_FAILURE;
    }

    return EXIT_SUCCESS;
}

Review Request:

It is rather slow when the files are a little large. For instance, it took 23 seconds to process the WAV file, which was 41 MBs large. I was previously reading the whole file into memory, but my system is low on memory, and the program simply crashed for anything sizing 1.5 GB. How can I improve the performance?

General coding comments, style, et cetera.

\$\endgroup\$

4 Answers 4

9
\$\begingroup\$

It seems the program overwrites pixels each time it processes a chunk

this means wasting 2*MAP_SIZE*MAP_SIZE log operations for every partial "rendering" of pixels

To ensure the separation of the 2 phases consider using different functions

static _Bool process_file_onto_map(FILE stream[static 1])
{
    char content[CHUNK_SIZE];
    char *p = NULL;
    size_t nbytes = 0;

    while ((p = read_next_chunk(stream, content, &nbytes))) {
        for (size_t i = 0; i < nbytes - 1; ++i) {
            uint8_t x = (uint8_t)content[i];
            uint8_t y = (uint8_t)content[i + 1];

            map[y][x] += 1;
        }
    }

    return feof(stream);
}

static void render_map_onto_pixels(void){
    float max = 0;
    ...
}

Be careful that in float t = log(map[y][x]) / max; :

  • max could be zero (division by zero)
  • map[y][x] could be zero (pole error in log)

both cases lead to UB in the subsequent conversion to uint32_t


Also, regarding the "rendering", notice that the max value of log is the log of the max value, which spares half of the log calculations

size_t max_value = 0;

for (size_t y = 0; y < MAP_SIZE; ++y) {
    for (size_t x = 0; x < MAP_SIZE; ++x) {
        if (map[y][x] > max_value) {
            max_value = map[y][x];
        }
    }
}

float max = 0;
if (max_value != 0) max = log(max_value);

By splitting the calculation into chunks the program is missing all the in-between points:

if we had {(chunk_1 bytes)..., 0xHH}, {0xKK, ...(chunk_2 bytes)}, the (0xHH, 0xKK) point won't be counted; this could be done by keeping a record of the last byte in the chunk

bool is_first = true;
uint8_t last_byte;
while (...) {
    if (!is_first) map[last_byte][(uint8_t)content[0]] += 1;
    else is_first = false;
    ...
    last_byte = (uint8_t)content[nbytes-1];
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ "max could be zero (division by zero)" => Yes, execution should not reach the for loops if the max value was 0 and the function should return. That would take care of both case of undefined behavior. \$\endgroup\$
    – Harith
    Commented Mar 10 at 10:19
8
\$\begingroup\$

I will be mostly focusing on performance. All benchmarks are done using the time commands on an 2018 MacBook Air (8GiB) using a 32MiB PDF file. I will be computing the sha256sum of the generated image to ensure correctness.


Enabling optimizations

Make sure you compile with optimizations enabled. For instance, processing the PDF file without optimizations take ~7.5s on my laptop. With -O3 optimizations, the time goes down to ~4.5s, a 1.67x increase.


Computing the max

You compute the max value by iterating through the entire map every time you read a chunk. This is wasteful, especially since you're calculating the log of each value, which can be slow. Instead, you can just keep track of the max value when updating the map and calculate the log(max_value) when writing to pixels.

int maxv = 0;
...
map[y][x] += 1;
if (map[y][x] > maxv) {
  maxv = map[y][x];
}
...
float max = log(maxv);
for (size_t y = 0; y < MAP_SIZE; ++y) {
   for (size_t x = 0; x < MAP_SIZE; ++x) {
       float t = log(map[y][x]) / max;
    ...

This takes the runtime from ~4.5s to ~2.4s, a 1.87x increase.


Writing the pixels outside the loop

By far, the biggest culprit when it comes to runtime is the loop that writes to the pixels array. Computing log may not be cheap and since you're processing the binary file in 8KiB chunks, you end up doing (256 * 256 * n * 1024 / 8) log computations when n is the size of the binary file in MiB. So, for a 40MiB file, you're computing ~335 million log computations.

This loop can be easily moved out the chunk loop.

while ((p = read_next_chunk(stream, content, &nytes)) {
   for (size_t i = 0; i < nbytes; i++) {
         uint8_t x = (uint8_t)content[i];
         uint8_t y = (uint8_t)content[i+1];
         map[y][x] += 1;
         if (map[y][x] > maxv) {
           maxv = map[y][x];
         }
   }
}

float max = log(maxv);
for (size_t y = 0; y < MAP_SIZE; ++y) {
    for (size_t x = 0; x < MAP_SIZE; ++x) {
        float t = log(map[y][x]) / max;
        uint32_t b = t * MAP_SIZE;

        pixels[y][x] = 0XFF000000 | b | (b << 8) | (b << 16);
    }
}

This leads to a drastic increase in performance, going from ~4.5s (~2.4s after the first optimization) to just a mere ~0.1s, a 45x increase from the original.


Reusing the pixels array

You don't need the map array. You can just use the pixels array to store the value itself, and compute the final value once all processing is done.

...
uint8_t x = content[i];
uint8_t y = content[i+1];
pixels[y][x] += 1;

...

for (size_t y = 0; y < MAP_SIZE; y++) {
   for (size_t x = 0; x < MAP_SIZE; x++) { 
      float t = log(pixels[y][x]) / max;
       ...
   }
}

This doesn't impact the runtime performance, but does some amount of memory.


General comments

  • [static N]: While still valid C, I would personally recommend against using this as it's not common-place and can cause confusion among those who read your cod.

  • _Bool: Include stdbool.h and use bool, it's more idiomatic. No one uses _Bool directly.

  • read_next_chunk can return a bool instead of char*.

  • argv[0] check: Is that necessary? Unless it impacts the correctness your program somehow, you should elide the check.

\$\endgroup\$
2
  • \$\begingroup\$ I had never seen the [static N] syntax and it puzzled me, too, but I find it just so expressive that I would prefer to have it. It addresses one of the core criticisms of C, which is pointers, without implying any runtime overhead -- well, at least if I understand it correctly. \$\endgroup\$
    – uli
    Commented Mar 13 at 12:30
  • \$\begingroup\$ @uli It has been here since C99. Per 6.7.6.3 7, the pointer must point to at least N elements. This means the pointer must not be null, which the compiler can use to optimize some things. Some examples: double a[static 1] is a pointer to a single object. double a[static 10] is a pointer to a collection of objects of known (10) members. double a[n] is a pointer to a collection of objects of unknown numbers. If a null pointer is still passed in, the compiler is required to raise a diagnostic message. And double *a denotes a pointer to a single object of the type of a null pointer. \$\endgroup\$
    – Harith
    Commented Mar 13 at 15:29
5
\$\begingroup\$

Very interesting approach to create a pattern for reverse engineering. I remember a similar plot (3D actually) done to the timespan between two heartbeats. The normal plot looked like a cigar-shaped cloud in the diagonal of the three axis', various heart problems could be seen as different patterns in that cloud.

Anyhow, let's look at the code. I won't be going into what others already said, you received some excellent feedback there already. Generally, the code is well readable, clearly structured and consistently formatted. Thumbs up for that.

Performance

When processing a file, you toggle between CPU bound and IO bound. First, your CPU sits there, waiting for a chunk of data to be read. Then your IO hardware sits there, while the chunk is being processed. This means that any time, at least one of the two resources IO and CPU is idle. If you use one thread to read the data and one thread to process it, you should be able to saturate one of the two resources, CPU or IO, which would improve performance.

On a similar note, it could be that using multiple threads to process the data would improve performance even more. You could increase the number of threads up to the point that either the CPU or the IO of your machine is maxed out.

Lastly, increase the CHUNK_SIZE. At the moment, it's 8kiB, I'd go to 64kiB if not even higher. The point here is, that each read operation (no matter the size) has a fixed overhead, in addition to the time it takes to retrieve the data. If you fragment the reads too much, that overhead dwarfes the actually usefully spent time.

Buffer Overflow

I'd say that

chunk[rcount] = '\0';

Will cause a buffer overflow when rcount hits the CHUNK_SIZE limit. Since you're not treating the data as string anyway, there is no need for a terminating NUL.

Refactor read_next_chunk()

First oddity is that it returns something that looks valid if the read operation fails. I have only seen this fail due to EOF regularly (which is not really a failure). The other cases are a broken harddisk, network failure for remote files or an unplugged thumbdrive. All these other cases are hard errors and I'd just output an error message and exit(FAILURE), like you do in xfopen(). Yes, the opened file is leaked, but since it's terminating the program, I wouldn't worry about that: The OS will take care of it. In other languages, I'd throw an exception, but in this isolated code it's fine to terminate.

Then, why doesn't this take a uint8_t buffer[static CHUNK_SIZE]? It would render all the casts outside of that function obsolete, you would only need a single one inside this function.

Further, the last parameter seems to be an optional pointer. However, you don't know how much data is received if you don't provide it, which is also why this parameter is never NULL. Consequently, I would expect it to be size_t [static 1].

Lastly, just return the number of bytes read or zero on EOF. The signature then becomes

static size_t read_next_chunk(FILE stream[static 1], uint8_t chunk[static CHUNK_SIZE])

This makes it less complicated in use, I believe.

Overlaps Between Chunks

I think you don't process the pair of octets formed by the last octet of one chunk and the first of the next. This introduces a small, probably negligible error. The error becomes less significant the larger the chunks you read (see previous point on that). If you ignore this, I'd document that decision in the code though.

Treat uint8_t Pair As uint16_t

In order to jump to the right cell in a 2D array, you need two times the operations as for in a 1D array. Maybe the compiler figures out that it can combine the two jumps, maybe it doesn't. This might affect the performance positively while making the code a bit less nice to read.

Various Things

  • No need to reset errno before opening a file. If it fails, it will be set accordingly anyway, if it succeeds it doesn't matter.
  • You could use memory mapping with a sliding window. It ensures that buffers are optimally aligned to create the least overhead while reading. I'm not sure if it would make much difference though.
  • Make sure you test this with an empty file as a cornercase. Currently, it works. However, if you compute max outside of the file-processing loop, it will be zero then.
\$\endgroup\$
2
  • 1
    \$\begingroup\$ "No need to reset errno before opening a file. If it fails, it will be set accordingly anyway, if it succeeds it doesn't matter." ==> The C standard does not require an implementation to set errno if fopen() fails. If it wasn't set to 0 prior to calling fopen(), and fopen() failed without setting errno, and errno was nonzero before the fopen() call, the diagnostic information would be incorrect. Aside from that, great answer. You raised some unique points. \$\endgroup\$
    – Harith
    Commented Mar 13 at 9:45
  • \$\begingroup\$ en.cppreference.com/w/c/io/fopen -- indeed, it doesn't, though POSIX does according to that reference. I'm so Linux-centered that this slipped under my radar, thanks for the clarification. \$\endgroup\$
    – uli
    Commented Mar 13 at 9:54
4
\$\begingroup\$

This ticks a lot of the boxes for good C, at least in usage.

read_next_chunk returning a char * is not helpful for two reasons. First, it contains no more information than if it were to just return a bool; and second, it encourages the assignment-in-predicate form of

while ((p = read_next_chunk(stream, content, &nbytes)))

that you should avoid.

Move your max assignment to

        if (map[y][x] > 0) {
            f = log(map[y][x]);

            if (f > max) {
                max = f;
            }
        }

since that's the only place where f will change.

You should always shift by unsigned literals (i.e. << 8u); see INT13-C. Use bitwise operators only on unsigned operands.

Despite the fact that _Bool is part of the C99 standard, it looks worse than bool, and the latter is offered by <stdbool.h>. Using bool will also smooth compatibility with C++.

occured is spelled occurred.

Never sprintf without specifying a buffer size. snprintf is included in C99.

\$\endgroup\$
1
  • 2
    \$\begingroup\$ Come on, even those overly-strict rules say: "If the right-side operand to a shift operator is known at compile time, it is acceptable for the value to be represented with a signed type provided it is positive." \$\endgroup\$
    – user555045
    Commented Mar 9 at 23:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.