diff options
| -rw-r--r-- | compilerplugins/clang/sallogareas.cxx | 107 | ||||
| -rw-r--r-- | compilerplugins/clang/test/sallogareas.cxx | 58 | ||||
| -rw-r--r-- | include/tools/diagnose_ex.h | 24 | ||||
| -rw-r--r-- | solenv/CompilerTest_compilerplugins_clang.mk | 1 | ||||
| -rw-r--r-- | tools/source/debug/debug.cxx | 7 | ||||
| -rw-r--r-- | xmloff/source/draw/shapeexport.cxx | 6 | ||||
| -rw-r--r-- | xmloff/source/draw/ximpshap.cxx | 30 |
7 files changed, 161 insertions, 72 deletions
diff --git a/compilerplugins/clang/sallogareas.cxx b/compilerplugins/clang/sallogareas.cxx index 98cafb596fd5..1095302d9ada 100644 --- a/compilerplugins/clang/sallogareas.cxx +++ b/compilerplugins/clang/sallogareas.cxx @@ -49,54 +49,73 @@ bool SalLogAreas::VisitCallExpr( const CallExpr* call ) { if( ignoreLocation( call )) return true; - if( const FunctionDecl* func = call->getDirectCallee()) + const FunctionDecl* func = call->getDirectCallee(); + if( !func ) + return true; + + if( !( func->getNumParams() == 5 && func->getIdentifier() != NULL + && ( func->getName() == "sal_detail_log" || func->getName() == "log" || func->getName() == "DbgUnhandledException")) ) + return true; + + auto tc = loplugin::DeclCheck(func); + enum class LogCallKind { Sal, DbgUnhandledException}; + LogCallKind kind; + int areaArgIndex; + if( tc.Function("sal_detail_log") || tc.Function("log").Namespace("detail").Namespace("sal").GlobalNamespace() ) + { + kind = LogCallKind::Sal; // fine + areaArgIndex = 1; + } + else if( tc.Function("DbgUnhandledException").GlobalNamespace() ) + { + kind = LogCallKind::DbgUnhandledException; // ok + areaArgIndex = 3; + } + else + return true; + + // The SAL_DETAIL_LOG_STREAM macro expands to two calls to sal::detail::log(), + // so do not warn repeatedly about the same macro (the area->getLocStart() of all the calls + // from the same macro should be the same). + if( kind == LogCallKind::Sal ) + { + SourceLocation expansionLocation = compiler.getSourceManager().getExpansionLoc( call->getLocStart()); + if( expansionLocation == lastSalDetailLogStreamMacro ) + return true; + lastSalDetailLogStreamMacro = expansionLocation; + }; + if( const clang::StringLiteral* area = dyn_cast< clang::StringLiteral >( call->getArg( areaArgIndex )->IgnoreParenImpCasts())) { - if( func->getNumParams() == 5 && func->getIdentifier() != NULL - && ( func->getName() == "sal_detail_log" || func->getName() == "log" )) + if( area->getKind() == clang::StringLiteral::Ascii ) + checkArea( area->getBytes(), area->getExprLoc()); + else + report( DiagnosticsEngine::Warning, "unsupported string literal kind (plugin needs fixing?)", + area->getLocStart()); + return true; + } + if( kind == LogCallKind::DbgUnhandledException ) // below checks don't apply + return true; + if( loplugin::DeclCheck(inFunction).Function("log").Namespace("detail").Namespace("sal").GlobalNamespace() + || loplugin::DeclCheck(inFunction).Function("sal_detail_logFormat").GlobalNamespace() ) + return true; // These functions only forward to sal_detail_log, so ok. + if( call->getArg( areaArgIndex )->isNullPointerConstant( compiler.getASTContext(), + Expr::NPC_ValueDependentIsNotNull ) != Expr::NPCK_NotNull ) + { // If the area argument is a null pointer, that is allowed only for SAL_DEBUG. + const SourceManager& source = compiler.getSourceManager(); + for( SourceLocation loc = call->getLocStart(); + loc.isMacroID(); + loc = source.getImmediateExpansionRange( loc ).first ) { - auto tc = loplugin::DeclCheck(func); - if( tc.Function("sal_detail_log") || tc.Function("log").Namespace("detail").Namespace("sal").GlobalNamespace() ) - { - // The SAL_DETAIL_LOG_STREAM macro expands to two calls to sal::detail::log(), - // so do not warn repeatedly about the same macro (the area->getLocStart() of all the calls - // from the same macro should be the same). - SourceLocation expansionLocation = compiler.getSourceManager().getExpansionLoc( call->getLocStart()); - if( expansionLocation == lastSalDetailLogStreamMacro ) - return true; - lastSalDetailLogStreamMacro = expansionLocation; - if( const clang::StringLiteral* area = dyn_cast< clang::StringLiteral >( call->getArg( 1 )->IgnoreParenImpCasts())) - { - if( area->getKind() == clang::StringLiteral::Ascii ) - checkArea( area->getBytes(), area->getExprLoc()); - else - report( DiagnosticsEngine::Warning, "unsupported string literal kind (plugin needs fixing?)", - area->getLocStart()); - return true; - } - if( loplugin::DeclCheck(inFunction).Function("log").Namespace("detail").Namespace("sal").GlobalNamespace() - || loplugin::DeclCheck(inFunction).Function("sal_detail_logFormat").GlobalNamespace() ) - return true; // These functions only forward to sal_detail_log, so ok. - if( call->getArg( 1 )->isNullPointerConstant( compiler.getASTContext(), - Expr::NPC_ValueDependentIsNotNull ) != Expr::NPCK_NotNull ) - { // If the area argument is a null pointer, that is allowed only for SAL_DEBUG. - const SourceManager& source = compiler.getSourceManager(); - for( SourceLocation loc = call->getLocStart(); - loc.isMacroID(); - loc = source.getImmediateExpansionRange( loc ).first ) - { - StringRef inMacro = Lexer::getImmediateMacroName( loc, source, compiler.getLangOpts()); - if( inMacro == "SAL_DEBUG" || inMacro == "SAL_DEBUG_BACKTRACE" ) - return true; // ok - } - report( DiagnosticsEngine::Warning, "missing log area", - call->getArg( 1 )->IgnoreParenImpCasts()->getLocStart()); - return true; - } - report( DiagnosticsEngine::Warning, "cannot analyse log area argument (plugin needs fixing?)", - call->getLocStart()); - } + StringRef inMacro = Lexer::getImmediateMacroName( loc, source, compiler.getLangOpts()); + if( inMacro == "SAL_DEBUG" || inMacro == "SAL_DEBUG_BACKTRACE" ) + return true; // ok } + report( DiagnosticsEngine::Warning, "missing log area", + call->getArg( 1 )->IgnoreParenImpCasts()->getLocStart()); + return true; } + report( DiagnosticsEngine::Warning, "cannot analyse log area argument (plugin needs fixing?)", + call->getLocStart()); return true; } diff --git a/compilerplugins/clang/test/sallogareas.cxx b/compilerplugins/clang/test/sallogareas.cxx new file mode 100644 index 000000000000..6a9035a05f30 --- /dev/null +++ b/compilerplugins/clang/test/sallogareas.cxx @@ -0,0 +1,58 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include <tools/diagnose_ex.h> + +void func1(); + +int main() +{ + SAL_WARN( + "bob.none", + "message"); // expected-error@-2 {{unknown log area 'bob.none' (check or extend include/sal/log-areas.dox) [loplugin:sallogareas]}} + + SAL_WARN("xmloff", "message"); + + try + { + func1(); + } + catch (std::exception const&) + { + DBG_UNHANDLED_EXCEPTION( + "bob.none", + "message"); // expected-error@-2 {{unknown log area 'bob.none' (check or extend include/sal/log-areas.dox) [loplugin:sallogareas]}} + } + try + { + func1(); + } + catch (std::exception const&) + { + DBG_UNHANDLED_EXCEPTION("xmloff", "message"); + } + try + { + func1(); + } + catch (std::exception const&) + { + DBG_UNHANDLED_EXCEPTION("xmloff"); + } + try + { + func1(); + } + catch (std::exception const&) + { + DBG_UNHANDLED_EXCEPTION(); + } +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/include/tools/diagnose_ex.h b/include/tools/diagnose_ex.h index df71bfa01711..50a65f01a03f 100644 --- a/include/tools/diagnose_ex.h +++ b/include/tools/diagnose_ex.h @@ -29,10 +29,9 @@ TOOLS_DLLPUBLIC void DbgUnhandledException(const css::uno::Any& caughtException, const char* currentFunction, const char* fileAndLineNo, - const char* explanatory = nullptr); + const char* area = nullptr, const char* explanatory = nullptr); #if OSL_DEBUG_LEVEL > 0 - #include <com/sun/star/configuration/CorruptedConfigurationException.hpp> #include <cppuhelper/exc_hlp.hxx> #include <osl/thread.h> @@ -40,16 +39,25 @@ TOOLS_DLLPUBLIC void DbgUnhandledException(const css::uno::Any& caughtException, Note that whenever you use this, it might be an indicator that your error handling is not correct .... + This takes two optional parameters: area and explanatory */ - #define DBG_UNHANDLED_EXCEPTION() \ - DbgUnhandledException( ::cppu::getCaughtException(), OSL_THIS_FUNC, SAL_DETAIL_WHERE); + #define DBG_UNHANDLED_EXCEPTION_0_ARGS() \ + DbgUnhandledException( ::cppu::getCaughtException(), OSL_THIS_FUNC, SAL_DETAIL_WHERE ); + #define DBG_UNHANDLED_EXCEPTION_1_ARGS(area) \ + DbgUnhandledException( ::cppu::getCaughtException(), OSL_THIS_FUNC, SAL_DETAIL_WHERE, area ); + #define DBG_UNHANDLED_EXCEPTION_2_ARGS(area, explanatory) \ + DbgUnhandledException( ::cppu::getCaughtException(), OSL_THIS_FUNC, SAL_DETAIL_WHERE, area, explanatory ); + + #define DBG_UNHANDLED_FUNC_CHOOSER(_f1, _f2, _f3, ...) _f3 + #define DBG_UNHANDLED_FUNC_RECOMPOSER(argsWithParentheses) DBG_UNHANDLED_FUNC_CHOOSER argsWithParentheses + #define DBG_UNHANDLED_CHOOSE_FROM_ARG_COUNT(...) DBG_UNHANDLED_FUNC_RECOMPOSER((__VA_ARGS__, DBG_UNHANDLED_EXCEPTION_2_ARGS, DBG_UNHANDLED_EXCEPTION_1_ARGS, DBG_UNHANDLED_EXCEPTION_0_ARGS, )) + #define DBG_UNHANDLED_NO_ARG_EXPANDER() ,,DBG_UNHANDLED_EXCEPTION_0_ARGS + #define DBG_UNHANDLED_MACRO_CHOOSER(...) DBG_UNHANDLED_CHOOSE_FROM_ARG_COUNT(DBG_UNHANDLED_NO_ARG_EXPANDER __VA_ARGS__ ()) + #define DBG_UNHANDLED_EXCEPTION(...) DBG_UNHANDLED_MACRO_CHOOSER(__VA_ARGS__)(__VA_ARGS__) - #define DBG_UNHANDLED_EXCEPTION_WHEN(explain) \ - DbgUnhandledException( ::cppu::getCaughtException(), OSL_THIS_FUNC, SAL_DETAIL_WHERE, explain); #else // OSL_DEBUG_LEVEL - #define DBG_UNHANDLED_EXCEPTION() - #define DBG_UNHANDLED_EXCEPTION_WHEN(explain) + #define DBG_UNHANDLED_EXCEPTION(...) #endif // OSL_DEBUG_LEVEL /** This macro asserts the given condition (in debug mode), and throws diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk index d417702d19fe..5187f60beb2f 100644 --- a/solenv/CompilerTest_compilerplugins_clang.mk +++ b/solenv/CompilerTest_compilerplugins_clang.mk @@ -42,6 +42,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \ compilerplugins/clang/test/refcounting \ compilerplugins/clang/test/salbool \ compilerplugins/clang/test/salcall \ + compilerplugins/clang/test/sallogareas \ compilerplugins/clang/test/salunicodeliteral \ compilerplugins/clang/test/simplifybool \ compilerplugins/clang/test/simplifydynamiccast \ diff --git a/tools/source/debug/debug.cxx b/tools/source/debug/debug.cxx index 6725670f82ce..440c73418ed9 100644 --- a/tools/source/debug/debug.cxx +++ b/tools/source/debug/debug.cxx @@ -76,7 +76,7 @@ void DbgTestSolarMutex() #endif void DbgUnhandledException(const css::uno::Any & caught, const char* currentFunction, const char* fileAndLineNo, - const char* explanatory) + const char* area, const char* explanatory) { OString sMessage( "DBG_UNHANDLED_EXCEPTION in " ); sMessage += currentFunction; @@ -120,9 +120,12 @@ void DbgUnhandledException(const css::uno::Any & caught, const char* currentFunc } sMessage += "\n"; + if (area == nullptr) + area = "legacy.osl"; + SAL_DETAIL_LOG_FORMAT( SAL_DETAIL_ENABLE_LOG_WARN, SAL_DETAIL_LOG_LEVEL_WARN, - "legacy.osl", fileAndLineNo, "%s", sMessage.getStr()); + area, fileAndLineNo, "%s", sMessage.getStr()); } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/xmloff/source/draw/shapeexport.cxx b/xmloff/source/draw/shapeexport.cxx index b0422e8fde8b..fd2ad7ca647c 100644 --- a/xmloff/source/draw/shapeexport.cxx +++ b/xmloff/source/draw/shapeexport.cxx @@ -554,7 +554,7 @@ void XMLShapeExport::collectShapeAutoStyles(const uno::Reference< drawing::XShap } catch(const uno::Exception&) { - DBG_UNHANDLED_EXCEPTION_WHEN( "collecting auto styles for a table" ); + DBG_UNHANDLED_EXCEPTION( "xmloff", "collecting auto styles for a table" ); } break; } @@ -746,7 +746,7 @@ void XMLShapeExport::exportShape(const uno::Reference< drawing::XShape >& xShape } catch(const uno::Exception&) { - DBG_UNHANDLED_EXCEPTION_WHEN( "exporting layer name for shape" ); + DBG_UNHANDLED_EXCEPTION( "xmloff", "exporting layer name for shape" ); } } } @@ -1867,7 +1867,7 @@ void XMLShapeExport::ImpExportDescription( const uno::Reference< drawing::XShape } catch( uno::Exception& ) { - DBG_UNHANDLED_EXCEPTION_WHEN( "exporting Title and/or Description for shape" ); + DBG_UNHANDLED_EXCEPTION( "xmloff", "exporting Title and/or Description for shape" ); } } diff --git a/xmloff/source/draw/ximpshap.cxx b/xmloff/source/draw/ximpshap.cxx index b48c92d3c577..edcbcf4af1bc 100644 --- a/xmloff/source/draw/ximpshap.cxx +++ b/xmloff/source/draw/ximpshap.cxx @@ -330,7 +330,7 @@ void SdXMLShapeContext::addGluePoint( const uno::Reference< xml::sax::XAttribute } catch(const uno::Exception&) { - DBG_UNHANDLED_EXCEPTION_WHEN( "during setting of glue points"); + DBG_UNHANDLED_EXCEPTION( "xmloff", "during setting of glue points"); } } } @@ -401,7 +401,7 @@ void SdXMLShapeContext::EndElement() } catch(const Exception&) { - DBG_UNHANDLED_EXCEPTION_WHEN("while setting hyperlink"); + DBG_UNHANDLED_EXCEPTION("xmloff", "while setting hyperlink"); } if( mxLockable.is() ) @@ -443,7 +443,7 @@ void SdXMLShapeContext::AddShape(uno::Reference< drawing::XShape >& xShape) } catch(const Exception&) { - DBG_UNHANDLED_EXCEPTION_WHEN( "while setting visible or printable" ); + DBG_UNHANDLED_EXCEPTION( "xmloff", "while setting visible or printable" ); } if(!mbTemporaryShape && (!GetImport().HasTextImport() @@ -678,7 +678,7 @@ void SdXMLShapeContext::SetStyle( bool bSupportsStyle /* = true */) } catch(const uno::Exception&) { - DBG_UNHANDLED_EXCEPTION_WHEN( "finding style for shape" ); + DBG_UNHANDLED_EXCEPTION( "xmloff", "finding style for shape" ); } } @@ -691,7 +691,7 @@ void SdXMLShapeContext::SetStyle( bool bSupportsStyle /* = true */) } catch(const uno::Exception&) { - DBG_UNHANDLED_EXCEPTION_WHEN( "setting style for shape" ); + DBG_UNHANDLED_EXCEPTION( "xmloff", "setting style for shape" ); } } @@ -986,7 +986,7 @@ void SdXMLRectShapeContext::StartElement(const uno::Reference< xml::sax::XAttrib } catch(const uno::Exception&) { - DBG_UNHANDLED_EXCEPTION_WHEN( "setting corner radius"); + DBG_UNHANDLED_EXCEPTION( "xmloff", "setting corner radius"); } } } @@ -1647,7 +1647,7 @@ void SdXMLTextBoxShapeContext::StartElement(const uno::Reference< xml::sax::XAtt } catch(const uno::Exception&) { - DBG_UNHANDLED_EXCEPTION_WHEN( "setting corner radius"); + DBG_UNHANDLED_EXCEPTION( "xmloff", "setting corner radius"); } } } @@ -1664,7 +1664,7 @@ void SdXMLTextBoxShapeContext::StartElement(const uno::Reference< xml::sax::XAtt } catch(const uno::Exception&) { - DBG_UNHANDLED_EXCEPTION_WHEN( "setting name of next chain link"); + DBG_UNHANDLED_EXCEPTION( "xmloff", "setting name of next chain link"); } } } @@ -2300,7 +2300,7 @@ void SdXMLCaptionShapeContext::StartElement(const uno::Reference< xml::sax::XAtt } catch(const uno::Exception&) { - DBG_UNHANDLED_EXCEPTION_WHEN( "setting corner radius"); + DBG_UNHANDLED_EXCEPTION( "xmloff", "setting corner radius"); } } } @@ -3352,7 +3352,7 @@ void SdXMLFrameShapeContext::removeGraphicFromImportContext(const SvXMLImportCon } catch( uno::Exception& ) { - DBG_UNHANDLED_EXCEPTION_WHEN( "Error in cleanup of multiple graphic object import." ); + DBG_UNHANDLED_EXCEPTION( "xmloff", "Error in cleanup of multiple graphic object import." ); } } } @@ -3386,7 +3386,7 @@ uno::Reference<graphic::XGraphic> SdXMLFrameShapeContext::getGraphicFromImportCo } catch( uno::Exception& ) { - DBG_UNHANDLED_EXCEPTION_WHEN("Error in cleanup of multiple graphic object import."); + DBG_UNHANDLED_EXCEPTION("xmloff", "Error in cleanup of multiple graphic object import."); } return xGraphic; @@ -3410,7 +3410,7 @@ OUString SdXMLFrameShapeContext::getGraphicPackageURLFromImportContext(const SvX } catch( uno::Exception& ) { - DBG_UNHANDLED_EXCEPTION_WHEN( "Error in cleanup of multiple graphic object import." ); + DBG_UNHANDLED_EXCEPTION( "xmloff", "Error in cleanup of multiple graphic object import." ); } } @@ -3713,7 +3713,7 @@ void SdXMLCustomShapeContext::StartElement( const uno::Reference< xml::sax::XAtt } catch(const uno::Exception&) { - DBG_UNHANDLED_EXCEPTION_WHEN( "setting enhanced customshape geometry" ); + DBG_UNHANDLED_EXCEPTION( "xmloff", "setting enhanced customshape geometry" ); } SdXMLShapeContext::StartElement(xAttrList); } @@ -3796,7 +3796,7 @@ void SdXMLCustomShapeContext::EndElement() } catch(const uno::Exception&) { - DBG_UNHANDLED_EXCEPTION_WHEN( "setting enhanced customshape geometry" ); + DBG_UNHANDLED_EXCEPTION( "xmloff", "setting enhanced customshape geometry" ); } sal_Int32 nUPD; @@ -3834,7 +3834,7 @@ void SdXMLCustomShapeContext::EndElement() } catch(const uno::Exception&) { - DBG_UNHANDLED_EXCEPTION_WHEN("flushing after load"); + DBG_UNHANDLED_EXCEPTION("xmloff", "flushing after load"); } } |