Skip to content

Commit

Permalink
mx_util: Avoid "may be used uninitialized" warning
Browse files Browse the repository at this point in the history
The function

    int mx_strtoul(char *str, unsigned long int *to)

and three similar functions either returns with the success value 0 or
with a negative error code (-errno). The output argument `to` is only
written, when the return value signals success. If the function returns
with a negative value, the caller can not assument that `to` has been
set.

When a caller does something like this

    int value;
    res = int mx_strtoul(str, &value);
    if (res < 0)
        return;
    do_something_with(value)

everything is correct according to the description aboce.

The code in mx_strtoul has this pattern:

    errno = 0;
    ul = strtoul(str, &end, 0);
    if (errno) {
        return -errno;
    }
    *to = ul;

When the caller is in the same compilation unit, the compiler (gcc with
-O2 or higher and with -Wall) might raise a "may be used uninitialized"
warning for the output variable (`value` in the above example).

The problem here is, that the caller assumes a negative value for an
error status but the callee returns -errno for any errno value other
than zero which might include negative errno values. We know, that the
errno value will not be negative because we've set it to zero and the
library function will conditionally set it to a valid error number only,
which is alway positiv[1]. But the compiler doesn't share this
assumption and takes a negative errno value into account.

When the caller and the callee are in the same compilation unit, the
interprocedural optimizer might detect the code path with a negative
errno value which would in fact trigger a usage of an unitilialized
value.

We can fix that by just baking the assumption into the comparison:

    if (errno > 0)
        return -errno;

or by asserting that errno is not negative:

    if (errno) {
        assert(errno > 0);
        return -errno;
    }

or by defining and using an __assume macro:

    #if defined(__clang__)
    #define assume(cond) __builtin_assume(cond)
    #elif defined(__GNUC__)
    #define assume(cond) do { if (!(cond)) __builtin_unreachable(); } while (0)
    #endif

    ...

    if (errno) {
        assume(errno > 0);
        return -errno;
    }

Use the first option for all four affected functions.

Note, that clangs static analyzer is still not happy with this, but
this is probably a false positive of the analyzer [2].

[1]: errno(3): "Valid error numbers are all positive numbers.
[2]: https://github.com/llvm/llvm-project/issues/55241
  • Loading branch information
donald committed May 5, 2022
1 parent 65890b5 commit 88dd544
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions mx_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ int mx_strtoul(char *str, unsigned long int *to)

ul = strtoul(str, &end, 0);

if (errno)
if (errno > 0)
return -errno;

end = mx_strskipwhitespaces(end);
Expand Down Expand Up @@ -284,7 +284,7 @@ int mx_strtoull(char *str, unsigned long long int *to)

ull = strtoull(str, &end, 0);

if (errno)
if (errno > 0)
return -errno;

end = mx_strskipwhitespaces(end);
Expand Down Expand Up @@ -314,7 +314,7 @@ int mx_strtol(char *str, signed long int *to)

l = strtoul(str, &end, 0);

if (errno)
if (errno > 0)
return -errno;

end = mx_strskipwhitespaces(end);
Expand All @@ -339,7 +339,7 @@ int mx_strtoll(char *str, signed long long int *to)

ll = strtoll(str, &end, 0);

if (errno)
if (errno > 0)
return -errno;

end = mx_strskipwhitespaces(end);
Expand Down

0 comments on commit 88dd544

Please sign in to comment.