{"thread":{"id":"26745","subject":"[PATCH] valgrind: ignore SSE-based strlen invalid reads","startedAt":"2011-03-16T09:31:14Z","lastAt":"2011-03-16T22:43:07Z","messageCount":11,"participants":["Carlos Martín Nieto","Jonathan Nieder","Junio C Hamano","Andreas Schwab"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"163444","messageId":"1300267874-15365-1-git-send-email-cmn@elego.de","threadId":"26745","inReplyTo":null,"subject":"[PATCH] valgrind: ignore SSE-based strlen invalid reads","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-03-16T09:31:14Z","receivedAt":"2011-03-16T09:31:14Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"The C library uses SSE instructions to make strlen (among others)\nfaster, loading 4 bytes at a time and reading past the end of the\nallocated memory. This read is safe and when the strlen function is\ninlined, it is (obviously) not replaced by valgrind, which reports a\nfalse-possitive.\n\nTell valgrind to ignore this particular error, as the read is, in\nfact, safe.\n\nSigned-off-by: Carlos Martín Nieto \n---\n t/valgrind/default.supp | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/t/valgrind/default.supp b/t/valgrind/default.supp\nindex 9e013fa..327478c 100644\n--- a/t/valgrind/default.supp\n+++ b/t/valgrind/default.supp\n@@ -43,3 +43,9 @@\n \tfun:write_buffer\n \tfun:write_loose_object\n }\n+\n+{\n+\tignore-sse-strlen-invalid-read-size\n+\tMemcheck:Addr4\n+\tfun:copy_ref\n+}\n\\ No newline at end of file\n-- \n1.7.4.1\n"},{"id":"163449","messageId":"20110316095632.GA8277@elie","threadId":"26745","inReplyTo":"1300267874-15365-1-git-send-email-cmn@elego.de","subject":"Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T09:56:32Z","receivedAt":"2011-03-16T09:56:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Carlos,\n\nCarlos Martín Nieto wrote:\n\n> The C library uses SSE instructions to make strlen (among others)\n> faster, loading 4 bytes at a time and reading past the end of the\n> allocated memory. This read is safe and when the strlen function is\n> inlined, it is (obviously) not replaced by valgrind, which reports a\n> false-possitive.\n\nIt would be GCC rather than the C library if the strlen is inlined, I\nthink. Is this a distinct bug from\n? Has it been filed\nwith the valgrind maintainers?\n\nThanks,\nJonathan\n"},{"id":"163451","messageId":"1300272098.7214.5.camel@bee.lab.cmartin.tk","threadId":"26745","inReplyTo":"20110316095632.GA8277@elie","subject":"Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-03-16T10:41:33Z","receivedAt":"2011-03-16T10:41:33Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On mié, 2011-03-16 at 04:56 -0500, Jonathan Nieder wrote:\n> Hi Carlos,\n> \n> Carlos Martín Nieto wrote:\n> \n> > The C library uses SSE instructions to make strlen (among others)\n> > faster, loading 4 bytes at a time and reading past the end of the\n> > allocated memory. This read is safe and when the strlen function is\n> > inlined, it is (obviously) not replaced by valgrind, which reports a\n> > false-possitive.\n> \n> It would be GCC rather than the C library if the strlen is inlined, I\n> think. Is this a distinct bug from\n\n The strlen definition comes from the C library, as far as I know, but\nI'll amend to say it's the GNU C Library that's doing weird things.\n\n> ? Has it been filed\n> with the valgrind maintainers?\n\n It looks like the same issue, which should also be\nhttps://bugzilla.redhat.com/show_bug.cgi?id=518247 and\nhttp://bugs.debian.org/cgi-bin/bugreport.cgi?bug=590640 which have\npatches available. Newer versions of valgrind do not have this \"bug\".\n\n cmn\n"},{"id":"163452","messageId":"1300272453-25891-1-git-send-email-cmn@elego.de","threadId":"26745","inReplyTo":"20110316095632.GA8277@elie","subject":"[PATCH] valgrind: ignore SSE-based strlen invalid reads","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-03-16T10:47:33Z","receivedAt":"2011-03-16T10:47:33Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"The GNU C Library (glibc) uses SSE instructions to make strlen (among\nothers) faster, loading 4 bytes at a time and reading past the end of\nthe allocated memory. This read is safe and when the strlen function\nis inlined, it is (obviously) not replaced by valgrind, which reports\na false-possitive.\n\nTell valgrind to ignore this particular error, as the read is, in\nfact, safe.\n\nSigned-off-by: Carlos Martín Nieto \n---\n t/valgrind/default.supp | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/t/valgrind/default.supp b/t/valgrind/default.supp\nindex 9e013fa..327478c 100644\n--- a/t/valgrind/default.supp\n+++ b/t/valgrind/default.supp\n@@ -43,3 +43,9 @@\n \tfun:write_buffer\n \tfun:write_loose_object\n }\n+\n+{\n+\tignore-sse-strlen-invalid-read-size\n+\tMemcheck:Addr4\n+\tfun:copy_ref\n+}\n\\ No newline at end of file\n-- \n1.7.4.1\n"},{"id":"163453","messageId":"20110316105214.GB8277@elie","threadId":"26745","inReplyTo":"1300272453-25891-1-git-send-email-cmn@elego.de","subject":"Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T10:52:14Z","receivedAt":"2011-03-16T10:52:14Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Carlos Martín Nieto wrote:\n\n> The GNU C Library (glibc) uses SSE instructions to make strlen (among\n> others) faster, loading 4 bytes at a time and reading past the end of\n> the allocated memory. This read is safe and when the strlen function\n> is inlined, it is (obviously) not replaced by valgrind, which reports\n> a false-possitive.\n\nThis still makes no sense to me. How is it possible to inline a\nfunction from glibc? When I look in /usr/include/string.h, I see\n\nextern size_t strlen (__const char *__s)\n __THROW __attribute_pure__ __nonnull ((1));\n\n> Tell valgrind to ignore this particular error, as the read is, in\n> fact, safe.\n\nI'm happy to see a workaround. I would be even happier if it came\nwith documentation about which versions of valgrind need it.\n\nThanks again.\nJonathan\n"},{"id":"163455","messageId":"1300273819.7214.12.camel@bee.lab.cmartin.tk","threadId":"26745","inReplyTo":"20110316105214.GB8277@elie","subject":"Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-03-16T11:10:14Z","receivedAt":"2011-03-16T11:10:14Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On mié, 2011-03-16 at 05:52 -0500, Jonathan Nieder wrote:\n> Carlos Martín Nieto wrote:\n> \n> > The GNU C Library (glibc) uses SSE instructions to make strlen (among\n> > others) faster, loading 4 bytes at a time and reading past the end of\n> > the allocated memory. This read is safe and when the strlen function\n> > is inlined, it is (obviously) not replaced by valgrind, which reports\n> > a false-possitive.\n> \n> This still makes no sense to me. How is it possible to inline a\n> function from glibc? When I look in /usr/include/string.h, I see\n> \n> extern size_t strlen (__const char *__s)\n> __THROW __attribute_pure__ __nonnull ((1));\n\n Looking at that header, strlen is one of the few functions not being\nreplaced by its __builtin version, and I only see __builtin_strlen in\nthe C++ patches. I'll rephrase to something like \"Some versions of\nstrlen use SSE which then get inlined\" to avoid blaming anyone in\nparticular, though thinking about it, it does seem logical that it's\nGCC's builtin strlen.\n\n> \n> > Tell valgrind to ignore this particular error, as the read is, in\n> > fact, safe.\n> \n> I'm happy to see a workaround. I would be even happier if it came\n> with documentation about which versions of valgrind need it.\n\n I think 3.6.1 doesn't need it, as Debian's 1:3.5.0+3.6.0svn20100609-1\nversion is reportedly fixed. I'll see if I can find more information in\nthe valgrind bug tracker.\n\n Cheers,\n cmn\n"},{"id":"163457","messageId":"20110316112547.GA15739@elie","threadId":"26745","inReplyTo":"1300273819.7214.12.camel@bee.lab.cmartin.tk","subject":"Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-16T11:25:47Z","receivedAt":"2011-03-16T11:25:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Carlos Martín Nieto wrote:\n\n> I think 3.6.1 doesn't need it, as Debian's 1:3.5.0+3.6.0svn20100609-1\n> version is reportedly fixed.\n\nAh, nice. A phrase like \"some versions of valgrind before 3.6.1\"\nwould be fine with me fwiw. :)\n\nSorry for the fuss.\nJonathan\n"},{"id":"163466","messageId":"1300275961-5798-1-git-send-email-cmn@elego.de","threadId":"26745","inReplyTo":"20110316112547.GA15739@elie","subject":"[PATCH] valgrind: ignore SSE-based strlen invalid reads","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-03-16T11:46:01Z","receivedAt":"2011-03-16T11:46:01Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Some versions of strlen use SSE to speed up the calculation and load 4\nbytes at a time, even if it means reading past the end of the\nallocated memory. This read is safe and when the strlen function is\ninlined, it is not replaced by valgrind, which reports a\nfalse-possitive.\n\nTell valgrind to ignore this particular error, as the read is, in\nfact, safe. Current upstream-released version 2.6.1 is affected. Some\ndistributions have this fixed in their latest versions.\n\nSigned-off-by: Carlos Martín Nieto \n---\n\n>> I think 3.6.1 doesn't need it, as Debian's 1:3.5.0+3.6.0svn20100609-1\n>> version is reportedly fixed.\n>\n>Ah, nice. A phrase like \"some versions of valgrind before 3.6.1\"\n>would be fine with me fwiw. :)\n\nI just downloaded and compiled the upstream release 2.6.1 and it's\nstill affected (it does fix some other related\nfalse-positives). Fedorea rawhide has the fix in, according to their\nbug tracker. I haven't tested the reportedly-fixed version in Debian\nyet.\n\n t/valgrind/default.supp | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/t/valgrind/default.supp b/t/valgrind/default.supp\nindex 9e013fa..327478c 100644\n--- a/t/valgrind/default.supp\n+++ b/t/valgrind/default.supp\n@@ -43,3 +43,9 @@\n \tfun:write_buffer\n \tfun:write_loose_object\n }\n+\n+{\n+\tignore-sse-strlen-invalid-read-size\n+\tMemcheck:Addr4\n+\tfun:copy_ref\n+}\n\\ No newline at end of file\n-- \n1.7.4.1\n"},{"id":"163505","messageId":"7vr5a67rb7.fsf@alter.siamese.dyndns.org","threadId":"26745","inReplyTo":"1300275961-5798-1-git-send-email-cmn@elego.de","subject":"Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-16T20:18:04Z","receivedAt":"2011-03-16T20:18:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Martín Nieto writes:\n\n> Some versions of strlen use SSE to speed up the calculation and load 4\n> bytes at a time, even if it means reading past the end of the\n> allocated memory. This read is safe and when the strlen function is\n> inlined, it is not replaced by valgrind, which reports a\n> false-possitive.\n>\n> Tell valgrind to ignore this particular error, as the read is, in\n> fact, safe. Current upstream-released version 2.6.1 is affected. Some\n> distributions have this fixed in their latest versions.\n>\n> Signed-off-by: Carlos Martín Nieto \n> ---\n>\n>>> I think 3.6.1 doesn't need it, as Debian's 1:3.5.0+3.6.0svn20100609-1\n>>> version is reportedly fixed.\n>>\n>>Ah, nice. A phrase like \"some versions of valgrind before 3.6.1\"\n>>would be fine with me fwiw. :)\n>\n> I just downloaded and compiled the upstream release 2.6.1 and it's\n> still affected (it does fix some other related\n> false-positives). Fedorea rawhide has the fix in, according to their\n> bug tracker. I haven't tested the reportedly-fixed version in Debian\n> yet.\n\nI take it that you meant 3.6.1 in both places above?\n\nThis somehow reminds me of my past life where I saw a buggy implementation\nof strlen() in the C library loaded one word too many from memory, and\nsegfaulted even when the string ended before the end of a mapped page when\nthe next page was unmapped.\n\nAnyway, nice digging.\n"},{"id":"163508","messageId":"1300308252.7214.62.camel@bee.lab.cmartin.tk","threadId":"26745","inReplyTo":"7vr5a67rb7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-03-16T20:44:11Z","receivedAt":"2011-03-16T20:44:11Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On mié, 2011-03-16 at 13:18 -0700, Junio C Hamano wrote:\n> Carlos Martín Nieto writes:\n> \n> > Some versions of strlen use SSE to speed up the calculation and load 4\n> > bytes at a time, even if it means reading past the end of the\n> > allocated memory. This read is safe and when the strlen function is\n> > inlined, it is not replaced by valgrind, which reports a\n> > false-possitive.\n> >\n> > Tell valgrind to ignore this particular error, as the read is, in\n> > fact, safe. Current upstream-released version 2.6.1 is affected. Some\n> > distributions have this fixed in their latest versions.\n> >\n> > Signed-off-by: Carlos Martín Nieto \n> > ---\n> >\n> >>> I think 3.6.1 doesn't need it, as Debian's 1:3.5.0+3.6.0svn20100609-1\n> >>> version is reportedly fixed.\n> >>\n> >>Ah, nice. A phrase like \"some versions of valgrind before 3.6.1\"\n> >>would be fine with me fwiw. :)\n> >\n> > I just downloaded and compiled the upstream release 2.6.1 and it's\n> > still affected (it does fix some other related\n> > false-positives). Fedorea rawhide has the fix in, according to their\n> > bug tracker. I haven't tested the reportedly-fixed version in Debian\n> > yet.\n> \n> I take it that you meant 3.6.1 in both places above?\n> \n\n Yeah, I blame the kernel ;)\n\n> This somehow reminds me of my past life where I saw a buggy implementation\n> of strlen() in the C library loaded one word too many from memory, and\n> segfaulted even when the string ended before the end of a mapped page when\n> the next page was unmapped.\n> \n> Anyway, nice digging.\n\n If all else fails, google will point you to the bug tracker :)\n"},{"id":"163521","messageId":"m2ei66ofes.fsf@igel.home","threadId":"26745","inReplyTo":"1300273819.7214.12.camel@bee.lab.cmartin.tk","subject":"Re: [PATCH] valgrind: ignore SSE-based strlen invalid reads","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2011-03-16T22:43:07Z","receivedAt":"2011-03-16T22:43:07Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Carlos Martín Nieto writes:\n\n> Looking at that header, strlen is one of the few functions not being\n> replaced by its __builtin version, and I only see __builtin_strlen in\n> the C++ patches. I'll rephrase to something like \"Some versions of\n> strlen use SSE which then get inlined\" to avoid blaming anyone in\n> particular, though thinking about it, it does seem logical that it's\n> GCC's builtin strlen.\n\nAll processor optimized versions of strlen in glibc are out-of-line, so\nany use of SSE can only come from __builtin_strlen.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"}]}