Incorrect implementation of calloc() introduces division by zero and how to detect it via testing?

Question

I had an assignment in which I had to write my own calloc function (as well as a few other Libc functions), the only built-in functions allowed were malloc() and free().

For context: the grading for my assignment is fully automated, some tests are run against my code on a server, and I get back either PASS or FAIL, if the code passed all tests or if it did not.
So what I'm trying to get at is I know my code didn't pass the requirements, but I don't know exactly why, it's just a guess.

Here is the code in question:

#include <stdlib.h>
#include "libft.h"    // this is where ft_bzero is declared

void    *ft_calloc(size_t nmemb, size_t size)
{
    void    *ptr;

    if (nmemb > (size_t)-1 / size && size)
        return (NULL);
    ptr = malloc(nmemb * size);
    if (!ptr)
        return (NULL);
    ft_bzero(ptr, nmemb * size);
    return (ptr);
}

I assume my mistake was that I mispositioned my conditional arguments and it should be like this instead:

if (size && nmemb > (size_t)-1 / size)

so that we check if size is zero before dividing, which introduces undefined behaviour.

If that is indeed the error and there is nothing else troublesome in my code, how can you even detect this by running some tests?

I've tried calling the function with size 0 and it didn't crash my program, I've also tried compiling with clang -fsanitize=integer-divide-by-zero and it didn't produce any errors at runtime.

I am aware that by definition undefined behaviour is undefined so there is not really a way to detect it but it seems like whatever test was running on the backend server did.

Any ideas?

Answer 1

-fsanitize=undefined catches the division by zero, with both gcc and clang, regardless of optimization level.

Try on godbolt. Note that with -O3 the division has been optimized into a multiplication, and no div instruction is performed so no hardware divide-by-zero trap is possible, but the test is done anyway.

This is probably a better idea than -fsanitize=integer-divide-by-zero anyway, as it will catch additional instances of UB of types that you may not have thought of.