From patchwork Fri Sep 23 19:19:57 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Lance Taylor X-Patchwork-Id: 116172 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 04417B6F85 for ; Sat, 24 Sep 2011 05:20:43 +1000 (EST) Received: (qmail 16699 invoked by alias); 23 Sep 2011 19:20:30 -0000 Received: (qmail 16325 invoked by uid 22791); 23 Sep 2011 19:20:26 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RP_MATCHES_RCVD, SPF_HELO_PASS, T_TVD_MIME_NO_HEADERS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (74.125.121.67) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 23 Sep 2011 19:20:14 +0000 Received: from wpaz29.hot.corp.google.com (wpaz29.hot.corp.google.com [172.24.198.93]) by smtp-out.google.com with ESMTP id p8NJK7rr016032 for ; Fri, 23 Sep 2011 12:20:07 -0700 Received: from iaen33 (iaen33.prod.google.com [10.12.165.33]) by wpaz29.hot.corp.google.com with ESMTP id p8NJK0f4007378 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Fri, 23 Sep 2011 12:20:01 -0700 Received: by iaen33 with SMTP id n33so5957591iae.0 for ; Fri, 23 Sep 2011 12:20:00 -0700 (PDT) Received: by 10.42.161.68 with SMTP id s4mr1876064icx.158.1316805600400; Fri, 23 Sep 2011 12:20:00 -0700 (PDT) Received: by 10.42.161.68 with SMTP id s4mr1876052icx.158.1316805600256; Fri, 23 Sep 2011 12:20:00 -0700 (PDT) Received: from coign.google.com ([2620:0:1000:2301:21c:25ff:fe14:8d86]) by mx.google.com with ESMTPS id g16sm16721850ibs.8.2011.09.23.12.19.58 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 23 Sep 2011 12:19:59 -0700 (PDT) From: Ian Lance Taylor To: Pierre Vittet Cc: gcc@gcc.gnu.org, Basile Starynkevitch , gcc-melt@googlegroups.com, Alexandre Lissy , sje@cup.hp.com, gcc-patches@gcc.gnu.org Subject: Re: misbehaviour with md5_process_bytes and maybe in optimization References: <4E7C987C.1040505@pvittet.com> <4E7CCD08.9030904@pvittet.com> Date: Fri, 23 Sep 2011 12:19:57 -0700 In-Reply-To: <4E7CCD08.9030904@pvittet.com> (Pierre Vittet's message of "Fri, 23 Sep 2011 20:16:40 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Pierre Vittet writes: > Thanks for your interest, > > I just checked revision 179127 of GCC. Last revision is 177700, it has > not been change for 6 weeks. > > My file is the same as this one: > http://gcc.gnu.org/viewcvs/trunk/libiberty/md5.c?revision=177700&view=markup > > in libiberty/md5.c, function md5_process_bytes start line 203. > > On 23/09/2011 17:13, Ian Lance Taylor wrote: >> Pierre Vittet writes: >> >>> The bug appears when: >>> 1) We use libiberty compiled with -O0 >>> 2) We first call md5_process_bytes with a less than 64 bits buffer (we >>> call his size len1). >>> 3) We make a new call of md5_process_bytes with a buffer which has a >>> size len2 such as: >>> len2 > 127 + 65 (so test in line 228 of md5.C will be true) > line 228 is the following: if (len > 64) >>> 128 -len1 != Mulint with Mulint % __alignof__ (md5_uint32) != 0 (so >>> condition on line 238 is true) > line 238 is the following: if (UNALIGNED_P (buffer)) >>> len2 - (128 - len1) = Mul64 and Mul64 such as Mul %64=0 (so the loop of >>> line 239 is broken with len = 64, this leads to the bug as, line 249, >>> (len & ~63) = 64 and we shift the buffer without processing the data). > > line 239 is the following: while (len > 64) > line 249: buffer = (const void *) ((const char *) buffer + (len & ~63)); >> >> The line numbers you mention do not correspond to any version of >> libiberty/md5.c that I can see. Can you list the exact line for each >> line number you mention, so that your explanation is easier to follow? >> Thanks. > > I give about the same explanation in the README (which is in the > attached archive of my previous mail) but I does not use line number but > direct quote of the code. It mights be more easy to try the plugin with > gdb but it needs to compile libiberty.a with -O0. Thanks, I think I have it sorted out now. It does not happen on x86 glibc-based systems at -O2 because at -O2 #defines STRING_ARCH_unaligned, so the problematic code is not compiled or executed. The error was introduced by this change: 2005-07-03 Steve Ellcey PR other/13906 * md5.c (md5_process_bytes): Check alignment. Thanks for noticing this problem, analyzing it, and reporting it. I committed this patch to mainline to fix the problem. Bootstrapped on x86_64-unknown-linux-gnu. Ian 2011-09-23 Ian Lance Taylor * md5.c (md5_process_bytes): Correct handling of unaligned buffer. Index: md5.c =================================================================== --- md5.c (revision 179127) +++ md5.c (working copy) @@ -1,6 +1,6 @@ /* md5.c - Functions to compute MD5 message digest of files or memory blocks according to the definition of MD5 in RFC 1321 from April 1992. - Copyright (C) 1995, 1996 Free Software Foundation, Inc. + Copyright (C) 1995, 1996, 2011 Free Software Foundation, Inc. NOTE: This source is derived from an old version taken from the GNU C Library (glibc). @@ -245,9 +245,11 @@ md5_process_bytes (const void *buffer, s } else #endif - md5_process_block (buffer, len & ~63, ctx); - buffer = (const void *) ((const char *) buffer + (len & ~63)); - len &= 63; + { + md5_process_block (buffer, len & ~63, ctx); + buffer = (const void *) ((const char *) buffer + (len & ~63)); + len &= 63; + } } /* Move remaining bytes in internal buffer. */