From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6263521DFA7A8 for ; Fri, 31 Mar 2017 11:44:37 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BDA09C0567B3; Fri, 31 Mar 2017 18:44:36 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com BDA09C0567B3 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com BDA09C0567B3 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-153.phx2.redhat.com [10.3.116.153]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3F14D17156; Fri, 31 Mar 2017 18:44:35 +0000 (UTC) To: Qin Long , edk2-devel@lists.01.org References: <20170331170517.4672-1-qin.long@intel.com> <20170331170517.4672-3-qin.long@intel.com> Cc: ting.ye@intel.com, hao.a.wu@intel.com, feng.tian@intel.com, eric.dong@intel.com From: Laszlo Ersek Message-ID: <2a380b37-ca12-6d00-5c83-9c77bda8d8a3@redhat.com> Date: Fri, 31 Mar 2017 20:44:34 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170331170517.4672-3-qin.long@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 31 Mar 2017 18:44:36 +0000 (UTC) Subject: Re: [Patch 2/4] CryptoPkg: Fix possible unresolved external symbol issue. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 31 Mar 2017 18:44:37 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 03/31/17 19:05, Qin Long wrote: > The compiler (visual studio) may optimize some explicit strcmp call > to use the intrinsic memcmp call. In CrtLibSupport.h, we just use > #define to mapping memcmp to CompareMem API. So in Link phase, this > kind of intrinsic optimization will cause the "unresolved external > symbol" error. For example: > OpensslLib.lib(v3_utl.obj) : error LNK2001: > unresolved external symbol _memcmp > > This patch will keep the memcmp mapping, and provide extra Intrinsic > memcmp wrapper to satisfy the symbol link. > > Cc: Ting Ye > Cc: Feng Tian > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Qin Long > --- > CryptoPkg/Include/CrtLibSupport.h | 1 + > CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c | 6 ++++++ > 2 files changed, 7 insertions(+) > > diff --git a/CryptoPkg/Include/CrtLibSupport.h b/CryptoPkg/Include/CrtLibSupport.h > index ddf7784a37..7f1ec12302 100644 > --- a/CryptoPkg/Include/CrtLibSupport.h > +++ b/CryptoPkg/Include/CrtLibSupport.h > @@ -133,6 +133,7 @@ void *malloc (size_t); > void *realloc (void *, size_t); > void free (void *); > void *memset (void *, int, size_t); > +int memcmp (const void *, const void *, size_t); > int isdigit (int); > int isspace (int); > int isxdigit (int); > diff --git a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c > index e8a76d07ff..33489aa6f4 100644 > --- a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c > +++ b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c > @@ -46,6 +46,12 @@ void * memset (void *dest, char ch, unsigned int count) > return dest; > } > > +/* Compare characters in two buffers. */ > +int memcmp (const void *buf1, const void *buf2, unsigned int count) > +{ > + return (int)(CompareMem(buf1,buf2,(UINTN)(count))); > +} > + > int strcmp (const char *s1, const char *s2) > { > return (int)AsciiStrCmp(s1, s2); > Can we just suppress the optimization to the intrinsic with VS? If we can't, then: - The prototype and the function definition don't match. The last argument should be "size_t" in the function definition too, not "unsigned int". - No need for the explicit cast to UINTN for "count", because size_t will match UINTN exactly. - No need for the parens around "count". - Please add spaces between the arguments, after the commas. - the comment should say, "compare bytes", not "compare characters". - In general, INTN cannot be safely cast to "int" (for the return value). In practice, it is likely safe, because the CompareMem implementations in edk2 return byte differences, which can be represented as "int". So I guess this is safe. (I had the same thought when I reviewed the strcmp() wrapper.) - Given that we are adding an actual memcmp() function, can we remove the #define for it? In commit 420e508397c7 ("CryptoPkg: Fix handling of &strcmp function pointers", 2017-03-23), we did the same for strcmp(): added the function, removed the macro. Thanks, Laszlo