[PATCH] valgrind: ignore SSE-based strlen invalid reads

[PATCH] valgrind: ignore SSE-based strlen invalid reads

[Carlos Martín Nieto]

The C library uses SSE instructions to make strlen (among others)
faster, loading 4 bytes at a time and reading past the end of the
allocated memory. This read is safe and when the strlen function is
inlined, it is (obviously) not replaced by valgrind, which reports a
false-possitive.

Tell valgrind to ignore this particular error, as the read is, in
fact, safe.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 t/valgrind/default.supp |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
index 9e013fa..327478c 100644
--- a/t/valgrind/default.supp
+++ b/t/valgrind/default.supp
@@ -43,3 +43,9 @@
 	fun:write_buffer
 	fun:write_loose_object
 }
+
+{
+	ignore-sse-strlen-invalid-read-size
+	Memcheck:Addr4
+	fun:copy_ref
+}
\ No newline at end of file
-- 
1.7.4.1

Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads

[Jonathan Nieder]

Hi Carlos,

Carlos Martín Nieto wrote:

> The C library uses SSE instructions to make strlen (among others)
> faster, loading 4 bytes at a time and reading past the end of the
> allocated memory. This read is safe and when the strlen function is
> inlined, it is (obviously) not replaced by valgrind, which reports a
> false-possitive.

It would be GCC rather than the C library if the strlen is inlined, I
think.  Is this a distinct bug from
<http://bugs.kde.org/show_bug.cgi?id=266961>?  Has it been filed
with the valgrind maintainers?

Thanks,
Jonathan

Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads

[Carlos Martín Nieto]

On mié, 2011-03-16 at 04:56 -0500, Jonathan Nieder wrote:
> Hi Carlos,
> 
> Carlos Martín Nieto wrote:
> 
> > The C library uses SSE instructions to make strlen (among others)
> > faster, loading 4 bytes at a time and reading past the end of the
> > allocated memory. This read is safe and when the strlen function is
> > inlined, it is (obviously) not replaced by valgrind, which reports a
> > false-possitive.
> 
> It would be GCC rather than the C library if the strlen is inlined, I
> think.  Is this a distinct bug from

 The strlen definition comes from the C library, as far as I know, but
I'll amend to say it's the GNU C Library that's doing weird things.

> <http://bugs.kde.org/show_bug.cgi?id=266961>?  Has it been filed
> with the valgrind maintainers?

 It looks like the same issue, which should also be
https://bugzilla.redhat.com/show_bug.cgi?id=518247 and
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=590640 which have
patches available. Newer versions of valgrind do not have this "bug".

   cmn

[PATCH] valgrind: ignore SSE-based strlen invalid reads

[Carlos Martín Nieto]

The GNU C Library (glibc) uses SSE instructions to make strlen (among
others) faster, loading 4 bytes at a time and reading past the end of
the allocated memory. This read is safe and when the strlen function
is inlined, it is (obviously) not replaced by valgrind, which reports
a false-possitive.

Tell valgrind to ignore this particular error, as the read is, in
fact, safe.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 t/valgrind/default.supp |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
index 9e013fa..327478c 100644
--- a/t/valgrind/default.supp
+++ b/t/valgrind/default.supp
@@ -43,3 +43,9 @@
 	fun:write_buffer
 	fun:write_loose_object
 }
+
+{
+	ignore-sse-strlen-invalid-read-size
+	Memcheck:Addr4
+	fun:copy_ref
+}
\ No newline at end of file
-- 
1.7.4.1

Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads

[Jonathan Nieder]

Carlos Martín Nieto wrote:

> The GNU C Library (glibc) uses SSE instructions to make strlen (among
> others) faster, loading 4 bytes at a time and reading past the end of
> the allocated memory. This read is safe and when the strlen function
> is inlined, it is (obviously) not replaced by valgrind, which reports
> a false-possitive.

This still makes no sense to me.  How is it possible to inline a
function from glibc?  When I look in /usr/include/string.h, I see

extern size_t strlen (__const char *__s)
     __THROW __attribute_pure__ __nonnull ((1));

> Tell valgrind to ignore this particular error, as the read is, in
> fact, safe.

I'm happy to see a workaround.  I would be even happier if it came
with documentation about which versions of valgrind need it.

Thanks again.
Jonathan

Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads

[Carlos Martín Nieto]

On mié, 2011-03-16 at 05:52 -0500, Jonathan Nieder wrote:
> Carlos Martín Nieto wrote:
> 
> > The GNU C Library (glibc) uses SSE instructions to make strlen (among
> > others) faster, loading 4 bytes at a time and reading past the end of
> > the allocated memory. This read is safe and when the strlen function
> > is inlined, it is (obviously) not replaced by valgrind, which reports
> > a false-possitive.
> 
> This still makes no sense to me.  How is it possible to inline a
> function from glibc?  When I look in /usr/include/string.h, I see
> 
> extern size_t strlen (__const char *__s)
>      __THROW __attribute_pure__ __nonnull ((1));

 Looking at that header, strlen is one of the few functions not being
replaced by its __builtin version, and I only see __builtin_strlen in
the C++ patches. I'll rephrase to something like "Some versions of
strlen use SSE which then get inlined" to avoid blaming anyone in
particular, though thinking about it, it does seem logical that it's
GCC's builtin strlen.

> 
> > Tell valgrind to ignore this particular error, as the read is, in
> > fact, safe.
> 
> I'm happy to see a workaround.  I would be even happier if it came
> with documentation about which versions of valgrind need it.

 I think 3.6.1 doesn't need it, as Debian's 1:3.5.0+3.6.0svn20100609-1
