Dear package maintainers and reviewers, Please do not approve this patch yet. I have thought further about the consequences of decorating CpuDeadLoop() and CpuBreakpoint() with the ANALYZER_NORETURN attribute and fear that the Analyzer, against the original intention, could issue incorrect warnings when they are used in the middle of code. All code following could probably be classified as unreachable (as the Analyzer will rightfully assume the Cpu* functions won't return).
I do not know if there is a way to limit this behavior to ASSERT. Maybe this patch should be gotten rid of and UNREACHABLE() should be added within the ASSERT() macro itself? This would make more sense than what I have done in this patch, looking back. If you have thoughts, please be kind enough to share them. Regards, Marvin. > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Marvin Häuser > Sent: Friday, June 10, 2016 11:02 PM > To: edk2-devel@lists.01.org > Cc: michael.d.kin...@intel.com; liming....@intel.com > Subject: [edk2] [PATCH v1 2/2] MdePkg/BaseLib: Flag CpuDeadLoop() and > CpuBreakpoint() as ANALYZER_NORETURN. > > Add the ANALYZER_NORETURN attribute to CpuDeadLoop() and > CpuBreakpoint() to avoid false 'possible NULL-dereference' warnings when > dereferencing pointers after having validated them with ASSERT(). As the > ANALYZER-prefixed versions are being used, the code following the calls will > not be optimized away. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Marvin Haeuser <marvin.haeu...@outlook.com> > --- > MdePkg/Library/BaseLib/CpuDeadLoop.c | 3 +++ > MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c | 3 +++ > MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.c | 3 +++ > MdePkg/Library/BaseLib/Ipf/CpuBreakpoint.c | 3 +++ > MdePkg/Library/BaseLib/Ipf/CpuBreakpointMsc.c | 3 +++ > MdePkg/Library/BaseLib/X64/CpuBreakpoint.c | 3 +++ > MdePkg/Library/BaseLib/X64/GccInline.c | 3 +++ > MdePkg/Include/Library/BaseLib.h | 2 ++ > MdePkg/Library/BaseLib/AArch64/CpuBreakpoint.S | 1 + > MdePkg/Library/BaseLib/Arm/CpuBreakpoint.S | 1 + > MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm | 1 + > MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm | 1 + > MdePkg/Library/BaseLib/X64/CpuBreakpoint.asm | 1 + > 13 files changed, 28 insertions(+) > > diff --git a/MdePkg/Library/BaseLib/CpuDeadLoop.c > b/MdePkg/Library/BaseLib/CpuDeadLoop.c > index 3f9440547d4d..7e386eab61a3 100644 > --- a/MdePkg/Library/BaseLib/CpuDeadLoop.c > +++ b/MdePkg/Library/BaseLib/CpuDeadLoop.c > @@ -28,6 +28,7 @@ > **/ > VOID > EFIAPI > +ANALYZER_NORETURN > CpuDeadLoop ( > VOID > ) > @@ -35,4 +36,6 @@ CpuDeadLoop ( > volatile UINTN Index; > > for (Index = 0; Index == 0;); > + > + ANALYZER_UNREACHABLE (); > } > diff --git a/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c > b/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c > index 9b7d875664bd..bd8da8a671d1 100644 > --- a/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c > +++ b/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c > @@ -29,11 +29,14 @@ _break ( > **/ > VOID > EFIAPI > +ANALYZER_NORETURN > CpuBreakpoint ( > VOID > ) > { > _break (3); > + > + ANALYZER_UNREACHABLE (); > } > > /** > diff --git a/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.c > b/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.c > index f5df7f2883c6..d68fd770ce3d 100644 > --- a/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.c > +++ b/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.c > @@ -32,10 +32,13 @@ void __debugbreak (); **/ VOID EFIAPI > +ANALYZER_NORETURN > CpuBreakpoint ( > VOID > ) > { > __debugbreak (); > + > + ANALYZER_UNREACHABLE (); > } > > diff --git a/MdePkg/Library/BaseLib/Ipf/CpuBreakpoint.c > b/MdePkg/Library/BaseLib/Ipf/CpuBreakpoint.c > index 302974bd5c98..29456b5f867d 100644 > --- a/MdePkg/Library/BaseLib/Ipf/CpuBreakpoint.c > +++ b/MdePkg/Library/BaseLib/Ipf/CpuBreakpoint.c > @@ -24,11 +24,14 @@ > **/ > VOID > EFIAPI > +ANALYZER_NORETURN > CpuBreakpoint ( > VOID > ) > { > __break (0); > + > + ANALYZER_UNREACHABLE (); > } > > /** > diff --git a/MdePkg/Library/BaseLib/Ipf/CpuBreakpointMsc.c > b/MdePkg/Library/BaseLib/Ipf/CpuBreakpointMsc.c > index 89b0acfd801b..2b098d462bee 100644 > --- a/MdePkg/Library/BaseLib/Ipf/CpuBreakpointMsc.c > +++ b/MdePkg/Library/BaseLib/Ipf/CpuBreakpointMsc.c > @@ -29,11 +29,14 @@ > **/ > VOID > EFIAPI > +ANALYZER_NORETURN > CpuBreakpoint ( > VOID > ) > { > __break (0); > + > + ANALYZER_UNREACHABLE (); > } > > /** > diff --git a/MdePkg/Library/BaseLib/X64/CpuBreakpoint.c > b/MdePkg/Library/BaseLib/X64/CpuBreakpoint.c > index d654f845716f..72055c2acfaa 100644 > --- a/MdePkg/Library/BaseLib/X64/CpuBreakpoint.c > +++ b/MdePkg/Library/BaseLib/X64/CpuBreakpoint.c > @@ -30,10 +30,13 @@ void __debugbreak (); **/ VOID EFIAPI > +ANALYZER_NORETURN > CpuBreakpoint ( > VOID > ) > { > __debugbreak (); > + > + ANALYZER_UNREACHABLE (); > } > > diff --git a/MdePkg/Library/BaseLib/X64/GccInline.c > b/MdePkg/Library/BaseLib/X64/GccInline.c > index 3d175ee9314e..b3443c55cb27 100644 > --- a/MdePkg/Library/BaseLib/X64/GccInline.c > +++ b/MdePkg/Library/BaseLib/X64/GccInline.c > @@ -99,11 +99,14 @@ CpuPause ( > **/ > VOID > EFIAPI > +ANALYZER_NORETURN > CpuBreakpoint ( > VOID > ) > { > __asm__ __volatile__ ("int $3"); > + > + ANALYZER_UNREACHABLE (); > } > > > diff --git a/MdePkg/Include/Library/BaseLib.h > b/MdePkg/Include/Library/BaseLib.h > index 79f421a97111..87f1e39d61db 100644 > --- a/MdePkg/Include/Library/BaseLib.h > +++ b/MdePkg/Include/Library/BaseLib.h > @@ -3960,6 +3960,7 @@ SwitchStack ( > **/ > VOID > EFIAPI > +ANALYZER_NORETURN > CpuBreakpoint ( > VOID > ); > @@ -3976,6 +3977,7 @@ CpuBreakpoint ( > **/ > VOID > EFIAPI > +ANALYZER_NORETURN > CpuDeadLoop ( > VOID > ); > diff --git a/MdePkg/Library/BaseLib/AArch64/CpuBreakpoint.S > b/MdePkg/Library/BaseLib/AArch64/CpuBreakpoint.S > index 6323cffaa981..e4008ec1d3fd 100644 > --- a/MdePkg/Library/BaseLib/AArch64/CpuBreakpoint.S > +++ b/MdePkg/Library/BaseLib/AArch64/CpuBreakpoint.S > @@ -28,6 +28,7 @@ GCC_ASM_EXPORT(CpuBreakpoint) #**/ #VOID > #EFIAPI > +#ANALYZER_NORETURN > #CpuBreakpoint ( > # VOID > # ); > diff --git a/MdePkg/Library/BaseLib/Arm/CpuBreakpoint.S > b/MdePkg/Library/BaseLib/Arm/CpuBreakpoint.S > index b6b80a1326d0..accb6f1b3287 100644 > --- a/MdePkg/Library/BaseLib/Arm/CpuBreakpoint.S > +++ b/MdePkg/Library/BaseLib/Arm/CpuBreakpoint.S > @@ -27,6 +27,7 @@ GCC_ASM_EXPORT(CpuBreakpoint) #**/ #VOID > #EFIAPI > +#ANALYZER_NORETURN > #CpuBreakpoint ( > # VOID > # ); > diff --git a/MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm > b/MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm > index 8a8065159bf2..5d51d13930f3 100644 > --- a/MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm > +++ b/MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm > @@ -27,6 +27,7 @@ > ;**/ > ;VOID > ;EFIAPI > +;ANALYZER_NORETURN > ;CpuBreakpoint ( > ; VOID > ; ); > diff --git a/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm > b/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm > index e4364055e8ee..e48a0fa92517 100644 > --- a/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm > +++ b/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm > @@ -28,6 +28,7 @@ > > ;------------------------------------------------------------------------------ > ; VOID > ; EFIAPI > +; ANALYZER_NORETURN > ; CpuBreakpoint ( > ; VOID > ; ); > diff --git a/MdePkg/Library/BaseLib/X64/CpuBreakpoint.asm > b/MdePkg/Library/BaseLib/X64/CpuBreakpoint.asm > index 25dd9b48e808..f0e4e93714fb 100644 > --- a/MdePkg/Library/BaseLib/X64/CpuBreakpoint.asm > +++ b/MdePkg/Library/BaseLib/X64/CpuBreakpoint.asm > @@ -25,6 +25,7 @@ > > ;------------------------------------------------------------------------------ > ; VOID > ; EFIAPI > +; ANALYZER_NORETURN > ; CpuBreakpoint ( > ; VOID > ; ); > -- > 2.7.4.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
