[PATCH] memset 0 in obscure is optimized away by compiler

[Cathey, Jim]

Generally C compilers assume that an address
passed to a function (free(p)) _might_ be dereferenced,
and so would not optimize away the memset(p,...)

If a particular C compiler thinks it knows what
free() is, and that it doesn't dereference its argument,
then it might think it was justified in doing this.

That's a broken C compiler if you ask me, unless
you _can_ tell it not to make assumptions about what
particularly-named functions do.  (Else cross-compiling,
or developing in/for peculiar environments is impossible.)

A C compiler that _knows_ free() doesn't dereference its
argument _is_ entirely justified in eliminating the
memset.  The tool to tell the C compiler that there
are other factors operating behind the C compiler's
back is "volatile", which basically tells it "you don't
know everything here, so stop screwing around and do exactly
what I say."  It's an overlarge hammer in this case, though.

-- Jim

-----Original Message-----
From: busybox-bounces at busybox.net [mailto:busybox-bounces at busybox.net] On Behalf Of Denys Vlasenko
Sent: Wednesday, April 16, 2014 10:52 AM
To: Tito
Cc: busybox
Subject: Re: [PATCH] memset 0 in obscure is optimized away by compiler

On Wed, Apr 16, 2014 at 6:47 PM, Tito <farmatito at tiscali.it> wrote:
> Hi,
> while reading some interesting stuff about memset being optimized
> away by compilers if the variable is not read after the memset call
> I recalled there was something similar in libbb/obscure.c file:
>
> static int string_checker(const char *p1, const char *p2)
> {
>         int size, i;
>         /* check string */
>         int ret = string_checker_helper(p1, p2);
>         /* make our own copy */
>         char *p = xstrdup(p1);
>
>         /* reverse string */
>         i = size = strlen(p1);
>         while (--i >= 0) {
>                 *p++ = p1[i];
>         }
>         p -= size; /* restore pointer */
>
>         /* check reversed string */
>         ret |= string_checker_helper(p, p2);
>
>         /* clean up */
>         memset(p, 0, size);
>
>         free(p);
>
>         return ret;
> }

$ make libbb/obscure.s

and I see the memset (inlined by compiler):

        cld
        xorl    %eax, %eax      # tmp76
        movl    %ebx, %edi      # p.101,
        movl    %esi, %ecx      # D.7349, D.7349
        rep
        stosb

Perhaps because compiler doesn't know that free(p) doesn't use
the contents of *p.

>
> -       /* clean up */
> -       memset(p, 0, size);
> +       /* clean up, don't use memset as it is optimized away by compiler */
> +       /*memset(p, 0, size);*/
> +       nuke_str(p);
>         free(p);

This may be unnecessary wrt correctness (memset isn't eliminated),
but it is also _fewer bytes of code_!

Applied to git, thanks!
_______________________________________________
busybox mailing list
busybox at busybox.net
http://lists.busybox.net/mailman/listinfo/busybox