> -----Original Message----- > From: Wang, Jian J > Sent: Wednesday, September 19, 2018 9:13 AM > To: Justen, Jordan L; [email protected] > Cc: Wu, Hao A; Bi, Dandan; Laszlo Ersek; Dong, Eric; Kinney, Michael D > Subject: RE: [edk2] [PATCH] UefiCpuPkg/CpuMpPei: fix unsafe way to get stack > pointer > > Jordan, > > No, it didn't help. But thanks for the help before. And I saw there's another > guy > having the same issue as mine. I tend to believe it's outlook (client/server) > issue > now. > > I think you're right about the "unsafe". The question is it's almost > impossible for > the static code checker to know how you use it. So it tend to give warning > anyway. > > Hao, do you have comment on adding the exception?
Hi, It is doable to tell the checker to ignore this issue. I think it will be better to also hear Mike's comment on this one. Best Regards, Hao Wu > > Regards, > Jian > > > > -----Original Message----- > > From: Justen, Jordan L > > Sent: Wednesday, September 19, 2018 2:03 AM > > To: Wang, Jian J <[email protected]>; [email protected] > > Cc: Wu, Hao A <[email protected]>; Bi, Dandan <[email protected]>; > > Laszlo Ersek <[email protected]>; Dong, Eric <[email protected]>; > Kinney, > > Michael D <[email protected]> > > Subject: Re: [edk2] [PATCH] UefiCpuPkg/CpuMpPei: fix unsafe way to get > stack > > pointer > > > > I guess the git config sendemail.from setting did not help your > > patches. ?? It still is coming through with a From field of > > <[email protected]>. > > > > Regarding this patch, I suppose it is worth asking if &StackBase in > > the old code could possibly be an address not on the stack. I don't > > think it is possible, and I'm guessing the C specification would > > probably back that up. > > > > It can be unsafe to get an address of something on the stack and then > > refer to that address after the variable is no longer in scope. I > > suspect this is what the static checker is noticing. By calling > > SetJump, aren't we just doing the same thing, but hiding what we are > > doing from the static checker? > > > > So, can't we just tell the static checker to ignore the error because > > we know what we are doing? > > > > -Jordan > > > > On 2018-09-18 02:04:48, wrote: > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1186 > > > > > > This patch uses SetJump() to get the stack pointer from esp/rsp > > > register to replace local variable way, which was marked by static > > > code checker as an unsafe way. > > > > > > Cc: Dandan Bi <[email protected]> > > > Cc: Hao A Wu <[email protected]> > > > Cc: Eric Dong <[email protected]> > > > Cc: Laszlo Ersek <[email protected]> > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Jian J Wang <[email protected]> > > > --- > > > UefiCpuPkg/CpuMpPei/CpuMpPei.h | 8 ++++++++ > > > UefiCpuPkg/CpuMpPei/CpuPaging.c | 9 +++++++-- > > > 2 files changed, 15 insertions(+), 2 deletions(-) > > > > > > diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h > > b/UefiCpuPkg/CpuMpPei/CpuMpPei.h > > > index d097a66aa8..fe61f5e3bc 100644 > > > --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h > > > +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h > > > @@ -35,6 +35,14 @@ > > > > > > extern EFI_PEI_PPI_DESCRIPTOR mPeiCpuMpPpiDesc; > > > > > > +#if defined (MDE_CPU_IA32) > > > +#define CPU_STACK_POINTER(Context) ((Context).Esp) > > > +#elif defined (MDE_CPU_X64) > > > +#define CPU_STACK_POINTER(Context) ((Context).Rsp) > > > +#else > > > +#error CPU type not supported! > > > +#endif > > > + > > > /** > > > This service retrieves the number of logical processor in the platform > > > and the number of those logical processors that are enabled on this > > > boot. > > > diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c > > b/UefiCpuPkg/CpuMpPei/CpuPaging.c > > > index c7e0822452..997c20c26e 100644 > > > --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c > > > +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c > > > @@ -517,9 +517,14 @@ GetStackBase ( > > > IN OUT VOID *Buffer > > > ) > > > { > > > - EFI_PHYSICAL_ADDRESS StackBase; > > > + EFI_PHYSICAL_ADDRESS StackBase; > > > + BASE_LIBRARY_JUMP_BUFFER Context; > > > > > > - StackBase = (EFI_PHYSICAL_ADDRESS)(UINTN)&StackBase; > > > + // > > > + // Retrieve stack pointer from current processor context. > > > + // > > > + SetJump (&Context); > > > + StackBase = (EFI_PHYSICAL_ADDRESS)CPU_STACK_POINTER (Context); > > > StackBase += BASE_4KB; > > > StackBase &= ~((EFI_PHYSICAL_ADDRESS)BASE_4KB - 1); > > > StackBase -= PcdGet32(PcdCpuApStackSize); > > > -- > > > 2.16.2.windows.1 > > > > > > _______________________________________________ > > > edk2-devel mailing list > > > [email protected] > > > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
