From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.43; helo=mga05.intel.com; envelope-from=jiewen.yao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 EEBD121B00DC4 for ; Thu, 16 Nov 2017 19:39:17 -0800 (PST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Nov 2017 19:43:27 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,406,1505804400"; d="scan'208";a="5958675" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga001.jf.intel.com with ESMTP; 16 Nov 2017 19:43:27 -0800 Received: from FMSMSX109.amr.corp.intel.com (10.18.116.9) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 16 Nov 2017 19:43:26 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx109.amr.corp.intel.com (10.18.116.9) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 16 Nov 2017 19:43:26 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.175]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.159]) with mapi id 14.03.0319.002; Fri, 17 Nov 2017 11:43:25 +0800 From: "Yao, Jiewen" To: Paulo Alcantara CC: "edk2-devel@lists.01.org" , Laszlo Ersek , "Dong, Eric" Thread-Topic: [edk2] [RFC v2 1/3] UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support Thread-Index: AQHTXnkhjviIJpWzXUmXsIl6yh6ASaMWO1uQgADRygCAAN51gA== Date: Fri, 17 Nov 2017 03:43:25 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503AA2121E@shsmsx102.ccr.corp.intel.com> References: <11840074660da43fd43fac88cff851f1ccc31143.1510778784.git.pcacjr@zytor.com> <74D8A39837DF1E4DA445A8C0B3885C503AA1E9D1@shsmsx102.ccr.corp.intel.com> In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZDQ2Njg1OTgtY2EyOC00ZjI1LWJkZWUtYWYyMTA4OTM0M2JhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJrQmw4TGJKZk1GcUhnMWpYSzNhd0tPblpjSEE2M08yVlBKUDcxSlwvWlpyWU9QTGtYSDRsKzEwUlY4VUNMQ3FFeCJ9 x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [RFC v2 1/3] UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Nov 2017 03:39:18 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks for your reply. Comments below: > -----Original Message----- > From: Paulo Alcantara [mailto:pcacjr@zytor.com] > Sent: Friday, November 17, 2017 6:13 AM > To: Yao, Jiewen > Cc: Paulo Alcantara ; edk2-devel@lists.01.org; Laszlo E= rsek > ; Dong, Eric > Subject: RE: [edk2] [RFC v2 1/3] UefiCpuPkg/CpuExceptionHandlerLib/X64: A= dd > stack trace support >=20 > Hi Jiewen, >=20 > On Wed, November 15, 2017 11:57 pm, Yao, Jiewen wrote: > > Hi Paulo > > Thanks to bring this cool feature. > > > > I have same feeling as Jeff Fan. It is great! > > > > I have some questions, hope you can clarify. > > 1) Would you please let us know which tool change is validated? Such as > > MSVC, or GCC? >=20 > GCC only. This should not work on MSVC because it relies on frame pointer= s. [Jiewen] Not work is OK. But my question is more about: Have you tested? Or have you tried? Do you see basic exception information can be dumped without any problem fo= r MSVC? Or do you see system crash immediately for MSVC? > > 2) Would you please let us know which phase is validated? Such as PEI > > PreMemory, PEI PostMemory, DXE, SMM? >=20 > DXE. I'm currently testing it by placing a random "CpuBreakpoint ()" in > PartitionDxe driver. >=20 [Jiewen] Ummm. Based upon the fact that the code you update may be used in all those phase= s. I recommend to validate them in the final patch. > > 3) Would you please let us know if there is any special condition > > validated? Such as code is in an interrupt handler, SMM initialization > > code, thunk code during mode switch, etc. > > I ask this because I have seen lots of discussion on sanity check, to > > avoid double exception. >=20 > At the moment I'm only ensuring proper aligned RSP and ESP values. But we > still need to validate the RIP values, etc. as Andrew and Brian suggested= . [Jiewen] OK. I look forward to seeing your patch to validate more. Same as #2. I suggest you validate all those corner cases in the final patc= h. > > 4) We supported some security feature to create a hole in normal memory > > map, such as PageGuard, StackGuard, SMM communication protection, etc. > > Accessing those memory directly will cause #PF immediately. > > Would you please let us know how we detect that case, to avoid RIP or R= SP > > fail to the non-present page? >=20 > Sorry. I have no idea :-( I'd hope to get some help from you guys. [Jiewen] One possible solution is to scan page table, before access this ad= dress, to make sure you are accessing present page. You can also filer invalid address directly by checking CPUID physical addr= ess bit before that. The invalid address will cause #GP instead of #PF. All I all, I would like to clarify the goal for exception lib: whatever exc= eption condition the BIOS meets, the exception handler can always dump the = *basic* exception information and halt. The missing of advanced symbol info is acceptable. But system crash, when w= e want to dump the advanced info, is not the best choice. > > 5) May I know why we check RIP < ImageBase? Is that legal or illegal if > > RIP > ImageBase+ImageSize, but RIP in another PE/COFF image? >=20 > That check was a wrong assumption that I had in the beginning. RIP may > point to either lower or higher addresses where the other PE/COFF images > are located. Fixed it in v3. [Jiewen] Great. Thanks. > Sorry for the delay in the response. I'm only able to work on this in my > free time. >=20 > Thanks! > Paulo >=20 > >> + // > >> + // Check if RIP is within another PE/COFF image base address > >> + // > >> + if (Rip < ImageBase) { > >> + // > >> + // Search for the respective PE/COFF image based on RIP > >> + // > > > > Thank you > > Yao Jiewen > > > > > >> -----Original Message----- > >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > >> Paulo > >> Alcantara > >> Sent: Thursday, November 16, 2017 9:18 AM > >> To: edk2-devel@lists.01.org > >> Cc: Laszlo Ersek ; Dong, Eric > >> Subject: [edk2] [RFC v2 1/3] UefiCpuPkg/CpuExceptionHandlerLib/X64: Ad= d > >> stack > >> trace support > >> > >> This patch adds stack trace support during a X64 CPU exception. > >> > >> It will dump out back trace, stack contents as well as image module > >> names that were part of the call stack. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Cc: Eric Dong > >> Cc: Laszlo Ersek > >> Signed-off-by: Paulo Alcantara > >> --- > >> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c = | > >> 369 > >> +++++++++++++++++++- > >> 1 file changed, 367 insertions(+), 2 deletions(-) > >> > >> diff --git > >> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c > >> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c > >> index 65f0cff680..11cd7c9e1c 100644 > >> --- > >> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c > >> +++ > >> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c > >> @@ -14,6 +14,11 @@ > >> > >> #include "CpuExceptionCommon.h" > >> > >> +// > >> +// Unknown PDB file name > >> +// > >> +GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 > >> *mUnknownPdbFileName =3D "????"; > >> + > >> /** > >> Return address map of exception handler template so that C code can > >> generate > >> exception tables. > >> @@ -242,6 +247,350 @@ DumpCpuContext ( > >> ); > >> } > >> > >> +/** > >> + Get absolute path and file name of PDB file in PE/COFF image. > >> + > >> + @param[in] ImageBase Base address of PE/COFF image. > >> + @param[out] PdbAbsoluteFilePath Absolute path of PDB file. > >> + @param[out] PdbFileName File name of PDB file. > >> +**/ > >> +STATIC > >> +VOID > >> +GetPdbFileName ( > >> + IN UINTN ImageBase, > >> + OUT CHAR8 **PdbAbsoluteFilePath, > >> + OUT CHAR8 **PdbFileName > >> + ) > >> +{ > >> + VOID *PdbPointer; > >> + CHAR8 *Str; > >> + > >> + // > >> + // Get PDB file name from PE/COFF image > >> + // > >> + PdbPointer =3D PeCoffLoaderGetPdbPointer ((VOID *)ImageBase); > >> + if (PdbPointer =3D=3D NULL) { > >> + // > >> + // No PDB file name found. Set it to an unknown file name. > >> + // > >> + *PdbFileName =3D (CHAR8 *)mUnknownPdbFileName; > >> + if (PdbAbsoluteFilePath !=3D NULL) { > >> + *PdbAbsoluteFilePath =3D NULL; > >> + } > >> + } else { > >> + // > >> + // Get file name portion out of PDB file in PE/COFF image > >> + // > >> + Str =3D (CHAR8 *)((UINTN)PdbPointer + > >> + AsciiStrLen ((CHAR8 *)PdbPointer) - sizeof *Str); > >> + for (; *Str !=3D '/' && *Str !=3D '\\'; Str--) { > >> + ; > >> + } > >> + > >> + // > >> + // Set PDB file name (also skip trailing path separator: '/' or > >> '\\') > >> + // > >> + *PdbFileName =3D Str + 1; > >> + > >> + if (PdbAbsoluteFilePath !=3D NULL) { > >> + // > >> + // Set absolute file path of PDB file > >> + // > >> + *PdbAbsoluteFilePath =3D PdbPointer; > >> + } > >> + } > >> +} > >> + > >> +/** > >> + Dump stack contents. > >> + > >> + @param[in] CurrentRsp Current stack pointer address. > >> + @param[in] UnwondStacksCount Count of unwond stack frames. > >> +**/ > >> +STATIC > >> +VOID > >> +DumpStackContents ( > >> + IN UINT64 CurrentRsp, > >> + IN INTN UnwondStacksCount > >> + ) > >> +{ > >> + // > >> + // Check for proper stack pointer alignment > >> + // > >> + if (((UINTN)CurrentRsp & (CPU_STACK_ALIGNMENT - 1)) !=3D 0) { > >> + InternalPrintMessage ("!!!! Unaligned stack pointer. !!!!\n"); > >> + return; > >> + } > >> + > >> + // > >> + // Dump out stack contents > >> + // > >> + InternalPrintMessage ("\nStack dump:\n"); > >> + while (UnwondStacksCount-- > 0) { > >> + InternalPrintMessage ( > >> + "0x%016lx: %016lx %016lx\n", > >> + CurrentRsp, > >> + *(UINT64 *)CurrentRsp, > >> + *(UINT64 *)((UINTN)CurrentRsp + 8) > >> + ); > >> + > >> + // > >> + // Point to next stack > >> + // > >> + CurrentRsp +=3D CPU_STACK_ALIGNMENT; > >> + } > >> +} > >> + > >> +/** > >> + Dump all image module names from call stack. > >> + > >> + @param[in] SystemContext Pointer to EFI_SYSTEM_CONTEXT. > >> +**/ > >> +STATIC > >> +VOID > >> +DumpImageModuleNames ( > >> + IN EFI_SYSTEM_CONTEXT SystemContext > >> + ) > >> +{ > >> + EFI_STATUS Status; > >> + UINT64 Rip; > >> + UINTN ImageBase; > >> + VOID *EntryPoint; > >> + CHAR8 *PdbAbsoluteFilePath; > >> + CHAR8 *PdbFileName; > >> + UINT64 Rbp; > >> + > >> + // > >> + // Set current RIP address > >> + // > >> + Rip =3D SystemContext.SystemContextX64->Rip; > >> + > >> + // > >> + // Set current frame pointer address > >> + // > >> + Rbp =3D SystemContext.SystemContextX64->Rbp; > >> + > >> + // > >> + // Check for proper frame pointer alignment > >> + // > >> + if (((UINTN)Rbp & (CPU_STACK_ALIGNMENT - 1)) !=3D 0) { > >> + InternalPrintMessage ("!!!! Unaligned frame pointer. !!!!\n"); > >> + return; > >> + } > >> + > >> + // > >> + // Get initial PE/COFF image base address from current RIP > >> + // > >> + ImageBase =3D PeCoffSearchImageBase (Rip); > >> + if (ImageBase =3D=3D 0) { > >> + InternalPrintMessage ("!!!! Could not find image module names. > >> !!!!"); > >> + return; > >> + } > >> + > >> + // > >> + // Get initial PE/COFF image's entry point > >> + // > >> + Status =3D PeCoffLoaderGetEntryPoint ((VOID *)ImageBase, &EntryPoin= t); > >> + if (EFI_ERROR (Status)) { > >> + EntryPoint =3D NULL; > >> + } > >> + > >> + // > >> + // Get file name and absolute path of initial PDB file > >> + // > >> + GetPdbFileName (ImageBase, &PdbAbsoluteFilePath, &PdbFileName); > >> + > >> + // > >> + // Print out initial image module name (if any) > >> + // > >> + if (PdbAbsoluteFilePath !=3D NULL) { > >> + InternalPrintMessage ( > >> + "\n%a (ImageBase=3D0x%016lx, EntryPoint=3D0x%016lx):\n", > >> + PdbFileName, > >> + ImageBase, > >> + (UINTN)EntryPoint > >> + ); > >> + InternalPrintMessage ("%a\n", PdbAbsoluteFilePath); > >> + } > >> + > >> + // > >> + // Walk through call stack and find next module names > >> + // > >> + for (;;) { > >> + // > >> + // Set RIP with return address from current stack frame > >> + // > >> + Rip =3D *(UINT64 *)((UINTN)Rbp + 8); > >> + > >> + // > >> + // If RIP is zero, then stop unwinding the stack > >> + // > >> + if (Rip =3D=3D 0) { > >> + break; > >> + } > >> + > >> + // > >> + // Check if RIP is within another PE/COFF image base address > >> + // > >> + if (Rip < ImageBase) { > >> + // > >> + // Search for the respective PE/COFF image based on RIP > >> + // > >> + ImageBase =3D PeCoffSearchImageBase (Rip); > >> + if (ImageBase =3D=3D 0) { > >> + // > >> + // Stop stack trace > >> + // > >> + break; > >> + } > >> + > >> + // > >> + // Get PE/COFF image's entry point > >> + // > >> + Status =3D PeCoffLoaderGetEntryPoint ((VOID *)ImageBase, > >> &EntryPoint); > >> + if (EFI_ERROR (Status)) { > >> + EntryPoint =3D NULL; > >> + } > >> + > >> + // > >> + // Get file name and absolute path of PDB file > >> + // > >> + GetPdbFileName (ImageBase, &PdbAbsoluteFilePath, > &PdbFileName); > >> + > >> + // > >> + // Print out image module name (if any) > >> + // > >> + if (PdbAbsoluteFilePath !=3D NULL) { > >> + InternalPrintMessage ( > >> + "%a (ImageBase=3D0x%016lx, EntryPoint=3D0x%016lx):\n", > >> + PdbFileName, > >> + ImageBase, > >> + (UINTN)EntryPoint > >> + ); > >> + InternalPrintMessage ("%a\n", PdbAbsoluteFilePath); > >> + } > >> + } > >> + > >> + // > >> + // Unwind the stack > >> + // > >> + Rbp =3D *(UINT64 *)(UINTN)Rbp; > >> + } > >> +} > >> + > >> +/** > >> + Dump stack trace. > >> + > >> + @param[in] SystemContext Pointer to EFI_SYSTEM_CONTEXT. > >> + @param[out] UnwondStacksCount Count of unwond stack frames. > >> +**/ > >> +STATIC > >> +VOID > >> +DumpStackTrace ( > >> + IN EFI_SYSTEM_CONTEXT SystemContext, > >> + OUT INTN *UnwondStacksCount > >> + ) > >> +{ > >> + UINT64 Rip; > >> + UINT64 Rbp; > >> + UINTN ImageBase; > >> + CHAR8 *PdbFileName; > >> + > >> + // > >> + // Set current RIP address > >> + // > >> + Rip =3D SystemContext.SystemContextX64->Rip; > >> + > >> + // > >> + // Set current frame pointer address > >> + // > >> + Rbp =3D SystemContext.SystemContextX64->Rbp; > >> + > >> + // > >> + // Get initial PE/COFF image base address from current RIP > >> + // > >> + ImageBase =3D PeCoffSearchImageBase (Rip); > >> + if (ImageBase =3D=3D 0) { > >> + InternalPrintMessage ("!!!! Could not find backtrace information. > >> !!!!"); > >> + return; > >> + } > >> + > >> + // > >> + // Get PDB file name from initial PE/COFF image > >> + // > >> + GetPdbFileName (ImageBase, NULL, &PdbFileName); > >> + > >> + // > >> + // Initialize count of unwond stacks > >> + // > >> + *UnwondStacksCount =3D 1; > >> + > >> + // > >> + // Print out back trace > >> + // > >> + InternalPrintMessage ("\nCall trace:\n"); > >> + > >> + for (;;) { > >> + // > >> + // Print stack frame in the following format: > >> + // > >> + // # @ + (RBP) in [ > | ????] > >> + // > >> + InternalPrintMessage ( > >> + "%d 0x%016lx @ 0x%016lx+0x%x (0x%016lx) in %a\n", > >> + *UnwondStacksCount - 1, > >> + Rip, > >> + ImageBase, > >> + Rip - ImageBase - 1, > >> + Rbp, > >> + PdbFileName > >> + ); > >> + > >> + // > >> + // Set RIP with return address from current stack frame > >> + // > >> + Rip =3D *(UINT64 *)((UINTN)Rbp + 8); > >> + > >> + // > >> + // If RIP is zero, then stop unwinding the stack > >> + // > >> + if (Rip =3D=3D 0) { > >> + break; > >> + } > >> + > >> + // > >> + // Check if RIP is within another PE/COFF image base address > >> + // > >> + if (Rip < ImageBase) { > >> + // > >> + // Search for the respective PE/COFF image based on RIP > >> + // > >> + ImageBase =3D PeCoffSearchImageBase (Rip); > >> + if (ImageBase =3D=3D 0) { > >> + // > >> + // Stop stack trace > >> + // > >> + break; > >> + } > >> + > >> + // > >> + // Get PDB file name > >> + // > >> + GetPdbFileName (ImageBase, NULL, &PdbFileName); > >> + } > >> + > >> + // > >> + // Unwind the stack > >> + // > >> + Rbp =3D *(UINT64 *)(UINTN)Rbp; > >> + > >> + // > >> + // Increment count of unwond stacks > >> + // > >> + (*UnwondStacksCount)++; > >> + } > >> +} > >> + > >> /** > >> Display CPU information. > >> > >> @@ -254,9 +603,25 @@ DumpImageAndCpuContent ( > >> IN EFI_SYSTEM_CONTEXT SystemContext > >> ) > >> { > >> + INTN UnwondStacksCount; > >> + > >> + // > >> + // Dump CPU context > >> + // > >> DumpCpuContext (ExceptionType, SystemContext); > >> + > >> + // > >> + // Dump stack trace > >> + // > >> + DumpStackTrace (SystemContext, &UnwondStacksCount); > >> + > >> + // > >> + // Dump image module names > >> + // > >> + DumpImageModuleNames (SystemContext); > >> + > >> // > >> - // Dump module image base and module entry point by RIP > >> + // Dump stack contents > >> // > >> - DumpModuleImageInfo (SystemContext.SystemContextX64->Rip); > >> + DumpStackContents (SystemContext.SystemContextX64->Rsp, > >> UnwondStacksCount); > >> } > >> -- > >> 2.14.3 > >> > >> _______________________________________________ > >> edk2-devel mailing list > >> edk2-devel@lists.01.org > >> https://lists.01.org/mailman/listinfo/edk2-devel > > > > >=20 >=20 > -- > Paulo Alcantara, HP Inc. > Speaking for myself only.