Re: [PATCH v2] x86-64: Optimize bzero

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 10/02/2022 14:50, Alejandro Colomar (man-pages) wrote:
> Hi,
> 
> 
> On 2/10/22 14:16, Adhemerval Zanella via Libc-alpha wrote:
>> On 10/02/2022 10:10, Adhemerval Zanella wrote:
>>> On 10/02/2022 10:01, Wilco Dijkstra wrote:
>>>>>> Agreed it's not clear if it's worth it to start replacing memset
> calls with
>>>>>> bzero calls, but at the very least this will improve existing code
> that
>>>>>> uses bzero.
>>>>
>>>> No code uses bzero, no compiler emits bzero. It died 2 decades ago...
> 
> Sorry to ruin your day, but there's a bzero(3) user here :)
> 
> There are rational reasons to prefer bzero(3) due to it's better
> interface, and only one to prefer memset(3): standards people decided to
> remove bzero(3).  See <https://stackoverflow.com/a/17097978/6872717>.
> 
> Consider the following quote from "UNIX Network Programming" by Stevens
> et al., Section 1.2 (emphasis added):
>> `bzero` is not an ANSI C function. It is derived from early Berkely
>> networking code. Nevertheless, we use it throughout the text, instead
>> of the ANSI C `memset` function, because `bzero` is easier to remember
>> (with only two arguments) than `memset` (with three arguments). Almost
>> every vendor that supports the sockets API also provides `bzero`, and
>> if not, we provide a macro definition in our `unp.h` header.
>>
>> Indeed, **the author of TCPv3 [TCP/IP Illustrated, Volume 3 - Stevens
> 1996] made the mistake of swapping the second
>> and third arguments to `memset` in 10 occurrences in the first
>> printing**. A C compiler cannot catch this error because both arguments
>> are of the same type. (Actually, the second argument is an `int` and
>> the third argument is `size_t`, which is typically an `unsigned int`,
>> but the values specified, 0 and 16, respectively, are still acceptable
>> for the other type of argument.) The call to `memset` still worked,
>> because only a few of the socket functions actually require that the
>> final 8 bytes of an Internet socket address structure be set to 0.
>> Nevertheless, it was an error, and one that could be avoided by using
>> `bzero`, because swapping the two arguments to `bzero` will always be
>> caught by the C compiler if function prototypes are used.
> 
> I consistently use bzero(3), and dislike the interface of memset(3) for
> zeroing memory.  I checked how many memset(3) calls there are in a
> codebase of mine, and there is exactly 1 call to memset(3), for setting
> an array representing a large bitfield to 1s: memset(arr, UCHAR_MAX,
> sizeof(arr)).  And there are 41 calls to bzero(3).

The bzero won't go away since glibc will continue to support old POSIX 
interfaces, but Wilco has a point: it is an interface that has been 
deprecated for more than 2 decades and usually only BSD-like code
still keeps using it (explicit_bzero was initialized from openbsd).

> 
>>>>
>>>>> My point is this is a lot of code and infrastructure for a symbol
> marked
>>>>> as legacy for POSIX.1-2001 and removed on POSIX.1-2008 for the sake of
>>>>> marginal gains in specific cases.
>>>>
>>>> Indeed, what we really should discuss is how to remove the last
> traces of
>>>> bcopy and bcmp from GLIBC. Do we need to keep a compatibility symbol
>>>> or could we just get rid of it altogether?
> 
> I think it's sad that POSIX removed bzero(3).  In the end, people need
> to zero memory, and there's no simpler interface than bzero(3) for that.
>  memset(3) has a useless extra parameter.  Even if you just do a simple
> wrapper as the following, which is no big overhead for glibc I guess,
> you would be improving (or not worsening) my and hopefully others' lives:
> 
> static inline bzero(s, n)
> {
> 	memset(s, 0, n);
> }
> 
> Is that really a pain to maintain?

Yes, this is *exactly* what we are trying to remove and I will oppose to
reintroduce it after all the cleanups we did in this front (for instance
18b10de7ced9e9).

Besides your code most likely is calling memset under the hoods, gcc by 
default converts bzero calls to memset.  It seems to be really tricky to 
actually force gcc emit a bzero call to libc, I would could make by not 
including string.h, adding an explicit prototype to bzero, along with
-fno-builtin:

$ cat bzero.c 
#include <stddef.h>
extern void bzero (void *, size_t s);

int main (int argc, char *argv[])
{
  char b[1024];
  bzero (b, sizeof b);
  return 0;
}
$ gcc -Wall -fno-builtin bzero.c -o test && objdump -R test | grep -w bzero
0000000000003fd0 R_X86_64_JUMP_SLOT  bzero@GLIBC_2.2.5

> 
> If libc ever removes bzero(3), it's not like I'm going to start using
> memset(3).  I'll just define it myself.  That's not an improvement, but
> the opposite, IMO.
> 
> Ideally, POSIX would reconsider some day, and reincorporate bzero(3),
> but I don't expect that any soon :(.

I really think it is unlikely, but it is just my view.

> 
>>>
>>> We need to keep the symbols as-is afaiu, since callers might still target
>>> old POSIX where the symbol is defined as supported.  We might add either
>>> compiler or linker warning stating the symbols is deprecated, but imho it
>>> would just be better if we stop trying to microoptimize it and just use
>>> the generic interface (which call memset).
> 
> No please, I know they are deprecated, and explicitly want to use it.
> Don't need some extra spurious warning.
> 
> Other projects that I have touched (including nginx and shadow-utils),
> seem to have some similar opinion to me, and they define memzero() or
> some other similar name, with an interface identical to bzero(3) (just a
> different name to avoid problems), to wrap either bzero(3) or memset(3),
> depending on what is available.

We are discussing different subjects here: what I want is to remove the
glibc *internal* optimization for bzero, which is essentially an
implementation detail.  In a first glance it would change performance,
however gcc does a hard job replacing bzero/bcmp/bcopy with their
str* counterparts, so it highly unlike that newer binaries will actually
call bzero.

I did an experiment on my system and indeed there is no calls to bzero.

$ cat count_bzero.sh
#!/bin/bash

files=`IFS=':';for i in $PATH; do test -d "$i" && find "$i" -maxdepth 1 -executable -type f; done`
total=0
for file in $files; do
  bzero=`objdump -R $file 2>&1`
  if [ $? -eq 0 ]; then
    ncalls=`echo $bzero | grep -w bzero | wc -l`
    ((total=total+ncalls))
    if [ $ncalls -gt 0 ]; then
      echo "$file: $ncalls"
    fi
  fi
done
echo "TOTAL=$total"
$ ./count_bzero.sh
TOTAL=0