[PATCH] optimize explicit_bzero for size

[PATCH] optimize explicit_bzero for size

[Alexander Monakov]

Avoid saving/restoring the incoming argument by reusing memset return value.
---

I think it's unfortunate that the commit adding explicit_bzero does not say
the rationale for the magic empty asm; LTO being the "obvious" explanation,
of course, IMHO is not a reason to omit the explanation. Does it imply an
intention to support LTO, and if so, would other magic asms elsewhere be
accepted if they help with LTO issues?

Alexander

 src/string/explicit_bzero.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/string/explicit_bzero.c b/src/string/explicit_bzero.c
index 3d270040..f2e12f23 100644
--- a/src/string/explicit_bzero.c
+++ b/src/string/explicit_bzero.c
@@ -3,6 +3,6 @@
 
 void explicit_bzero(void *d, size_t n)
 {
-	memset(d, 0, n);
+	d = memset(d, 0, n);
 	__asm__ __volatile__ ("" : : "r"(d) : "memory");
 }
-- 
2.11.0

Re: [PATCH] optimize explicit_bzero for size

[Rich Felker]

On Thu, Jun 28, 2018 at 08:57:29PM +0300, Alexander Monakov wrote:
> Avoid saving/restoring the incoming argument by reusing memset return value.
> ---
> 
> I think it's unfortunate that the commit adding explicit_bzero does not say
> the rationale for the magic empty asm; LTO being the "obvious" explanation,
> of course, IMHO is not a reason to omit the explanation.

LTO is the only plausible "mechanical" reason I know of, but formally
it's just about producing a dependency on the stores.

> Does it imply an
> intention to support LTO,

Yes, that's always been the intention. Last I checked there was a
linker-side bug whereby LTO broke crt1.c (due to failure to consider
the reference from the file-scope asm) which we might want to
workaround by forcibly disabling LTO for startfiles, but the intent is
that everything in musl be correct without relying on extern calls as
some sort of magic barriers.

> and if so, would other magic asms elsewhere be
> accepted if they help with LTO issues?

Ideally no -- explicit_bzero is special because its whole purpose is
to do something that doesn't really make sense in the abstract model
of the language, but that's nonetheless desired for real-world
hardening. I'm not aware of anything else like that. If there are
places where there's a real problem caused by "lack of barriers" with
LTO, we should first try to fix it in a way that's correct with regard
to the abstract model, I think.

>  src/string/explicit_bzero.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/string/explicit_bzero.c b/src/string/explicit_bzero.c
> index 3d270040..f2e12f23 100644
> --- a/src/string/explicit_bzero.c
> +++ b/src/string/explicit_bzero.c
> @@ -3,6 +3,6 @@
>  
>  void explicit_bzero(void *d, size_t n)
>  {
> -	memset(d, 0, n);
> +	d = memset(d, 0, n);
>  	__asm__ __volatile__ ("" : : "r"(d) : "memory");
>  }
> -- 
> 2.11.0

Or if you like it:

-	memset(d, 0, n);
-	__asm__ __volatile__ ("" : : "r"(d) : "memory");
+	__asm__ __volatile__ ("" : : "r"(memset(d, 0, n)) : "memory");

Not sure if this is nice or hideous... ;-)

Rich

Re: [PATCH] optimize explicit_bzero for size

[David CARLIER]

On Thu, 28 Jun 2018 at 20:36, Rich Felker <dalias@libc.org> wrote:
>
> On Thu, Jun 28, 2018 at 08:57:29PM +0300, Alexander Monakov wrote:
> > Avoid saving/restoring the incoming argument by reusing memset return value.
> > ---
> >


Why not even though I m wondering if this saving/restoring really
occurs/instruction really generated with last compilers.
Prefer Alexander's version otherwise :-)

Re: [PATCH] optimize explicit_bzero for size

[Rich Felker]

On Thu, Jun 28, 2018 at 08:42:13PM +0100, David CARLIER wrote:
> On Thu, 28 Jun 2018 at 20:36, Rich Felker <dalias@libc.org> wrote:
> >
> > On Thu, Jun 28, 2018 at 08:57:29PM +0300, Alexander Monakov wrote:
> > > Avoid saving/restoring the incoming argument by reusing memset return value.
> > > ---
> 
> Why not even though I m wondering if this saving/restoring really
> occurs/instruction really generated with last compilers.
> Prefer Alexander's version otherwise :-)

It necessarily has to because of ABI. Any register you could use to
save the value before making an external call is a register whose old
value you would have to save before using it, so no matter what you
do, something has to get spilled to the stack and thus a stack frame
has to be created.

Rich