public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Andrew Fish" <afish@apple.com>
To: devel@edk2.groups.io, rebecca@bsdio.com
Cc: Mike Kinney <michael.d.kinney@intel.com>, Ray' 'Ni <ray.ni@intel.com>
Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
Date: Thu, 18 May 2023 11:21:42 -0700	[thread overview]
Message-ID: <D6F0BB22-5A58-4493-8A7B-D8CB09377016@apple.com> (raw)
In-Reply-To: <caf17031-27a9-47f7-8231-7a549291fa70@app.fastmail.com>

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

Rebecca,

It looks like VC++ is trying to honor the volatile by reading the variable, incase that has side effects. But the loop is not checking the value of the variable and it is just doing an unconditional jump. This is why I think it is likely a compiler bug. Since the compiler emitted a hard code jmp in a loop it optimized out the return instruction….

$LN10@CpuDeadLoo:
mov       rax, QWORD PTR Index$[rsp]
call      CpuPause
jmp       SHORT $LN10@CpuDeadLoo
….

So changing the variable does not break you out of the loop. If you pc += 2 when you are at the jmp instruction that will not return you from CpuDeadLoop() that will just fall into the next function. That might work if CpuDeadLoop() was inlined, but if it was a call you would start running the next function in the binary. 

Thanks,

Andrew Fish


> On May 18, 2023, at 10:36 AM, Rebecca Cran <rebecca@bsdio.com> wrote:
> 
> When I use CpuDeadLoop for debugging on Aarch64 I have symbols loaded so I can just do ‘set Index=1’ and resume, but it sounds like the issue is that people want to sometimes debug without symbols/source, and the generated assembly is making that difficult.
> 
> Rebecca
> 
> On Thu, May 18, 2023, at 9:36 AM, Michael D Kinney 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> 
>> *Sent:* Thursday, May 18, 2023 3:00 AM
>> *To:* devel@edk2.groups.io
>> *Cc:* Kinney, Michael D <michael.d.kinney@intel.com>; Rebecca Cran 
>> <rebecca@bsdio.com>; Ni, Ray <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:
>> 1. Manually adjust rsp by increasing 40
>> 2. 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: Type: text/html, Size: 10243 bytes --]

      reply	other threads:[~2023-05-18 18:21 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-18  9:59 CpuDeadLoop() is optimized by compiler Ni, Ray
2023-05-18 13:19 ` [edk2-devel] " Pedro Falcato
2023-05-18 15:36 ` Michael D Kinney
2023-05-18 16:49   ` [edk2-devel] " Andrew Fish
2023-05-18 17:05     ` Michael D Kinney
2023-05-18 17:08       ` Andrew Fish
2023-05-18 17:19         ` Michael D Kinney
2023-05-18 17:22           ` Andrew Fish
2023-05-18 17:24           ` Andrew Fish
2023-05-18 18:45             ` Andrew Fish
     [not found]             ` <17605136DCF3E084.26337@groups.io>
2023-05-18 20:45               ` Andrew Fish
2023-05-18 21:42                 ` Michael D Kinney
2023-05-19  0:42                   ` Andrew Fish
2023-05-19  2:53                     ` Ni, Ray
2023-05-19  3:03                       ` Jeff Fan
2023-05-19 15:31                       ` Rebecca Cran
2023-05-19 16:31                         ` Andrew Fish
2023-10-31  2:51                           ` Ni, Ray
2023-10-31  3:37                             ` Michael D Kinney
2023-10-31  8:30                               ` Ni, Ray
2023-10-31 14:19                                 ` Michael D Kinney
2024-06-05  1:07                                   ` Michael D Kinney
2024-06-05 16:48                                     ` Oliver Smith-Denny
2024-06-07 16:57                                       ` Hernandez Miramontes, Jose Miguel
     [not found]                       ` <1760952DCE55DF8D.29365@groups.io>
2023-05-19 16:09                         ` Rebecca Cran
2023-05-18 17:36   ` Rebecca Cran
2023-05-18 18:21     ` Andrew Fish [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=D6F0BB22-5A58-4493-8A7B-D8CB09377016@apple.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox