public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Andrew Fish <afish@apple.com>
To: Liu Yu <pedroa.liu@outlook.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>,
	"ruiyu.ni@intel.com" <ruiyu.ni@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: EmulatorPkg Unix Host Segmentation fault.
Date: Sun, 18 Nov 2018 14:37:09 -0800	[thread overview]
Message-ID: <81FDCF97-4145-4A7D-93B9-70A4D8B505FF@apple.com> (raw)
In-Reply-To: <TY2PR02MB283107D99637038D2F5129E78FDF0@TY2PR02MB2831.apcprd02.prod.outlook.com>



> On Nov 18, 2018, at 4:07 AM, Liu Yu <pedroa.liu@outlook.com> wrote:
> 
> sorry your  path can't fix this issue.   if this path just turn off 
> optimization option within sec.c not global project.
> 
> I have tested different version GCC such as (GCC4,8, GCC5.x, GCC7.x)  
> and all of them can duplicate this issue  (Ubuntu 16.04, 16.10,18.04 )
> 
> I have traced this issue on my hand.
> 
> you can see Dispatcher.c (MdeModulePkg/Pei/DIspatcher/) Line 792:
> 
> 
> 790      if (StackOffsetPositive) {
> 791       SecCoreData = (CONST EFI_SEC_PEI_HAND_OFF *)((UINTN)(VOID 
> *)SecCoreData + StackOffset);
> 792      Private = (PEI_CORE_INSTANCE *)((UINTN)(VOID *)Private + 
> StackOffset);
> 793     } else {
> 794      ..........
> 795      ..........
> 796    }
> 
>  790 --792 disassembly code
> 
>  0x10200f2ca <PeiCheckAndSwitchStack+1030>:    test %r14b,%r14b
>  0x10200f2cd <PeiCheckAndSwitchStack+1033>:    je 0x10200f2df 
> <PeiCheckAndSwitchStack+1051>
>  0x10200f2cf <PeiCheckAndSwitchStack+1035>:    mov 0x38(%rsp),%rax
>  0x10200f2d4 <PeiCheckAndSwitchStack+1040>:    lea 0x0(%rbp,%rax,1),%r14
>  0x10200f2d9 <PeiCheckAndSwitchStack+1045>:    lea (%rbx,%rax,1),%rbp
> 
>  we can see Private value have been stored in %rbp  (rbp register be 
> used as general register )   so when call 
> TemporaryRamSupportPpi->TemporaryRamMigration()
> 

The calling conventions define RBP as non-volatile must be preserved by callee. Using RBP as the frame pointer is optional. 

Is it possible the assembly coder is assuming RBP is a frame pointer? That would imply for gcc/clang the correct answer would be to have compiler flags force frame pointer usage? 

Assuming -O 0 does something seems like we are matching an implementation at a given point in time. I'd rather force the frame pointer usage (that is optional in the ABI) if that fixes the RBP usage assumption. I guess the other option would be to have different assembler if the compiler is using frame pointers or not. and I don't think we have that concept. 

Given this is the common frame pointer pattern:

	pushq	%rbp
	movq	%rsp, %rbp
...
	popq	%rbp
	retq

It follows the calling convention rules even if the frame pointer is not in general use. Thus it only seems like you would hit issues when you move the stack around. 

Thanks,

Andrew Fish

PS Xcode clang always emits the frame pointer. 

> this function would modify rbp value because it treat rbp as "stack base 
> address ".
> 
> 816     MigrateMemoryPages (Private, TRUE);
> 
> // Private pointer point to other address, so this function would get a 
> NULL pointer that result in segment fault
> 
> I think we can turn off optimization options like this.
> 
> 1. modify  EmulatorPkg.dsc
> 
>       MdeModulePkg/Core/Pei/PeiMain.inf {
>          <BuildOptions>
>           GCC:*_*_*_CC_FLAGS = -O0
>   }
> 
> Reference GCC Manual description:
> 
>   -O also turns on -fomit-frame-pointer on machines where doing so does 
> not interfere with debugging.
> 
> 
> 
> 在 2018/11/18 下午5:27, Jordan Justen 写道:
>> On 2018-11-17 20:51:11, Liu Yu wrote:
>>> OS: Ubuntu
>>> 
>>> Toolchain:GCC48
>> I don't have gcc-4.8, so I couldn't reproduce the issue, but I wonder
>> if this branch can fix the issue for you?
>> 
>> https://github.com/jljusten/edk2/tree/emulator-temp-ram
>> 
>> You can fetch this branch locally to a branch named `test` with a
>> command like this:
>> 
>> $ git fetch --no-tags https://github.com/jljusten/edk2.git emulator-temp-ram:test
>> 
>> Then checkout the `test` branch to try it.
>> 
>> First, there is some patches to cleanup Sec, but then I added a patch:
>> 
>> 53a432e149 "EmulatorPkg/Sec: Disable optimizations for TemporaryRamMigration function"
>> 
>> Which I hope might help in your case.
>> 
>> -Jordan
>> 
>>> Issue Description :
>>> 
>>>   Program received signal SIGSEGV, Segmentation fault.
>>>    at /home/pedroa/workspace/orign/edkcrb/MdeModulePkg/Core/Pei/Memory/MemoryServices.c:129
>>> 129      Private->MemoryPages.Size = (UINTN) (Private->HobList.HandoffInformationTable->EfiMemoryTop -
>>> 
>>> 
>>> if the GCC optimization option is used not -O0 so the "rbp" register will be used as "general register"
>>> 
>>> in the SecTemporaryRamSupport function as below, this function will modify the rbp (as general register not stack base address pointer)value that result in program crash.
>>> 
>>> ASM_PFX(SecTemporaryRamSupport):
>>>   // Adjust callers %rbp to account for stack move
>>>   subq    %rdx, %rbp     // Calc offset of %rbp in Temp Memory
>>>   addq    %r8,  %rbp     // add in permanent base to offset
>>> 
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel



  reply	other threads:[~2018-11-18 22:38 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-18  4:51 EmulatorPkg Unix Host Segmentation fault Liu Yu
2018-11-18  9:27 ` Jordan Justen
2018-11-18 12:07   ` Liu Yu
2018-11-18 22:37     ` Andrew Fish [this message]
2018-11-19  1:13       ` Jordan Justen
2018-11-19 19:16         ` Jordan Justen
2018-11-19 21:22           ` Andrew Fish
2018-11-19 22:12             ` Laszlo Ersek
2018-11-19 23:39               ` Andrew Fish
2018-11-19 22:29             ` Jordan Justen
2018-11-20  0:54               ` Andrew Fish
2018-11-20  8:57               ` Laszlo Ersek
2019-02-16  7:29 ` Ni, Ray
2019-02-16  7:40 ` Ni, Ray
2019-02-16 20:23   ` Andrew Fish
2019-02-16  7:43 ` Ni, Ray
2019-02-16  8:05   ` Ni, Ray
2019-02-16  9:11     ` Jordan Justen
2019-02-18  2:25       ` Ni, Ray
2019-02-18  2:45         ` Jordan Justen

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=81FDCF97-4145-4A7D-93B9-70A4D8B505FF@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