version is reportedly fixed. I'll see if I can find more information in
the valgrind bug tracker.

 Cheers,
   cmn

Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads

[Jonathan Nieder]

Carlos Martín Nieto wrote:

>  I think 3.6.1 doesn't need it, as Debian's 1:3.5.0+3.6.0svn20100609-1
> version is reportedly fixed.

Ah, nice.  A phrase like "some versions of valgrind before 3.6.1"
would be fine with me fwiw. :)

Sorry for the fuss.
Jonathan

[PATCH] valgrind: ignore SSE-based strlen invalid reads

[Carlos Martín Nieto]

Some versions of strlen use SSE to speed up the calculation and load 4
bytes at a time, even if it means reading past the end of the
allocated memory. This read is safe and when the strlen function is
inlined, it is not replaced by valgrind, which reports a
false-possitive.

Tell valgrind to ignore this particular error, as the read is, in
fact, safe. Current upstream-released version 2.6.1 is affected. Some
distributions have this fixed in their latest versions.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---

>>  I think 3.6.1 doesn't need it, as Debian's 1:3.5.0+3.6.0svn20100609-1
>> version is reportedly fixed.
>
>Ah, nice.  A phrase like "some versions of valgrind before 3.6.1"
>would be fine with me fwiw. :)

I just downloaded and compiled the upstream release 2.6.1 and it's
still affected (it does fix some other related
false-positives). Fedorea rawhide has the fix in, according to their
bug tracker. I haven't tested the reportedly-fixed version in Debian
yet.

 t/valgrind/default.supp |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
index 9e013fa..327478c 100644
--- a/t/valgrind/default.supp
+++ b/t/valgrind/default.supp
@@ -43,3 +43,9 @@
 	fun:write_buffer
 	fun:write_loose_object
 }
+
+{
+	ignore-sse-strlen-invalid-read-size
+	Memcheck:Addr4
+	fun:copy_ref
+}
\ No newline at end of file
-- 
1.7.4.1

Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads

[Junio C Hamano]

Carlos Martín Nieto <cmn@elego.de> writes:

> Some versions of strlen use SSE to speed up the calculation and load 4
> bytes at a time, even if it means reading past the end of the
> allocated memory. This read is safe and when the strlen function is
> inlined, it is not replaced by valgrind, which reports a
> false-possitive.
>
> Tell valgrind to ignore this particular error, as the read is, in
> fact, safe. Current upstream-released version 2.6.1 is affected. Some
> distributions have this fixed in their latest versions.
>
> Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
> ---
>
>>>  I think 3.6.1 doesn't need it, as Debian's 1:3.5.0+3.6.0svn20100609-1
>>> version is reportedly fixed.
>>
>>Ah, nice.  A phrase like "some versions of valgrind before 3.6.1"
>>would be fine with me fwiw. :)
>
> I just downloaded and compiled the upstream release 2.6.1 and it's
> still affected (it does fix some other related
> false-positives). Fedorea rawhide has the fix in, according to their
> bug tracker. I haven't tested the reportedly-fixed version in Debian
> yet.

I take it that you meant 3.6.1 in both places above?

This somehow reminds me of my past life where I saw a buggy implementation
of strlen() in the C library loaded one word too many from memory, and
segfaulted even when the string ended before the end of a mapped page when
the next page was unmapped.

Anyway, nice digging.

Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads

[Carlos Martín Nieto]

On mié, 2011-03-16 at 13:18 -0700, Junio C Hamano wrote:
> Carlos Martín Nieto <cmn@elego.de> writes:
> 
> > Some versions of strlen use SSE to speed up the calculation and load 4
> > bytes at a time, even if it means reading past the end of the
> > allocated memory. This read is safe and when the strlen function is
> > inlined, it is not replaced by valgrind, which reports a
> > false-possitive.
> >
> > Tell valgrind to ignore this particular error, as the read is, in
> > fact, safe. Current upstream-released version 2.6.1 is affected. Some
> > distributions have this fixed in their latest versions.
> >
> > Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
> > ---
> >
> >>>  I think 3.6.1 doesn't need it, as Debian's 1:3.5.0+3.6.0svn20100609-1
> >>> version is reportedly fixed.
> >>
> >>Ah, nice.  A phrase like "some versions of valgrind before 3.6.1"
> >>would be fine with me fwiw. :)
> >
> > I just downloaded and compiled the upstream release 2.6.1 and it's
> > still affected (it does fix some other related
> > false-positives). Fedorea rawhide has the fix in, according to their
> > bug tracker. I haven't tested the reportedly-fixed version in Debian
> > yet.
> 
> I take it that you meant 3.6.1 in both places above?
> 

 Yeah, I blame the kernel ;)

> This somehow reminds me of my past life where I saw a buggy implementation
> of strlen() in the C library loaded one word too many from memory, and
> segfaulted even when the string ended before the end of a mapped page when
> the next page was unmapped.
> 
> Anyway, nice digging.

 If all else fails, google will point you to the bug tracker :)

Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads

[Andreas Schwab]

Carlos Martín Nieto <cmn@elego.de> writes:

>  Looking at that header, strlen is one of the few functions not being
> replaced by its __builtin version, and I only see __builtin_strlen in
> the C++ patches. I'll rephrase to something like "Some versions of
> strlen use SSE which then get inlined" to avoid blaming anyone in
> particular, though thinking about it, it does seem logical that it's
> GCC's builtin strlen.

All processor optimized versions of strlen in glibc are out-of-line, so
any use of SSE can only come from __builtin_strlen.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."