Re: [PATCH] MdePkg/BaseSynchronizationLib: fix XADD operands in GCC IA32/X64 assembly

[Laszlo Ersek <lersek@redhat.com>]

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 <law@redhat.com> 4.8.5-29
> - s390 retpoline support for spectre mitigation (#1552021)
>
> * Fri Jan 19 2018 Marek Polacek <polacek@redhat.com> 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 <polacek@redhat.com> 4.8.5-27
> - bump for rebuild with RELRO enabled even for ppc64/ppc64le
>
> * Thu Jan 04 2018 Jeff Law <law@redhat.com> 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 <law@redhat.com> 4.8.5-25
> - Avoid red zone probe on aarch64 (#1469697)
>
> * Mon Nov 06 2017 Jeff Law <law@redhat.com> 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 <polacek@redhat.com> 4.8.5-23
> - rebuild to remove static relocations not known to older linkers (#1508968)
>
> * Thu Oct 19 2017 Marek Polacek <polacek@redhat.com> 4.8.5-22
> - fix gcc.c-torture/execute/pr80692.x
> - fix divmod expansion (PR middle-end/78416)
>
> * Wed Oct 18 2017 Marek Polacek <polacek@redhat.com> 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 <polacek@redhat.com> 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 <law@redhat.com> 4.8.5-19
> - Backport stack clash protection from upstream (#1469697)
>
> * Fri Oct 06 2017 Marek Polacek <polacek@redhat.com> 4.8.5-18
> - backport several -mprofile-kernel fixes (#1468546)
>
> * Thu Sep 14 2017 Marek Polacek <polacek@redhat.com> 4.8.5-17
> - fix -mcpu=power8 atomic expansion (#1437220, PR target/69644)
> - fix .toc alignment (#1487434)
>
> * Thu Jun 01 2017 Marek Polacek <polacek@redhat.com> 4.8.5-16
> - disable emitting profiling in functions marked with a special
>   attribute (#1457969)
>
> * Wed May 31 2017 Marek Polacek <polacek@redhat.com> 4.8.5-15
> - properly apply the PR70549 patch (#1349067)
>
> * Mon Mar 13 2017 Marek Polacek <polacek@redhat.com> 4.8.5-14
> - promote reloads of a PLUS to RELOAD_OTHER (#1402585)
>
> * Fri Mar 10 2017 Jakub Jelinek <jakub@redhat.com> 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 <jakub@redhat.com> 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 <jakub@redhat.com> 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 <jakub@redhat.com> 4.8.5-10
> - include vecintrin.h intrinsic header on s390 (#1182152)
>
> * Fri Jul 15 2016 Jakub Jelinek <jakub@redhat.com> 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 <jakub@redhat.com> 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 <jakub@redhat.com> 4.8.5-7
> - ensure the timestamp on printers.py is always the same (#1344291)
>
> * Mon Jun 06 2016 Jakub Jelinek <jakub@redhat.com> 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 <jakub@redhat.com> 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 <jakub@redhat.com> 4.8.5-4
> - fix up basic_streambuf copy constructor and assignment operator
>   (#1243366)
>
> * Thu Jul 02 2015 Jakub Jelinek <jakub@redhat.com> 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 <jakub@redhat.com> 4.8.5-2
> - add --enable-targets=powerpcle-linux on ppc64le (#1237363)
>
> * Tue Jun 23 2015 Jakub Jelinek <jakub@redhat.com> 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_<mode>* 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