This check is analogous to md32_common.h, L135.

Actually, looking at the implemenation of sha3_update(), I can't see any undefined behaviour even in the absence of this check, since memcpy(dest, src, 0) is well behaved and a no-op, provided dest and src are valid pointers. What's your opinion @dot-asm?

I'm sad. Because it's still a non-problem. Indeed, what does language standard say about memcpy? "The memcpy function copies n characters from the object pointed to by s2 into the object pointed to by s1. If copying takes place between objects that overlap, the behavior is undefined." Full stop. This means that passing zero length is in fact a no-op(*). Moreover, what does sanitizer documentation say? "While violating nullability does not have undefined behavior, it is often unintentional, so UBSan offers to catch it." Again, "does not"! This is just sad. It's sad to become a hostage of somebody else's idea about how bad programmers are. I mean it's offered with rationale that people would appreciate it, right? But program would crash anyway, so what [real] value does it actually add? ... Anyway, this means that whole premise that it's about undefined behaviour is essentially wrong. It's not about undefined behavour, but about sanitizer being overzealous. And with this in mind wouldn't it be more appropriate to just pass empty string as argument to first test in curve448_internal_test.c?

(*) One might be able to argue that memcpy(NULL,NULL,0) would be undefined, because NULL overlaps NULL. Even if one can, it's not case here, only one argument is NULL.

One might be able to argue that memcpy(NULL,NULL,0) would be undefined, because NULL overlaps NULL.

I object! If one thing is for sure then the fact that two empty ranges never overlap regardless of what src and dest pointers are, because the intersection of two empty sets is always empty ;-)

But seriously: I almost anticipated that you would dislike this check, that's why I asked you for your opinion. I wasn't worrying about undefined behaviour because of the overlap, but because of the invalid memory references. My understanding was - admittedly taken from second-hand sources like this one - that the no-op guarantee was given only in the case where src and dest point to valid memory:

Yes, the C standard explicitly addresses this in §7.24.1/2, which applies to memcpy and all other functions from string.h

Where an argument declared as size_t n specifies the length of the array for a function, n can have the value zero on a call to that function. Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. On such a call, a function that locates a character finds no occurrence, a function that compares two character sequences returns zero, and a function that copies characters copies zero characters

It seems however that they are referring to a newer version of the standard and that ISO C90 is not mentioning the fact of invalid memory references at all, as you already wrote, citing section 7.11.2.1 The memcpy function of the standard.

So what do you suggest? Revert only Matt's commit and don't add the extra check to m_sha3.c? Then how do we deal with this specific ubsan warning? Can it be disabled selectively? Wouldn't it be better to just keep the check in order to reassure all worried programmers and stop the need to have endless discussions?

I almost anticipated that you would dislike this check,

It's rather that I dislike things that are done for wrong reason. But of course, if memcpy(0,0,0) is legitimately(*) undefined, then among all suggested solutions, the one in this request is most appropriate. At least you can observe the check and potentially problematic call on same page [nor does it affect general-purpose proxy interface]. For completeness sake it's possible to disable it selectively with

#ifdef __clang__
__attribute__((no_sanitize("undefined")))
#endif
static int sha3_update(...

But I'd say it would be too much compiler specifics. I mean code preferably should have minimum possible compiler dependencies.

(*) It should be noted that standard can have bugs too ;-)