From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org 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 6146A21B02822 for ; Thu, 27 Sep 2018 03:19:26 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 917BC3001754; Thu, 27 Sep 2018 10:19:25 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-187.rdu2.redhat.com [10.10.120.187]) by smtp.corp.redhat.com (Postfix) with ESMTP id 80C8E5DD6B; Thu, 27 Sep 2018 10:19:24 +0000 (UTC) To: "Shao, Ming" Cc: "edk2-devel@lists.01.org" References: <20180925194857.10514-1-lersek@redhat.com> <8ecbcc60-8e0f-e418-614e-666aa7fb007b@Intel.com> <0D32B2537B667F42AD320D616D521AF738B92170@shsmsx102.ccr.corp.intel.com> From: Laszlo Ersek Message-ID: <51937bdb-7599-1987-ec4e-483a2571da52@redhat.com> Date: Thu, 27 Sep 2018 12:19:23 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <0D32B2537B667F42AD320D616D521AF738B92170@shsmsx102.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.46]); Thu, 27 Sep 2018 10:19:25 +0000 (UTC) Subject: Re: [PATCH] MdePkg/BaseSynchronizationLib: fix XADD operands in GCC IA32/X64 assembly X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Sep 2018 10:19:27 -0000 Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit On 09/27/18 11:28, Shao, Ming wrote: > Hi Laszlo, > > I build Ruiyu's code with gcc 4.8.5 as X64. And got below disassembled > code: > > [cid:image002.jpg@01D45686.AFEDF140] > > So I didn't see register used as both destination and source of xadd > instruction. > > Then I build your patch, and got exactly the same disassembled code. > > Both Ray's patch and yours can pass my test. That's not surprising: the gcc documentation says that not marking input-output operands as such may, or may not, break the code. Such a bug in the operand list of the inline assembly is not *required* to break the code, it just *can*. The only guarantee given is that, if the input-output operands are all marked properly as such, then the code will not break. > Could you provide more details about your environment so I can > reproduce this issue? Thanks. Sure. My compiler version is the following (on RHEL-7.5 Workstation): - gcc-4.8.5-28.el7_5.1.x86_64 Binutils (not that I think it matters in this case, but anyway): - binutils-2.27-28.base.el7_5.1.x86_64 The OVMF build command line was: build \ -a X64 \ -p OvmfPkg/OvmfPkgX64.dsc \ -t GCC48 \ -b NOOPT \ -D SECURE_BOOT_ENABLE \ -D TLS_ENABLE \ -D NETWORK_IP6_ENABLE \ -D HTTP_BOOT_ENABLE \ -n 4 \ --report-file=.../build.ovmf.report \ --log=.../build.ovmf.log It's very likely that the gcc version in RHEL-7.5 has a large number of backports from later upstream changes. The base version is indeed 4.8.5, but the internal build number is "-28.el7_5.1". That could mean a huge number of patches. I'll append the relevant part of the RPM changelog at the bottom of my email. I'd just like to point out that this is not a gcc bug; the compiler behaves according to its documentation. Thanks, Laszlo > * Tue Mar 27 2018 Jeff Law 4.8.5-29 > - s390 retpoline support for spectre mitigation (#1552021) > > * Fri Jan 19 2018 Marek Polacek 4.8.5-28 > - Minor testsuite fixes to clean up test results (#1469697) > - retpoline support for spectre mitigation (#1535655) > > * Thu Jan 11 2018 Marek Polacek 4.8.5-27 > - bump for rebuild with RELRO enabled even for ppc64/ppc64le > > * Thu Jan 04 2018 Jeff Law 4.8.5-26 > - Avoid red zone probing for zero residual dynamic allocation (#1469697) > - Avoid bogus CFIs for probes in noreturn fucntions on x86/x86_64 (#1469697) > > * Tue Nov 14 2017 Jeff Law 4.8.5-25 > - Avoid red zone probe on aarch64 (#1469697) > > * Mon Nov 06 2017 Jeff Law 4.8.5-24 > - Sync gcc48-rh1469697-13 patch to upstream (#1469697) > - Avoid probing in the red zone for noreturn functions (#1507980, #1469697) > - Avoid infinite loop if probing interval is less than guard size (#1469697) > - Fix debug information for large probing interval on aarch64 (#1469697) > - Fix ICE on ppc port with large probing interval (#1469697) > > - rebuild to remove static relocations not known to older linkers (#1508968) > > * Thu Nov 02 2017 Marek Polacek 4.8.5-23 > - rebuild to remove static relocations not known to older linkers (#1508968) > > * Thu Oct 19 2017 Marek Polacek 4.8.5-22 > - fix gcc.c-torture/execute/pr80692.x > - fix divmod expansion (PR middle-end/78416) > > * Wed Oct 18 2017 Marek Polacek 4.8.5-21 > - fix 27_io/basic_fstream/53984.cc > - fix for classes with bases with mutable members (PR c++/77375) > - fix handling side-effects of parameters (PR c/77767) > - fix combine's make_extraction (PR rtl-optimization/78378) > - fix gimplification of const var initialization from COND_EXPR (PR c++/80129) > - fix -A / -B to A / B folding (PR middle-end/80362) > - fix comparison of decimal float zeroes (PR middle-end/80692) > - fix __mulv[dt]i3 and expand_mul_overflow (PR target/82274) > > * Thu Oct 12 2017 Marek Polacek 4.8.5-20 > - handle exceptions in basic_istream::sentry (#1469384) > - don't run pr63354.c on ppc (#1468546) > - ensure proxy privatization safety (#1491395) > - fix incorrect codegen from rdseed intrinsic use (#1482762, CVE-2017-11671) > - on aarch64, remove libatomic.so (#1465510) > > * Sun Oct 08 2017 Jeff Law 4.8.5-19 > - Backport stack clash protection from upstream (#1469697) > > * Fri Oct 06 2017 Marek Polacek 4.8.5-18 > - backport several -mprofile-kernel fixes (#1468546) > > * Thu Sep 14 2017 Marek Polacek 4.8.5-17 > - fix -mcpu=power8 atomic expansion (#1437220, PR target/69644) > - fix .toc alignment (#1487434) > > * Thu Jun 01 2017 Marek Polacek 4.8.5-16 > - disable emitting profiling in functions marked with a special > attribute (#1457969) > > * Wed May 31 2017 Marek Polacek 4.8.5-15 > - properly apply the PR70549 patch (#1349067) > > * Mon Mar 13 2017 Marek Polacek 4.8.5-14 > - promote reloads of a PLUS to RELOAD_OTHER (#1402585) > > * Fri Mar 10 2017 Jakub Jelinek 4.8.5-13 > - add -mstack-protector-guard={tls,global}, -mstack-protector-guard-reg= > and -mstack-protector-guard-offset= options on ppc* (#1415952, > PR target/78875) > > * Tue Mar 07 2017 Jakub Jelinek 4.8.5-12 > - fix up std::rethrow_exception (#1375711, PR libstdc++/62258) > - use _Unwind_GetIPInfo in __gcc_personality_v0 (#1387402, PR libgcc/78064) > - fix vec_vsx_ld/st on ppc64le (#1389801, PR target/72863, PR target/78084) > - fix ICE in gfc_compare_derived_types (#1369183) > - fix EH from C++ thunks (#1427412, PR ipa/68184) > - on ppc64{,le} emit nop after recursive call if the current function > is replaceable (#1420723, PR target/79439) > - on aarch64 with -frounding-math use fnmul only with -(a*b) and not > with (-a)*b (#1418446, PR target/66731) > - constrain std::valarray functions and operators (#1416214, > PR libstdc++/69116) > - fix gimplification of aggregate assignments when lhs is used > (#1396298, PR middle-end/72747, PR c/78408) > - fix aarch64 TLS with -mcmodel=large (#1389276, PR target/78796) > - fix aarch64 reloading of floating point constants into general purpose > registers (#1349067, PR target/70549) > - fix DW_AT_decl_line on DW_TAG_enumeration_type for C enumeration > definitions following forward declarations (#1423460, PR c/79969) > > * Wed Aug 31 2016 Jakub Jelinek 4.8.5-11 > - on aarch64 emit scheduling barriers before stack deallocation in > function epilogues (#1362635, PR target/63293) > > * Wed Aug 10 2016 Jakub Jelinek 4.8.5-10 > - include vecintrin.h intrinsic header on s390 (#1182152) > > * Fri Jul 15 2016 Jakub Jelinek 4.8.5-9 > - backport OpenMP 4.5 support to libgomp (library only; #1357060, > PRs libgomp/68579, libgomp/64625) > > * Wed Jun 15 2016 Jakub Jelinek 4.8.5-8 > - fix a bug in C++ ref-to-ptr conversion (#1344807) > - fix combiner handling of jumps on aarch64 (#1344672, > PR rtl-optimization/52714) > > * Thu Jun 09 2016 Jakub Jelinek 4.8.5-7 > - ensure the timestamp on printers.py is always the same (#1344291) > > * Mon Jun 06 2016 Jakub Jelinek 4.8.5-6 > - backport s390 z13 support (#1182152) > - fix up -fsanitize=address on powerpc64 with 46-bit virtual address space > (#1312850) > - throw exception on std::random_device::_M_getval() failure (#1262846, > PR libstdc++/65142, CVE-2015-5276) > > * Tue May 10 2016 Jakub Jelinek 4.8.5-5 > - fix up libitm HTM fastpath (#1180633) > - on ppc64le default to -mcpu=power8 instead of -mcpu=power7 (#1213268) > - fix up size in .debug_pubnames (#1278872) > - turn powerpc* HTM insns into memory barriers (#1282755, PR target/67281) > - make sure to handle __builtin_alloca_with_align like alloca in > -fstack-protector* (#1289022, PR tree-optimization/68680) > - improve DW_AT_abstract_origin of DW_TAG_GNU_call_site on s390 with -fPIC > (#1312436) > - fix up libstdc++ pretty-printers (#1076690, PR libstdc++/53477) > - don't pass explicit --oformat option to ld on powerpc* (#1296211) > - backport Intel Memory Protection Keys ISA support - -mpku (#1304449) > > * Wed Jul 15 2015 Jakub Jelinek 4.8.5-4 > - fix up basic_streambuf copy constructor and assignment operator > (#1243366) > > * Thu Jul 02 2015 Jakub Jelinek 4.8.5-3 > - backport aarch64 crc enablement - -mcpu=generic+crc (#1179935) > - rebuild against fixed binutils to fix up systemtap markers on aarch64 > (#1238462) > > * Wed Jul 01 2015 Jakub Jelinek 4.8.5-2 > - add --enable-targets=powerpcle-linux on ppc64le (#1237363) > > * Tue Jun 23 2015 Jakub Jelinek 4.8.5-1 > - update from the 4.8 branch (#1230103) > - GCC 4.8.5 release > - fix -imacros handling (#1004526, PR c/57653) > - fix up IPA type handling (#1217267, PRs ipa/63551, ipa/64153) > - add PowerPC analyze swaps optimization pass (#1208103, #1200336) > - fix PowerPC vsx_extract_* patterns (#1206341) > - fix PowerPC -mcrypto handling (#1200335) > - Power8 unaligned vectorization improvements (#1199221, PR target/65456) > - PRs ada/47500, ada/63225, bootstrap/64213, c++/56710, c++/58624, > c++/63415, c++/63455, c++/64251, c++/64297, c++/64487, c++/65721, > c++/65727, c/52769, c/61553, c/64766, debug/63342, debug/65549, > fortran/56674, fortran/56867, fortran/57023, fortran/58813, > fortran/59016, fortran/59024, fortran/59488, fortran/60898, > fortran/61138, fortran/61407, fortran/63733, fortran/63744, > fortran/63938, fortran/64244, fortran/64528, fortran/65024, > gcov-profile/64634, inline-asm/63282, ipa/59626, ipa/63838, > libfortran/63589, libgfortran/59513, libgfortran/60956, libgomp/61200, > libstdc++/57440, libstdc++/59603, libstdc++/61947, libstdc++/63449, > libstdc++/63840, libstdc++/65279, libstdc++/65543, lto/65015, > lto/65193, middle-end/43631, middle-end/56917, middle-end/57748, > middle-end/58624, middle-end/59990, middle-end/63608, > middle-end/63665, middle-end/63704, middle-end/64067, > middle-end/64111, middle-end/64199, middle-end/64225, > middle-end/65409, middle-end/65680, middle-end/66133, > middle-end/66251, pch/65550, preprocessor/60436, > rtl-optimization/61058, rtl-optimization/63475, > rtl-optimization/63483, rtl-optimization/63659, > rtl-optimization/64037, rtl-optimization/64557, sanitizer/64265, > target/49423, target/52941, target/53988, target/55351, target/56846, > target/59593, target/60111, target/61413, target/62218, target/62642, > target/63335, target/63428, target/63673, target/63947, target/64113, > target/64115, target/64304, target/64358, target/64387, target/64409, > target/64452, target/64453, target/64479, target/64513, target/64579, > target/64580, target/64795, target/64882, target/64979, target/65138, > target/65163, target/65196, target/65286, target/65368, target/65787, > target/65849, target/66140, target/66148, target/66215, target/66275, > target/66470, target/66474, tree-optimization/59124, > tree-optimization/60656, tree-optimization/61634, > tree-optimization/61686, tree-optimization/61969, > tree-optimization/62031, tree-optimization/62167, > tree-optimization/63375, tree-optimization/63379, > tree-optimization/63551, tree-optimization/63593, > tree-optimization/63605, tree-optimization/63841, > tree-optimization/63844, tree-optimization/64269, > tree-optimization/64277, tree-optimization/64493, > tree-optimization/64495, tree-optimization/64563, > tree-optimization/65063, tree-optimization/65388, > tree-optimization/65518, tree-optimization/66123, > tree-optimization/66233, tree-optimization/66251, > tree-optimization/66272