llvm dev - Dec 2017 - Clang 5, UBsan, runtime error: addition of unsigned offset to X overflowed to Y

We have code that processes a buffer in the forward or backwards
direction. It looks similar to the following
(https://github.com/weidai11/cryptopp/blob/master/adv-simd.h#L1138):

uint8_t * ptr = ...
size_t len = ...
size_t inc = 16;

if (flags & REVERSE_DIRECTION)
{
    ptr += len - inc;
    inc = 0-inc;
}

while (len > 16)
{
    // process blocks

    ptr += inc;
    len -= 16;
}

Clang 5.0 and UBsan is producing findings
(https://travis-ci.org/noloader/cryptopp/jobs/317442299#L967):

adv-simd.h:1138:26: runtime error: addition of unsigned offset to
0x000003f78cf0 overflowed to 0x000003f78ce0
adv-simd.h:1140:26: runtime error: addition of unsigned offset to
0x000003f78ce0 overflowed to 0x000003f78cd0
adv-simd.h:1142:26: runtime error: addition of unsigned offset to
0x000003f78cd0 overflowed to 0x000003f78cc0
...

Lines 1138, 1140, 1142 (and friends) are the increment, which may
stride backwards:

    ptr += inc;

The overflow is troubling because signed integer overflow is undefined
behavior. The code above depends on unsigned wrap, and that's well
defined behavior. We don't know where the signed operations are coming
from.

Previous versions of Clang and GCC did not produce a finding.

My questions are, what is going on, and how do I fix it?

Jeff

It looks like the complaint is about overflow of the pointer itself, which
makes sense since you are adding a very large unsigned number to it. Maybe
use ptrdiff_t for inc?

I forget the rules for pointer overflow, but if pointer overflow is UB,
then this seems like a perfectly fine error.

-- Sean Silva

On Dec 16, 2017 12:18 PM, "Jeffrey Walton via llvm-dev" <
llvm-dev at lists.llvm.org> wrote:
> We have code that processes a buffer in the forward or backwards
> direction. It looks similar to the following
> (https://github.com/weidai11/cryptopp/blob/master/adv-simd.h#L1138):
>
> uint8_t * ptr = ...
> size_t len = ...
> size_t inc = 16;
>
> if (flags & REVERSE_DIRECTION)
> {
>     ptr += len - inc;
>     inc = 0-inc;
> }
>
> while (len > 16)
> {
>     // process blocks
>
>     ptr += inc;
>     len -= 16;
> }
>
> Clang 5.0 and UBsan is producing findings
> (https://travis-ci.org/noloader/cryptopp/jobs/317442299#L967):
>
> adv-simd.h:1138:26: runtime error: addition of unsigned offset to
> 0x000003f78cf0 overflowed to 0x000003f78ce0
> adv-simd.h:1140:26: runtime error: addition of unsigned offset to
> 0x000003f78ce0 overflowed to 0x000003f78cd0
> adv-simd.h:1142:26: runtime error: addition of unsigned offset to
> 0x000003f78cd0 overflowed to 0x000003f78cc0
> ...
>
> Lines 1138, 1140, 1142 (and friends) are the increment, which may
> stride backwards:
>
>     ptr += inc;
>
> The overflow is troubling because signed integer overflow is undefined
> behavior. The code above depends on unsigned wrap, and that's well
> defined behavior. We don't know where the signed operations are coming
> from.
>
> Previous versions of Clang and GCC did not produce a finding.
>
> My questions are, what is going on, and how do I fix it?
>
> Jeff
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171216/7f168722/attachment.html>

On Sat, Dec 16, 2017 at 4:53 PM, Sean Silva <chisophugis at gmail.com>
wrote:
> It looks like the complaint is about overflow of the pointer itself, which
> makes sense since you are adding a very large unsigned number to it. Maybe
> use ptrdiff_t for inc?
Thanks Sean.

It took a few tries, but that cleared it. All of the size_t's that
were being added to the pointer had to be swapped-out for ptrdiff_t's.

Jeff

> The overflow is troubling because signed integer overflow is undefined
> behavior. The code above depends on unsigned wrap, and that's well
> defined behavior. We don't know where the signed operations are coming
> from.
Unsigned integer overflow is defined, but pointers are not integers and 
pointer overflow is undefined.

John