Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

["Andrew Fish" <afish@apple.com>]

[-- Attachment #1: Type: text/plain, Size: 8961 bytes --]

Whoops wrong compiler. Here is an update. I added the flags so this one reproduces the issue.

https://godbolt.org/#z:OYLghAFBqd5QCxAYwPYBMCmBRdBLAF1QCcAaPECAMzwBtMA7AQwFtMQByARg9KtQYEAysib0QXACx8BBAKoBnTAAUAHpwAMvAFYTStJg1DIApACYAQuYukl9ZATwDKjdAGFUtAK4sGIM6SuADJ4DJgAcj4ARpjEINIADqgKhE4MHt6%2B/qRJKY4CIWGRLDFx0naYDmlCBEzEBBk%2BfgEVVQI1dQSFEdGx8ba19Y1ZLYNdoT0lfZIAlLaoXsTI7BwA9KsA1AAqAJ4JmBs7C8QbaFgbCLGYpBskG7SoTOgbhhuYqqwJ9AB0JhoAgqECBsFABHLx1TAQIEbBg%2BGYbEwAdisAI26I2xEwBEWDFhPg2ACp8SwTABmVH/ZEAEQ4c1onAArLw/BwtKRUJw3NZrCDjstEWYyTxSARNHS5gBrECMjT6TiSFnijmcXgKEBysVsumkOCwJBoFgJOixciUQ3G%2BhxYBcMxcPh0AixdUQKLKqKhOo7Tgiw1sQQAeQYtG92tIWBYhmA4jD%2BCxVQAbph1WH3pUvE6fbwgZgGWHaHgosQvR4sMqCMQ8Cws3MqAZgAoAGp4TAAdwD%2B1ZIv4ghEYnYUhkgkUKnUYd09oMRhQPMs%2BkL6sgc1QCXyDBTAFoA2SNhvI0sEOTqQpJTsDJLMGrc5U1y4GO5PE09MEJsVSnpcqkBMM/PbP2vujfPp7VaNcOiGR8shA682gYcDxiKXo4hAsYfz0BQxkApCJDmBR%2BQHUUsRWHh6SZJUw05DhVAADgANg3WjJA2YBkGQDZbW%2BLgNggblLGsG5cEIO5zGFG4PCNE0ThErgZl4LUtDmCADVQCSrTNCALUklApxtSQNDlGhaCdYgXTdMMPWYYhQ19FT/QIIMQ2VCMoxjdk4xvPAkxTdk02QDNiOzQRc2VAsixLDAVnZCsqxrPh6ybFt207LMh2EURxEHHt5CUNRlV0AIdJnPi51CxcIGXVc0k3bdd33ZBDzJY9T3PS9bBg28IFcND7RfRCpmQnJki/dJIN/Qa8jSLD%2BvQ9rqlQ0aZvsMDMNfbCUM6bqBk6Kb3xk%2BZFmWPQK0wALSI4ZlSFZdlKJo%2BjGNOHT2Mkb4NBe7jeKsOcNkEogpKFe0NnEy1YkFMkzFk0VxUUpB8CoKh1KyvsMukLKR1y/MEHVSdMdhqgCD2dg5WITHsmJhRcfx/YNTOi6rt4SjqTwOGNmbNsQduhimJYtiOK4hMFA2Dn7uQR6pBel65KhqUZTlPNFUu5VKLVbJ5IleUODMcjrtVSHtRmOYkxMtJ4iAA

Thanks,

Andrew Fish

> On May 18, 2023, at 11:45 AM, Andrew Fish via groups.io <afish=apple.com@groups.io> wrote:
> 
> Mike,
> 
> This is a good way to play around with fixes, and to report bugs. You can see the assembler for different compilers with different flag. 
> 
> https://godbolt.org/#z:OYLghAFBqd5QCxAYwPYBMCmBRdBLAF1QCcAaPECAMzwBtMA7AQwFtMQByARg9KtQYEAysib0QXACx8BBAKoBnTAAUAHpwAMvAFYTStJg1DIApACYAQuYukl9ZATwDKjdAGFUtAK4sGIM6SuADJ4DJgAcj4ARpjEINIADqgKhE4MHt6%2B/qRJKY4CIWGRLDFx0naYDmlCBEzEBBk%2BfgEVVQI1dQSFEdGx8ba19Y1ZLYNdoT0lfZIAlLaoXsTI7BwA9KsA1AAqAJ4JmBs7C8QbaFgbCLGYpBskG7SoTOgbhhuYqqwJ9AB0JhoAgqECBsFABHLx1TAQIEbBg%2BGYbEwAdisAI26I2xEwBEWDFhPg2ACp8SwTABmVH/ZEAEQ4c1onAArLw/BwtKRUJw3NZrCDjstEWYyTxSARNHS5gBrECMjT6TiSFnijmcXgKEBysVsumkOCwJBoFgJOixciUQ3G%2BhxYBcMxcPh0AixdUQKLKqKhOo7Tgiw1sQQAeQYtG92tIWBYhmA4jD%2BCxVQAbph1WH3pUvE6fbwgZgGWHaHgosQvR4sMqCMQ8Cws3MqAZgAoAGp4TAAdwD%2B1ZIv4ghEYnYUhkgkUKnUYd09oMRhQPMs%2BkL6sgc1QCXyDBTAFoA2SNhvI0sEOTqQpJTsDJLMGrc5U1y4GO5PE09MEJsVSnpcqkBMM/PbP2vujfPp7VaNcOiGR8shA682gYcDxiKXo4hAsYfz0BQxkApCJDmBR%2BQHUUsRWHh6SZJUw05DhVAADgANg3WjJA2YBkGQDZbW%2BLgNggblLGsG5cEIO5zGFG4PCNE0ThErgZl4LUtDmCADVQCSrTNCALUklApxtSQNDlGhaCdYgXTdMMPWYYhQ19FT/QIIMQ2VCMoxjdk4xvPAkxTdk02QDNiOzQRc2VAsixLDAVnZCsqxrPh6ybFt207LMh2EURxEHHt5CUNRlV0AIdJnPi51CxcIGXVc0k3bdd33ZBDzJY9T3PS9bBg28IFcND7RfRCpmQnJki/dJIN/Qa8jSLD%2BvQ9rqlQ0aZvsMDMNfbCUM6bqBk6Kb3xk%2BZFmWPQK0wALSI4ZlSFZdlKJo%2BjGNOHT2Mkb4NBe7jeKsOcNkEogpKFe0NnEy1YkFMkzFk0VxUUpB8CoKh1KyvsMukLKR1y/MEHVSdMdhqgCD2dg5WITHsmJhRcfx/YNTOi6rt4SjqTwOGNmbNsQduhimJYtiOK4hMFA2Dn7uQR6pBel65KhqUZTlPNFUu5VKLVbJ5IleUODMcjrtVSHtRmOYkxMtJ4iAA
> 
> Sorry I’m traveling and in Cupertino with lots of meetings so I did not have time to adjust the compiler flags….
> 
> Thanks,
> 
> Andrew Fish
> 
>> On May 18, 2023, at 10:24 AM, Andrew (EFI) Fish <afish@apple.com> wrote:
>> 
>> Mike,
>> 
>> I guess my other question… If this turns out to be a compiler bug should we scope the change to the broken toolchain. I’m not sure what the right answer is for that, but I want to ask the question? 
>> 
>> Thanks,
>> 
>> Andrew Fish
>> 
>>> On May 18, 2023, at 10:19 AM, Michael D Kinney <michael.d.kinney@intel.com> wrote:
>>> 
>>> Andrew,
>>>  
>>> This might work for XIP.  Set non const global to initial value that is expected value to stay in dead loop.
>>>  
>>> UINTN  mDeadLoopCount = 0;
>>>  
>>> VOID
>>> CpuDeadLoop(
>>>   VOID
>>>   ) 
>>> {
>>>   while (mDeadLoopCount == 0) {
>>>       CpuPause();
>>>   }
>>> }
>>>  
>>> When deadloop is entered, developer can not change value of mDeadLoopCount, but they can use debugger to force exit loop and return from function.
>>>  
>>> Mike
>>>  
>>>  
>>> From: Andrew (EFI) Fish <afish@apple.com <mailto:afish@apple.com>> 
>>> Sent: Thursday, May 18, 2023 10:09 AM
>>> To: Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
>>> Cc: edk2-devel-groups-io <devel@edk2.groups.io <mailto:devel@edk2.groups.io>>; Ni, Ray <ray.ni@intel.com <mailto:ray.ni@intel.com>>; Rebecca Cran <rebecca@bsdio.com <mailto:rebecca@bsdio.com>>
>>> Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
>>>  
>>> Mike,
>>>  
>>> Good point, that is why we are using the stack ….
>>>  
>>> The only other thing I can think of is to pass the address of Index to some inline assembler, or an asm no op function, to give it a side effect the compiler can’t resolve. 
>>>  
>>> Thanks,
>>>  
>>> Andrew Fish
>>> 
>>> 
>>> On May 18, 2023, at 10:05 AM, Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>> wrote:
>>>  
>>> Static global will not work for XIP
>>>  
>>> Mike
>>>  
>>> From: Andrew (EFI) Fish <afish@apple.com <mailto:afish@apple.com>> 
>>> Sent: Thursday, May 18, 2023 9:49 AM
>>> To: edk2-devel-groups-io <devel@edk2.groups.io <mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
>>> Cc: Ni, Ray <ray.ni@intel.com <mailto:ray.ni@intel.com>>; Rebecca Cran <rebecca@bsdio.com <mailto:rebecca@bsdio.com>>
>>> Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
>>>  
>>> Mike,
>>>  
>>> I pinged some compiler experts to see if our code is correct, or if the compiler has an issue. Seems to be trending compiler issue right now, but I’ve NOT gotten feedback from anyone on the spec committee yet. 
>>>  
>>> If we move Index to a static global that would likely work around the compiler issue.
>>>  
>>> Thanks,
>>>  
>>> Andrew Fish
>>> 
>>> 
>>> 
>>> On May 18, 2023, at 8:36 AM, Michael D Kinney <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>> wrote:
>>>  
>>> Hi Ray,
>>>  
>>> So the code generated does deadloop, but is just not easy to resume from as we have been able to do in the past.
>>>  
>>> We use CpuDeadloop() for 2 purposes.  One is a terminal condition with no reason to ever continue.
>>>  
>>> The 2nd is a debug aide for developers to halt the system at a specific location and then continue from that point, usually with a debugger, to step through code to an area to evaluate unexpected behavior.
>>>  
>>> We may have to do a NASM implementation of CpuDeadloop() to make sure it meets both use cases.
>>>  
>>> Mike
>>>  
>>> From: Ni, Ray <ray.ni@intel.com <mailto:ray.ni@intel.com>> 
>>> Sent: Thursday, May 18, 2023 3:00 AM
>>> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>>> Cc: Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>; Rebecca Cran <rebecca@bsdio.com <mailto:rebecca@bsdio.com>>; Ni, Ray <ray.ni@intel.com <mailto:ray.ni@intel.com>>
>>> Subject: CpuDeadLoop() is optimized by compiler
>>>  
>>> Hi,
>>> Starting from certain version of Visual Studio C compiler (I don’t have the exact version. I am using VS2019), CpuDeadLoop is now optimized quite well by compiler.
>>>  
>>> The optimization is so “good” that it becomes harder for developers to break out of the deadloop.
>>>  
>>> I copied the assembly instructions as below for your reference.
>>> The compiler does not generate instructions that jump out of the loop when the Index is not zero.
>>> So in order to break out of the loop, developers need to:
>>> Manually adjust rsp by increasing 40
>>> Manually “ret”
>>>  
>>> I am not sure if anyone has interest to re-write this function so that compiler can be “fooled” again.
>>> Thanks,
>>> Ray
>>>  
>>> =======================
>>> ; Function compile flags: /Ogspy
>>> ; File e:\work\edk2\MdePkg\Library\BaseLib\CpuDeadLoop.c
>>> ;              COMDAT CpuDeadLoop
>>> _TEXT    SEGMENT
>>> Index$ = 48
>>> CpuDeadLoop PROC                                                                    ; COMDAT
>>>  
>>> ; 26   : {
>>>  
>>> $LN12:
>>>   00000  48 83 ec 28         sub        rsp, 40                                ; 00000028H
>>>  
>>> ; 27   :   volatile UINTN  Index;
>>> ; 28   : 
>>> ; 29   :   for (Index = 0; Index == 0;) {
>>>  
>>>   00004  48 c7 44 24 30
>>>                00 00 00 00        mov      QWORD PTR Index$[rsp], 0
>>> $LN10@CpuDeadLoo:
>>>  
>>> ; 30   :     CpuPause ();
>>>  
>>>   0000d  48 8b 44 24 30   mov      rax, QWORD PTR Index$[rsp]
>>>   00012  e8 00 00 00 00   call        CpuPause
>>>   00017  eb f4                     jmp       SHORT $LN10@CpuDeadLoo
>>> CpuDeadLoop ENDP
>>> _TEXT    ENDS
>>> END
>>>  
>>>  
>>>  
>> 
> 
> 


[-- Attachment #2.1: Type: text/html, Size: 42488 bytes --]

[-- Attachment #2.2: favicon.png --]
[-- Type: image/png, Size: 12765 bytes --]