threads / patch / 46398

patcht: handle EOF in test_copy_bytes()

Subject: [PATCH] t: handle EOF in test_copy_bytes()

## tl;dr

4 messages between Jul 16, 2017 and Jul 16, 2017. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Jeff King· Jul 16, 2017, 10:45 UTC · lore

The test_copy_bytes() function claims to read up to N bytes, or until it gets EOF. But we never handle EOF in our loop, and a short input will cause perl to go into an infinite loop of read() getting zero bytes.

Signed-off-by: Jeff King <peff@peff.net>
---
I was playing with SANITIZE=undefined after René's patches to see how
far we had left to go. I forgot to turn off sha1dc, which causes most
programs to die due to the unaligned loads. That means git-archive in
t5000 generates no output, triggering the bug. :)

 t/test-lib-functions.sh | 1 +
 1 file changed, 1 insertion(+)

Show changes to t/test-lib-functions.sh +1 −0

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index db622c355..50a9a1d1c 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -999,6 +999,7 @@ test_copy_bytes () {
 			my $s;
 			my $nread = sysread(STDIN, $s, $len);
 			die "cannot read: $!" unless defined($nread);
+			last unless $nread;
 			print $s;
 			$len -= $nread;
 		}

-- 
2.14.0.rc0.452.gc10560834

Jeff King· Jul 16, 2017, 10:47 UTC · re: Jeff King · lore

Re: [PATCH] t: handle EOF in test_copy_bytes()

On Sun, Jul 16, 2017 at 06:45:32AM -0400, Jeff King wrote:

> I was playing with SANITIZE=undefined after René's patches to see how
> far we had left to go. I forgot to turn off sha1dc, which causes most
> programs to die due to the unaligned loads. That means git-archive in
> t5000 generates no output, triggering the bug. :)

And I was pleased to see that after setting OPENSSL_SHA1, the answer to "how far" is "we are there". Yay.

-Peff

René Scharfe· Jul 16, 2017, 11:10 UTC · re: Jeff King · lore

Re: [PATCH] t: handle EOF in test_copy_bytes()

Am 16.07.2017 um 12:47 schrieb Jeff King:

Show 9 quoted lines

> On Sun, Jul 16, 2017 at 06:45:32AM -0400, Jeff King wrote:
> 
>> I was playing with SANITIZE=undefined after René's patches to see how
>> far we had left to go. I forgot to turn off sha1dc, which causes most
>> programs to die due to the unaligned loads. That means git-archive in
>> t5000 generates no output, triggering the bug. :)
> 
> And I was pleased to see that after setting OPENSSL_SHA1, the answer to
> "how far" is "we are there". Yay.

True with GCC, but not with Clang 3.9. A patch for alignment issues in dir.c is coming up..

René

Jeff King· Jul 16, 2017, 11:21 UTC · re: René Scharfe · lore

Re: [PATCH] t: handle EOF in test_copy_bytes()

On Sun, Jul 16, 2017 at 01:10:50PM +0200, René Scharfe wrote:

Show 5 quoted lines

> > And I was pleased to see that after setting OPENSSL_SHA1, the answer to
> > "how far" is "we are there". Yay.
> 
> True with GCC, but not with Clang 3.9.  A patch for alignment issues in
> dir.c is coming up..

Good to know that there are differences. I used clang for my sanitize runs for the past few years (because of issues I had with gcc's version in the 4.8 era), but just switched a week or two ago back to using gcc.

My impression is that clang is the primary platform for sanitizer development, and so it would probably produce better results. We _could_ force "CC ?= clang" when SANITIZE is set, but that's probably a bit unfriendly to people who only have gcc.

FWIW, I just did an UBSan run with clang-5.0, and it comes up with only the dir.c problem.

-Peff