From 1148d6ad13b444c179aacdb693bd82e37d0ed32d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caol=C3=A1n=20McNamara?= Date: Mon, 29 Apr 2024 20:45:05 +0100 Subject: [PATCH] introduce SAL_RET_MAYBENULL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit which for debug builds and MSVC uses _Ret_maybenull_ and -analyze to enforce null checking Change-Id: Id7f0ad854be7841819fdbdcd56c862d1a2df86c3 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166734 Reviewed-by: Caolán McNamara Tested-by: Jenkins --- cli_ure/Executable_climaker.mk | 2 ++ include/sal/types.h | 17 +++++++++++++++-- include/sfx2/objsh.hxx | 12 ++++++------ include/sfx2/viewfrm.hxx | 8 ++++---- include/sfx2/viewsh.hxx | 12 ++++++------ sfx2/source/doc/objxtor.cxx | 8 +++----- sfx2/source/view/viewfrm.cxx | 8 ++++---- sfx2/source/view/viewsh.cxx | 11 ++++------- solenv/gbuild/platform/com_MSC_class.mk | 14 +++++++++++--- solenv/gbuild/platform/com_MSC_defs.mk | 5 +++++ solenv/vs/LibreOffice.ruleset | 6 ++++++ 11 files changed, 66 insertions(+), 37 deletions(-) create mode 100644 solenv/vs/LibreOffice.ruleset diff --git a/cli_ure/Executable_climaker.mk b/cli_ure/Executable_climaker.mk index f0769b627cb2..2c2aaaf8a33e 100644 --- a/cli_ure/Executable_climaker.mk +++ b/cli_ure/Executable_climaker.mk @@ -13,9 +13,11 @@ $(eval $(call gb_Executable_use_package,climaker,\ cli_basetypes_copy \ )) +# -analyze- to turn that off due to internal compiler error $(eval $(call gb_Executable_add_cxxclrflags,climaker,\ -LN \ -wd4715 \ + -analyze- \ )) $(eval $(call gb_Executable_add_ldflags,climaker,\ diff --git a/include/sal/types.h b/include/sal/types.h index a2ef256239e1..61e12c8eb4d5 100644 --- a/include/sal/types.h +++ b/include/sal/types.h @@ -273,10 +273,10 @@ typedef void * sal_Handle; #endif /** Use this as markup for functions and methods whose return value must be - checked. + used. Compilers that support a construct of this nature will emit a compile - time warning on unchecked return value. + time warning on unused return value. */ #if defined LIBO_INTERNAL_ONLY && defined __cplusplus #define SAL_WARN_UNUSED_RESULT [[nodiscard]] @@ -288,6 +288,19 @@ typedef void * sal_Handle; # define SAL_WARN_UNUSED_RESULT #endif +/** Use this as markup for functions and methods whose return value may be + null and should not be derefenced unconditionally. + + Compilers that support a construct of this nature will emit a compile + time warning on unconditional dereference of returned pointer. +*/ +#ifdef _MSC_VER +# define SAL_RET_MAYBENULL _Ret_maybenull_ +#else +# define SAL_RET_MAYBENULL +#endif + + /** Use this for pure virtual classes, e.g. class SAL_NO_VTABLE Foo { ... This hinders the compiler from setting a generic vtable stating that a pure virtual function was called and thus slightly reduces code size. diff --git a/include/sfx2/objsh.hxx b/include/sfx2/objsh.hxx index e0b0d3f411a3..eececb850bd9 100644 --- a/include/sfx2/objsh.hxx +++ b/include/sfx2/objsh.hxx @@ -249,12 +249,12 @@ public: static OUString CreateShellID( const SfxObjectShell* pShell ); // Document-Shell Iterator - SAL_WARN_UNUSED_RESULT static SfxObjectShell* GetFirst( const std::function& isObjectShell = nullptr, - bool bOnlyVisible = true ); - SAL_WARN_UNUSED_RESULT static SfxObjectShell* GetNext( const SfxObjectShell& rPrev, - const std::function& isObjectShell = nullptr, - bool bOnlyVisible = true ); - SAL_WARN_UNUSED_RESULT static SfxObjectShell* Current(); + SAL_RET_MAYBENULL static SfxObjectShell* GetFirst( const std::function& isObjectShell = nullptr, + bool bOnlyVisible = true ); + SAL_RET_MAYBENULL static SfxObjectShell* GetNext( const SfxObjectShell& rPrev, + const std::function& isObjectShell = nullptr, + bool bOnlyVisible = true ); + SAL_RET_MAYBENULL static SfxObjectShell* Current(); static css::uno::Reference< css::uno::XInterface > GetCurrentComponent(); static void SetCurrentComponent( const css::uno::Reference< css::uno::XInterface >& _rxComponent ); diff --git a/include/sfx2/viewfrm.hxx b/include/sfx2/viewfrm.hxx index 3d0bf2fcc08a..4622ff167fcf 100644 --- a/include/sfx2/viewfrm.hxx +++ b/include/sfx2/viewfrm.hxx @@ -96,11 +96,11 @@ public: static SfxViewFrame* LoadDocumentIntoFrame( SfxObjectShell const & i_rDoc, const css::uno::Reference< css::frame::XFrame >& i_rFrameItem ); static SfxViewFrame* DisplayNewDocument( SfxObjectShell const & i_rDoc, const SfxRequest& i_rCreateDocRequest ); - SAL_WARN_UNUSED_RESULT static SfxViewFrame* Current(); - SAL_WARN_UNUSED_RESULT static SfxViewFrame* GetFirst( const SfxObjectShell* pDoc = nullptr, bool bOnlyVisible = true ); - SAL_WARN_UNUSED_RESULT static SfxViewFrame* GetNext( const SfxViewFrame& rPrev, const SfxObjectShell* pDoc = nullptr, bool bOnlyVisible = true ); + SAL_RET_MAYBENULL static SfxViewFrame* Current(); + SAL_RET_MAYBENULL static SfxViewFrame* GetFirst( const SfxObjectShell* pDoc = nullptr, bool bOnlyVisible = true ); + SAL_RET_MAYBENULL static SfxViewFrame* GetNext( const SfxViewFrame& rPrev, const SfxObjectShell* pDoc = nullptr, bool bOnlyVisible = true ); - SAL_WARN_UNUSED_RESULT SAL_DLLPRIVATE static SfxViewFrame* Get( const css::uno::Reference< css::frame::XController>& i_rController, const SfxObjectShell* i_pDoc ); + SAL_RET_MAYBENULL SAL_DLLPRIVATE static SfxViewFrame* Get( const css::uno::Reference< css::frame::XController>& i_rController, const SfxObjectShell* i_pDoc ); SAL_DLLPRIVATE void DoActivate(bool bMDI); SAL_DLLPRIVATE void DoDeactivate(bool bMDI, SfxViewFrame const *pOld); diff --git a/include/sfx2/viewsh.hxx b/include/sfx2/viewsh.hxx index 3ebd2f4ec72f..b7461b160af1 100644 --- a/include/sfx2/viewsh.hxx +++ b/include/sfx2/viewsh.hxx @@ -204,14 +204,14 @@ protected: public: // Iteration - SAL_WARN_UNUSED_RESULT static SfxViewShell* GetFirst( bool bOnlyVisible = true, const std::function& isViewShell = nullptr ); - SAL_WARN_UNUSED_RESULT static SfxViewShell* GetNext( const SfxViewShell& rPrev, - bool bOnlyVisible = true, - const std::function& isViewShell = nullptr ); - SAL_WARN_UNUSED_RESULT static SfxViewShell* Current(); + SAL_RET_MAYBENULL static SfxViewShell* GetFirst( bool bOnlyVisible = true, const std::function& isViewShell = nullptr ); + SAL_RET_MAYBENULL static SfxViewShell* GetNext( const SfxViewShell& rPrev, + bool bOnlyVisible = true, + const std::function& isViewShell = nullptr ); + SAL_RET_MAYBENULL static SfxViewShell* Current(); SAL_WARN_UNUSED_RESULT static bool IsCurrentLokViewReadOnly(); - SAL_WARN_UNUSED_RESULT static SfxViewShell* Get( const css::uno::Reference< css::frame::XController>& i_rController ); + SAL_RET_MAYBENULL static SfxViewShell* Get( const css::uno::Reference< css::frame::XController>& i_rController ); // Initialize Constructors/Destructors SFX_DECL_INTERFACE(SFX_INTERFACE_SFXVIEWSH) diff --git a/sfx2/source/doc/objxtor.cxx b/sfx2/source/doc/objxtor.cxx index e726f9a55cb9..deb64e6186ac 100644 --- a/sfx2/source/doc/objxtor.cxx +++ b/sfx2/source/doc/objxtor.cxx @@ -458,7 +458,7 @@ OUString SfxObjectShell::CreateShellID( const SfxObjectShell* pShell ) // returns a pointer the first SfxDocument of specified type -SfxObjectShell* SfxObjectShell::GetFirst +SAL_RET_MAYBENULL SfxObjectShell* SfxObjectShell::GetFirst ( const std::function& isObjectShell, bool bOnlyVisible @@ -483,7 +483,7 @@ SfxObjectShell* SfxObjectShell::GetFirst // returns a pointer to the next SfxDocument of specified type behind *pDoc -SfxObjectShell* SfxObjectShell::GetNext +SAL_RET_MAYBENULL SfxObjectShell* SfxObjectShell::GetNext ( const SfxObjectShell& rPrev, const std::function& isObjectShell, @@ -512,14 +512,12 @@ SfxObjectShell* SfxObjectShell::GetNext return nullptr; } - -SfxObjectShell* SfxObjectShell::Current() +SAL_RET_MAYBENULL SfxObjectShell* SfxObjectShell::Current() { SfxViewFrame *pFrame = SfxViewFrame::Current(); return pFrame ? pFrame->GetObjectShell() : nullptr; } - bool SfxObjectShell::IsInPrepareClose() const { return pImpl->bInPrepareClose; diff --git a/sfx2/source/view/viewfrm.cxx b/sfx2/source/view/viewfrm.cxx index c77de75697fc..1838700e9cb1 100644 --- a/sfx2/source/view/viewfrm.cxx +++ b/sfx2/source/view/viewfrm.cxx @@ -2067,14 +2067,14 @@ void SfxViewFrame::KillDispatcher_Impl() } } -SfxViewFrame* SfxViewFrame::Current() +SAL_RET_MAYBENULL SfxViewFrame* SfxViewFrame::Current() { SfxApplication* pApp = SfxApplication::Get(); return pApp ? pApp->Get_Impl()->pViewFrame : nullptr; } // returns the first window of spec. type viewing the specified doc. -SfxViewFrame* SfxViewFrame::GetFirst +SAL_RET_MAYBENULL SfxViewFrame* SfxViewFrame::GetFirst ( const SfxObjectShell* pDoc, bool bOnlyIfVisible @@ -2097,7 +2097,7 @@ SfxViewFrame* SfxViewFrame::GetFirst } // returns the next window of spec. type viewing the specified doc. -SfxViewFrame* SfxViewFrame::GetNext +SAL_RET_MAYBENULL SfxViewFrame* SfxViewFrame::GetNext ( const SfxViewFrame& rPrev, const SfxObjectShell* pDoc, @@ -2448,7 +2448,7 @@ SfxViewFrame* SfxViewFrame::DisplayNewDocument( SfxObjectShell const & i_rDoc, c ); } -SfxViewFrame* SfxViewFrame::Get( const Reference< XController>& i_rController, const SfxObjectShell* i_pDoc ) +SAL_RET_MAYBENULL SfxViewFrame* SfxViewFrame::Get( const Reference< XController>& i_rController, const SfxObjectShell* i_pDoc ) { if ( !i_rController.is() ) return nullptr; diff --git a/sfx2/source/view/viewsh.cxx b/sfx2/source/view/viewsh.cxx index 248d82bd8ce9..5204f8306c67 100644 --- a/sfx2/source/view/viewsh.cxx +++ b/sfx2/source/view/viewsh.cxx @@ -2835,8 +2835,7 @@ bool SfxViewShell::PrepareClose return true; } - -SfxViewShell* SfxViewShell::Current() +SAL_RET_MAYBENULL SfxViewShell* SfxViewShell::Current() { SfxViewFrame *pCurrent = SfxViewFrame::Current(); return pCurrent ? pCurrent->GetViewShell() : nullptr; @@ -2850,7 +2849,7 @@ bool SfxViewShell::IsCurrentLokViewReadOnly() return pCurrent && pCurrent->IsLokReadOnlyView(); } -SfxViewShell* SfxViewShell::Get( const Reference< XController>& i_rController ) +SAL_RET_MAYBENULL SfxViewShell* SfxViewShell::Get( const Reference< XController>& i_rController ) { if ( !i_rController.is() ) return nullptr; @@ -2866,7 +2865,6 @@ SfxViewShell* SfxViewShell::Get( const Reference< XController>& i_rController ) return nullptr; } - SdrView* SfxViewShell::GetDrawView() const /* [Description] @@ -3011,7 +3009,7 @@ void SfxViewShell::WriteUserDataSequence ( uno::Sequence < beans::PropertyValue // returns the first shell of spec. type viewing the specified doc. -SfxViewShell* SfxViewShell::GetFirst +SAL_RET_MAYBENULL SfxViewShell* SfxViewShell::GetFirst ( bool bOnlyVisible, const std::function< bool ( const SfxViewShell& ) >& isViewShell @@ -3040,7 +3038,7 @@ SfxViewShell* SfxViewShell::GetFirst } // returns the next shell of spec. type viewing the specified doc. -SfxViewShell* SfxViewShell::GetNext +SAL_RET_MAYBENULL SfxViewShell* SfxViewShell::GetNext ( const SfxViewShell& rPrev, bool bOnlyVisible, @@ -3068,7 +3066,6 @@ SfxViewShell* SfxViewShell::GetNext return nullptr; } - void SfxViewShell::Notify( SfxBroadcaster& rBC, const SfxHint& rHint ) { diff --git a/solenv/gbuild/platform/com_MSC_class.mk b/solenv/gbuild/platform/com_MSC_class.mk index 94269f09de7a..6c1015e9b533 100644 --- a/solenv/gbuild/platform/com_MSC_class.mk +++ b/solenv/gbuild/platform/com_MSC_class.mk @@ -49,7 +49,7 @@ endef gb_Helper_remove_overridden_flags = \ $(filter-out -W4 -w -arch:SSE -arch:SSE2 -arch:AVX -arch:AVX2 -Od -O2 -Zc:inline -Zc:inline- \ -Zc:dllexportInlines -Zc:dllexportInlines- -EHs -EHa -DNOMINMAX -UNOMINMAX -D_X86_=1 -U_X86_ \ - -D_AMD64_=1 -U_AMD64_,$(1)) \ + -D_AMD64_=1 -U_AMD64_ $(MSVC_ANALYZE_FLAGS) -analyze-,$(1)) \ $(lastword $(filter -W4 -w,$(1))) \ $(lastword $(filter -Od -O2,$(1))) \ $(lastword $(filter -arch:SSE -arch:SSE2 -arch:AVX -arch:AVX2,$(1))) \ @@ -58,7 +58,14 @@ gb_Helper_remove_overridden_flags = \ $(lastword $(filter -D_X86_=1 -U_X86_,$(1))) \ $(lastword $(filter -D_AMD64_=1 -U_AMD64_,$(1))) \ $(lastword $(filter -Zc:inline -Zc:inline-,$(1))) \ - $(lastword $(filter -Zc:dllexportInlines -Zc:dllexportInlines-,$(1))) + $(lastword $(filter -Zc:dllexportInlines -Zc:dllexportInlines-,$(1))) \ + $(lastword $(filter $(MSVC_ANALYZE_FLAGS) -analyze-,$(1))) + +# When gb_LinkTarget_use_clang is used, filter out MSVC flags that Clang doesn't know. +# $(call gb_CObject__filter_out_clang_cflags,cflags) +define gb_CObject__filter_out_clang_cflags + $(filter-out $(gb_FilterOutClangCFLAGS),$(1)) +endef # $(call gb_CObject__command_pattern,object,flags,source,dep-file,compiler-plugins,compiler) define gb_CObject__command_pattern @@ -71,7 +78,8 @@ $(call gb_Helper_abbreviate_dirs,\ $(call gb_Helper_remove_overridden_flags, \ $(DEFS) \ $(if $(filter YES,$(LIBRARY_X64)), ,$(gb_LTOFLAGS)) \ - $(2) $(if $(WARNINGS_DISABLED),$(gb_CXXFLAGS_DISABLE_WARNINGS)) \ + $(if $(6), $(call gb_CObject__filter_out_clang_cflags,$(2)),$(2)) \ + $(if $(WARNINGS_DISABLED),$(gb_CXXFLAGS_DISABLE_WARNINGS)) \ $(if $(EXTERNAL_CODE), \ $(if $(filter -clr,$(2)),,$(if $(COM_IS_CLANG),-Wno-undef)), \ $(gb_DEFS_INTERNAL)) \ diff --git a/solenv/gbuild/platform/com_MSC_defs.mk b/solenv/gbuild/platform/com_MSC_defs.mk index 1ab191f27776..506cfbe7d30a 100644 --- a/solenv/gbuild/platform/com_MSC_defs.mk +++ b/solenv/gbuild/platform/com_MSC_defs.mk @@ -104,6 +104,10 @@ gb_AFLAGS := $(AFLAGS) # C4706: assignment within conditional expression +MSVC_ANALYZE_FLAGS := -analyze:ruleset$(SRCDIR)/solenv/vs/LibreOffice.ruleset + +gb_FilterOutClangCFLAGS += $(MSVC_ANALYZE_FLAGS) + gb_CFLAGS := \ -utf-8 \ -Gd \ @@ -141,6 +145,7 @@ gb_CXXFLAGS := \ -wd4611 \ -wd4706 \ -bigobj \ + $(if $(ENABLE_DEBUG),$(MSVC_ANALYZE_FLAGS),) \ ifneq ($(COM_IS_CLANG),TRUE) gb_CXXFLAGS += -Zc:inline diff --git a/solenv/vs/LibreOffice.ruleset b/solenv/vs/LibreOffice.ruleset new file mode 100644 index 000000000000..3c7fce9c0adc --- /dev/null +++ b/solenv/vs/LibreOffice.ruleset @@ -0,0 +1,6 @@ + + + + + +