threads / patch / 26745

patchvalgrind: ignore SSE-based strlen invalid reads

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

## tl;dr

11 messages between Mar 16, 2011 and Mar 16, 2011. Diffs are folded; open one to read it.

replies: 10people: 4as markdown or json

Carlos Martín Nieto· Mar 16, 2011, 09:31 UTC · lore

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(-)
Show changes to t/valgrind/default.supp +6 −0
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
Jonathan Nieder· Mar 16, 2011, 09:56 UTC · re: Carlos Martín Nieto · lore

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

Hi Carlos,
Carlos Martín Nieto wrote:
Show 5 quoted lines
> 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

Carlos Martín Nieto· Mar 16, 2011, 10:41 UTC · re: Jonathan Nieder · lore

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

On mié, 2011-03-16 at 04:56 -0500, Jonathan Nieder wrote:
Show 12 quoted lines
> 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
Carlos Martín Nieto· Mar 16, 2011, 10:47 UTC · re: Jonathan Nieder · lore

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(-)
Show changes to t/valgrind/default.supp +6 −0
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
Jonathan Nieder· Mar 16, 2011, 10:52 UTC · re: Carlos Martín Nieto · lore

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

Carlos Martín Nieto wrote:
Show 5 quoted lines
> 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

Carlos Martín Nieto· Mar 16, 2011, 11:10 UTC · re: Jonathan Nieder · lore

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

On mié, 2011-03-16 at 05:52 -0500, Jonathan Nieder wrote:
Show 13 quoted lines
> 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.
Show 6 quoted lines
> 
> > 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
Jonathan Nieder· Mar 16, 2011, 11:25 UTC · re: Carlos Martín Nieto · lore

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

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

Carlos Martín Nieto· Mar 16, 2011, 11:46 UTC · re: Jonathan Nieder · lore

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>
---
Show 5 quoted lines
>>  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(-)
Show changes to t/valgrind/default.supp +6 −0
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
Junio C Hamano· Mar 16, 2011, 20:18 UTC · re: Carlos Martín Nieto · lore

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

Carlos Martín Nieto <cmn@elego.de> writes:
Show 24 quoted lines
> 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.
Carlos Martín Nieto· Mar 16, 2011, 20:44 UTC · re: Junio C Hamano · lore

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

On mié, 2011-03-16 at 13:18 -0700, Junio C Hamano wrote:
Show 29 quoted lines
> 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 ;)
Show 6 quoted lines
> 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 :)
Andreas Schwab· Mar 16, 2011, 22:43 UTC · re: Carlos Martín Nieto · lore

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

Carlos Martín Nieto <cmn@elego.de> writes:
Show 6 quoted lines
>  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."

← back to recent threads