basic/CppunitTest_basic_macros.mk | 1 basic/qa/cppunit/test_global_as_new.cxx | 86 ++++++++++++++++++++++++++++++++ basic/source/comp/dim.cxx | 27 ++++++++++ basic/source/inc/runtime.hxx | 1 basic/source/runtime/runtime.cxx | 34 ++++++++++-- 5 files changed, 143 insertions(+), 6 deletions(-)
New commits: commit b80069e133d30e80e50a792b2bc1d0c2f9a31bfa Author: John <j...@gmx.us> AuthorDate: Thu Mar 4 01:50:25 2021 -0500 Commit: Mike Kaganski <mike.kagan...@collabora.com> CommitDate: Sun Mar 21 21:54:11 2021 +0100 tdf#88442 SBasic: Don't double-initialize a Global ... As New ... "As New" variables use lazy instantiation, but that should not re-apply each time one re-enters the module. If the object is still there, don't replace it. Change-Id: Ic568a56b93db9bc9139d434d97a4a4deb05840df Reviewed-on: https://gerrit.libreoffice.org/c/core/+/112018 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com> diff --git a/basic/CppunitTest_basic_macros.mk b/basic/CppunitTest_basic_macros.mk index 6cce94737113..c2c1eb5b7a04 100644 --- a/basic/CppunitTest_basic_macros.mk +++ b/basic/CppunitTest_basic_macros.mk @@ -19,6 +19,7 @@ $(eval $(call gb_CppunitTest_add_exception_objects,basic_macros, \ basic/qa/cppunit/test_language_conditionals \ basic/qa/cppunit/test_nested_struct \ basic/qa/cppunit/test_vba \ + basic/qa/cppunit/test_global_as_new \ )) $(eval $(call gb_CppunitTest_use_libraries,basic_macros, \ diff --git a/basic/qa/cppunit/test_global_as_new.cxx b/basic/qa/cppunit/test_global_as_new.cxx new file mode 100644 index 000000000000..db64a974cb0f --- /dev/null +++ b/basic/qa/cppunit/test_global_as_new.cxx @@ -0,0 +1,86 @@ +/* -*- 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 <basic/sbstar.hxx> +#include <basic/sbmeth.hxx> +#include <basic/basrdll.hxx> +#include <cppunit/TestSuite.h> +#include <cppunit/plugin/TestPlugIn.h> +#include <cppunit/extensions/HelperMacros.h> +#include <iostream> + +namespace +{ +class GlobalAsNewTest : public CppUnit::TestFixture +{ + void testMaintainsValueAcrossCalls(); + + CPPUNIT_TEST_SUITE(GlobalAsNewTest); + CPPUNIT_TEST(testMaintainsValueAcrossCalls); + CPPUNIT_TEST_SUITE_END(); + + BasicDLL lib; + StarBASICRef interpreter; + + SbModuleRef Module() + { + interpreter = new StarBASIC(); + auto mod = interpreter->MakeModule("GlobalAsNew", R"BAS( +Global aDate As New "com.sun.star.util.Date" + +Function GetDateAsString As String + DIM local_Date As New "com.sun.star.util.Date" + GetDateAsString = TRIM(STR(aDate.Year)) + "-" + TRIM(STR(local_Date.Year)) + TRIM(STR(aDate.Month)) + "-" + TRIM(STR(aDate.Day)) +End Function + +Function SetDate + aDate.Month = 6 + aDate.Day = 30 + aDate.Year = 2019 + SetDate = GetDateAsString() +End Function + + )BAS"); + CPPUNIT_ASSERT(mod->Compile()); + CPPUNIT_ASSERT_EQUAL(StarBASIC::GetErrBasic(), ERRCODE_NONE); + CPPUNIT_ASSERT_EQUAL(SbxBase::GetError(), ERRCODE_NONE); + CPPUNIT_ASSERT(mod->IsCompiled()); + return mod; + } +}; + +void GlobalAsNewTest::testMaintainsValueAcrossCalls() +{ + auto m = Module(); + auto GetDateAsString = m->FindMethod("GetDateAsString", SbxClassType::Method); + CPPUNIT_ASSERT_MESSAGE("Could not Find GetDateAsString in module", GetDateAsString != nullptr); + + // There is no SbxMethod::call(), the basic code is exercised here in the copy ctor + SbxVariableRef returned = new SbxMethod{ *GetDateAsString }; + CPPUNIT_ASSERT(returned->IsString()); + //0-00-0 is the result of reading the default-initialized date + CPPUNIT_ASSERT_EQUAL(OUString{ "0-00-0" }, returned->GetOUString()); + + auto SetDate = m->FindMethod("SetDate", SbxClassType::Method); + CPPUNIT_ASSERT_MESSAGE("Could not Find SetDate in module", SetDate != nullptr); + returned = new SbxMethod{ *SetDate }; + CPPUNIT_ASSERT(returned->IsString()); + OUString set_val("2019-06-30"); + CPPUNIT_ASSERT_EQUAL(set_val, returned->GetOUString()); + + returned = new SbxMethod{ *GetDateAsString }; + CPPUNIT_ASSERT(returned->IsString()); + //tdf#88442 The global should have maintained its state! + CPPUNIT_ASSERT_EQUAL(set_val, returned->GetOUString()); +} + +// Put the test suite in the registry +CPPUNIT_TEST_SUITE_REGISTRATION(GlobalAsNewTest); + +} // namespace diff --git a/basic/source/comp/dim.cxx b/basic/source/comp/dim.cxx index 20396cd729ad..cf14d1c7818c 100644 --- a/basic/source/comp/dim.cxx +++ b/basic/source/comp/dim.cxx @@ -446,12 +446,39 @@ void SbiParser::DefVar( SbiOpcode eOp, bool bStatic ) { SbiExpression aExpr( this, *pDef ); aExpr.Gen(); + + /* tdf#88442 + * Don't initialize a + * Global X as New SomeObjectType + * if it has already been initialized. + * This approach relies on JUMPT evaluating Object->NULL as being 'false' + * But the effect of this code is similar to inserting + * If IsNull(YourGlobal) + * Set YourGlobal = ' new obj + * End If ' If IsNull(YourGlobal) + * Only for globals. For locals that check is skipped as it's unnecessary + */ + sal_uInt32 come_from = 0; + if ( pDef->GetScope() == SbGLOBAL ) + { + come_from = aGen.Gen( SbiOpcode::JUMPT_, 0 ); + aGen.Gen( SbiOpcode::FIND_, pDef->GetId(), pDef->GetTypeId() ); + } + SbiOpcode eOp_ = pDef->IsNew() ? SbiOpcode::CREATE_ : SbiOpcode::TCREATE_; aGen.Gen( eOp_, pDef->GetId(), pDef->GetTypeId() ); if ( bVBASupportOn ) aGen.Gen( SbiOpcode::VBASET_ ); else aGen.Gen( SbiOpcode::SET_ ); + + if ( come_from ) + { + // See other tdf#88442 comment above where come_from is + // initialized. This is effectively 'inserting' the + // End If ' If IsNull(YourGlobal) + aGen.BackChain( come_from ); + } } } else diff --git a/basic/source/inc/runtime.hxx b/basic/source/inc/runtime.hxx index 1169820e753b..73e56838e2aa 100644 --- a/basic/source/inc/runtime.hxx +++ b/basic/source/inc/runtime.hxx @@ -288,6 +288,7 @@ class SbiRuntime // #56204 swap out DIM-functionality into help method (step0.cxx) void DimImpl(const SbxVariableRef& refVar); + bool EvaluateTopOfStackAsBool(); static bool implIsClass( SbxObject const * pObj, const OUString& aClass ); diff --git a/basic/source/runtime/runtime.cxx b/basic/source/runtime/runtime.cxx index 9de5da64d5f2..d1dcd3d74502 100644 --- a/basic/source/runtime/runtime.cxx +++ b/basic/source/runtime/runtime.cxx @@ -2975,24 +2975,46 @@ void SbiRuntime::StepJUMP( sal_uInt32 nOp1 ) pCode = pImg->GetCode() + nOp1; } +bool SbiRuntime::EvaluateTopOfStackAsBool() +{ + SbxVariableRef tos = PopVar(); + // In a test e.g. If Null then + // will evaluate Null will act as if False + if ( bVBAEnabled && tos->IsNull() ) + { + return false; + } + if ( tos->IsObject() ) + { + //GetBool applied to an Object attempts to dereference and evaluate + //the underlying value as Bool. Here, we're checking rather that + //it is not null + return tos->GetObject(); + } + else + { + return tos->GetBool(); + } +} + // evaluate TOS, conditional jump (+target) void SbiRuntime::StepJUMPT( sal_uInt32 nOp1 ) { - SbxVariableRef p = PopVar(); - if( p->GetBool() ) + if ( EvaluateTopOfStackAsBool() ) + { StepJUMP( nOp1 ); + } } // evaluate TOS, conditional jump (+target) void SbiRuntime::StepJUMPF( sal_uInt32 nOp1 ) { - SbxVariableRef p = PopVar(); - // In a test e.g. If Null then - // will evaluate Null will act as if False - if( ( bVBAEnabled && p->IsNull() ) || !p->GetBool() ) + if ( !EvaluateTopOfStackAsBool() ) + { StepJUMP( nOp1 ); + } } // evaluate TOS, jump into JUMP-table (+MaxVal) _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits