mirror of
https://github.com/OPM/ResInsight.git
synced 2026-10-25 06:38:13 -05:00
#14778 Fix crashes from unguarded pointer and index accesses
* Skip deleted plot widgets in RiuMultiPlotBook::visiblePlotWidgets * Guard missing calculation in detectCyclicCalculation * Guard invalid node index in RigFemPart node lookups * Guard null well path in RimWellLogExtractionCurve::extractGeomData * Guard unopened case in RimGridCalculation::removeDependentObjects * Guard empty subset in createLateralAtMeasuredDepth
This commit is contained in:
1 parent
4d2a868079
commit
3437cf9b94
12 files changed
+129
-14
No files matched your search
+7
-3
@@ -94,9 +94,13 @@ RimModeledWellPath* RicNewWellPathLateralAtDepthFeature::createLateralAtMeasured
|
||||
parentWellPath->wellPathGeometry()->clippedPointSubset( parentWellPath->wellPathGeometry()->measuredDepths().front(),
|
||||
parentWellMD );
|
||||
|
||||
newModeledWellPath->geometryDefinition()->setMdAtFirstTarget( measuredDepths.back() );
|
||||
newModeledWellPath->geometryDefinition()->setFixedWellPathPoints( pointVector );
|
||||
newModeledWellPath->geometryDefinition()->setFixedMeasuredDepths( measuredDepths );
|
||||
// The subset is empty if the tie-in depth is above the start of the parent well path
|
||||
if ( !measuredDepths.empty() )
|
||||
{
|
||||
newModeledWellPath->geometryDefinition()->setMdAtFirstTarget( measuredDepths.back() );
|
||||
newModeledWellPath->geometryDefinition()->setFixedWellPathPoints( pointVector );
|
||||
newModeledWellPath->geometryDefinition()->setFixedMeasuredDepths( measuredDepths );
|
||||
}
|
||||
}
|
||||
|
||||
newModeledWellPath->geometryDefinition()->setIsAttachedToParentWell( true );
|
||||
|
||||
@@ -276,16 +276,21 @@ void RigFemPart::calculateNodeToElmRefs()
|
||||
//--------------------------------------------------------------------------------------------------
|
||||
///
|
||||
//--------------------------------------------------------------------------------------------------
|
||||
const std::vector<int>& RigFemPart::elementsUsingNode( int nodeIndex ) const
|
||||
std::vector<int> RigFemPart::elementsUsingNode( int nodeIndex ) const
|
||||
{
|
||||
// Elements referring to nodes outside the node list, e.g. a part with elements but no nodes, give an invalid index
|
||||
if ( nodeIndex < 0 || nodeIndex >= static_cast<int>( m_nodeToElmRefs.size() ) ) return {};
|
||||
|
||||
return m_nodeToElmRefs[nodeIndex];
|
||||
}
|
||||
|
||||
//--------------------------------------------------------------------------------------------------
|
||||
///
|
||||
//--------------------------------------------------------------------------------------------------
|
||||
const std::vector<unsigned char>& RigFemPart::elementLocalIndicesForNode( int nodeIndex ) const
|
||||
std::vector<unsigned char> RigFemPart::elementLocalIndicesForNode( int nodeIndex ) const
|
||||
{
|
||||
if ( nodeIndex < 0 || nodeIndex >= static_cast<int>( m_nodeGlobalToLocalIndices.size() ) ) return {};
|
||||
|
||||
return m_nodeGlobalToLocalIndices[nodeIndex];
|
||||
}
|
||||
|
||||
|
||||
@@ -75,9 +75,9 @@ public:
|
||||
RigFemPartNodes& nodes();
|
||||
const RigFemPartNodes& nodes() const;
|
||||
|
||||
void assertNodeToElmIndicesIsCalculated();
|
||||
const std::vector<int>& elementsUsingNode( int nodeIndex ) const;
|
||||
const std::vector<unsigned char>& elementLocalIndicesForNode( int nodeIndex ) const;
|
||||
void assertNodeToElmIndicesIsCalculated();
|
||||
std::vector<int> elementsUsingNode( int nodeIndex ) const;
|
||||
std::vector<unsigned char> elementLocalIndicesForNode( int nodeIndex ) const;
|
||||
|
||||
void assertElmNeighborsIsCalculated();
|
||||
int elementNeighbor( int elementIndex, int faceIndex ) const;
|
||||
|
||||
@@ -989,8 +989,12 @@ void RimGridCalculation::removeDependentObjects()
|
||||
}
|
||||
}
|
||||
|
||||
eclipseCase->results( porosityModel )->clearScalarResult( resAddr );
|
||||
eclipseCase->results( porosityModel )->setRemovedTagOnGeneratedResult( resAddr );
|
||||
// The results are not available if the case is not opened
|
||||
if ( auto results = eclipseCase->results( porosityModel ) )
|
||||
{
|
||||
results->clearScalarResult( resAddr );
|
||||
results->setRemovedTagOnGeneratedResult( resAddr );
|
||||
}
|
||||
|
||||
RimReloadCaseTools::updateAll3dViews( eclipseCase );
|
||||
}
|
||||
|
||||
@@ -178,12 +178,17 @@ bool RimSummaryCalculation::detectCyclicCalculation( int id, std::set<int>& ids
|
||||
RimSummaryCalculationCollection* calcColl = RimProject::current()->calculationCollection();
|
||||
auto calc = dynamic_cast<RimSummaryCalculation*>( calcColl->findCalculationById( id ) );
|
||||
|
||||
// The referenced calculation is already removed from the collection when it is being deleted
|
||||
if ( !calc ) return false;
|
||||
|
||||
// Check if any of the variables references already seen calculations
|
||||
auto vars = calc->variables();
|
||||
for ( size_t i = 0; i < vars->size(); i++ )
|
||||
{
|
||||
auto variable = dynamic_cast<RimSummaryCalculationVariable*>( vars->at( i ) );
|
||||
auto addr = variable->summaryAddress()->address();
|
||||
if ( !variable || !variable->summaryAddress() ) continue;
|
||||
|
||||
auto addr = variable->summaryAddress()->address();
|
||||
|
||||
if ( addr.id() != -1 && detectCyclicCalculation( addr.id(), ids ) ) return true;
|
||||
}
|
||||
|
||||
@@ -621,7 +621,7 @@ RimWellLogExtractionCurve::WellLogExtractionCurveData
|
||||
if ( maxDistanceBetweenCurvePoints.has_value() && maxDistanceBetweenCurvePoints.value() > 0.0 )
|
||||
{
|
||||
RigGeoMechCaseData* caseData = geoMechCase->geoMechData();
|
||||
auto wellPathGeometry = m_wellPath->wellPathGeometry();
|
||||
auto wellPathGeometry = m_wellPath ? m_wellPath->wellPathGeometry() : nullptr;
|
||||
if ( caseData && wellPathGeometry )
|
||||
{
|
||||
std::string errorIdName = ( m_wellPath->name() + " " + geoMechCase->caseUserDescription() ).toStdString();
|
||||
|
||||
@@ -162,6 +162,8 @@ set(SOURCE_UNITTEST_FILES
|
||||
${CMAKE_CURRENT_LIST_DIR}/RiaNameUniquenessTools-Test.cpp
|
||||
${CMAKE_CURRENT_LIST_DIR}/RifVtkSurfaceImporter-Test.cpp
|
||||
${CMAKE_CURRENT_LIST_DIR}/RifVtkReader-Test.cpp
|
||||
${CMAKE_CURRENT_LIST_DIR}/RigFemPart-Test.cpp
|
||||
${CMAKE_CURRENT_LIST_DIR}/RicNewWellPathLateralAtDepthFeature-Test.cpp
|
||||
${CMAKE_CURRENT_LIST_DIR}/RifVtkImportUtil-Test.cpp
|
||||
${CMAKE_CURRENT_LIST_DIR}/RiaKeyValueStore-Test.cpp
|
||||
${CMAKE_CURRENT_LIST_DIR}/RiaEnsembleImportTools-Test.cpp
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
#include "gtest/gtest.h"
|
||||
|
||||
#include "WellPathCommands/RicNewWellPathLateralAtDepthFeature.h"
|
||||
|
||||
#include "RiaApplication.h"
|
||||
|
||||
#include "Well/RigWellPath.h"
|
||||
|
||||
#include "RimModeledWellPath.h"
|
||||
#include "RimWellPath.h"
|
||||
|
||||
#include "cvfVector3.h"
|
||||
|
||||
#include <vector>
|
||||
|
||||
//--------------------------------------------------------------------------------------------------
|
||||
/// A tie-in depth above the start of the parent well path, like the default depth of the Python
|
||||
/// interface, gives an empty subset of the parent well path. The lateral must still be created.
|
||||
//--------------------------------------------------------------------------------------------------
|
||||
TEST( RicNewWellPathLateralAtDepthFeature, TieInDepthAboveStartOfParent )
|
||||
{
|
||||
RimWellPath parentWellPath;
|
||||
|
||||
const std::vector<cvf::Vec3d> points = { { 0.0, 0.0, 0.0 }, { 0.0, 0.0, -100.0 }, { 0.0, 0.0, -200.0 } };
|
||||
const std::vector<double> measuredDepths = { 100.0, 200.0, 300.0 };
|
||||
|
||||
auto* geometry = new RigWellPath;
|
||||
geometry->setWellPathPoints( points, measuredDepths );
|
||||
parentWellPath.setWellPathGeometry( geometry );
|
||||
|
||||
RimModeledWellPath* lateral = RicNewWellPathLateralAtDepthFeature::createLateralAtMeasuredDepth( &parentWellPath, 0.0 );
|
||||
EXPECT_TRUE( lateral != nullptr );
|
||||
|
||||
RiaApplication::instance()->closeProject();
|
||||
}
|
||||
@@ -0,0 +1,24 @@
|
||||
#include "gtest/gtest.h"
|
||||
|
||||
#include "RigFemPart.h"
|
||||
|
||||
//--------------------------------------------------------------------------------------------------
|
||||
/// A part with elements but no nodes, as read from a malformed model, gives connectivities that are
|
||||
/// outside the node list. Calculating neighbors must not read outside the node to element references.
|
||||
//--------------------------------------------------------------------------------------------------
|
||||
TEST( RigFemPart, ElmNeighbors_ElementsWithoutNodes )
|
||||
{
|
||||
RigFemPart part;
|
||||
|
||||
const int connectivities[8] = { 0, 1, 2, 3, 4, 5, 6, 7 };
|
||||
part.preAllocateElementStorage( 1 );
|
||||
part.appendElement( RigElementType::HEX8, 1, connectivities );
|
||||
|
||||
part.assertNodeToElmIndicesIsCalculated();
|
||||
part.assertElmNeighborsIsCalculated();
|
||||
|
||||
EXPECT_TRUE( part.elementsUsingNode( 0 ).empty() );
|
||||
EXPECT_TRUE( part.elementsUsingNode( -1 ).empty() );
|
||||
EXPECT_TRUE( part.elementLocalIndicesForNode( 0 ).empty() );
|
||||
EXPECT_TRUE( part.elementLocalIndicesForNode( -1 ).empty() );
|
||||
}
|
||||
@@ -116,3 +116,25 @@ TEST( RimGridCalculationTest, AdditionalEnsembleCasesAreOutputs )
|
||||
EXPECT_EQ( firstCase, outputCases[0] );
|
||||
EXPECT_EQ( secondCase, outputCases[1] );
|
||||
}
|
||||
|
||||
//--------------------------------------------------------------------------------------------------
|
||||
/// The results of a case that is not opened are not available, and must not be accessed when the calculation is removed
|
||||
//--------------------------------------------------------------------------------------------------
|
||||
TEST( RimGridCalculationTest, RemoveDependentObjectsForCaseNotOpened )
|
||||
{
|
||||
RimReservoirGridEnsemble ensemble;
|
||||
auto* notOpenedCase = new RimEclipseResultCase;
|
||||
ensemble.addCase( notOpenedCase );
|
||||
|
||||
RimGridCalculation calculation;
|
||||
calculation.setExpression( "MY_CALCULATION := 1" );
|
||||
|
||||
auto* destinationEnsembleField =
|
||||
dynamic_cast<caf::PdmPtrField<RimReservoirGridEnsemble*>*>( calculation.findField( "DestinationEnsemble" ) );
|
||||
ASSERT_TRUE( destinationEnsembleField != nullptr );
|
||||
destinationEnsembleField->setValue( &ensemble );
|
||||
|
||||
ASSERT_TRUE( notOpenedCase->results( RiaDefines::PorosityModelType::MATRIX_MODEL ) == nullptr );
|
||||
|
||||
calculation.removeDependentObjects();
|
||||
}
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
#include "RifEclipseSummaryAddress.h"
|
||||
#include "RifEclipseSummaryAddressDefines.h"
|
||||
#include "RimProject.h"
|
||||
#include "RimSummaryCalculation.h"
|
||||
|
||||
using SummaryCategory = RifEclipseSummaryAddressDefines::SummaryCategory;
|
||||
@@ -28,6 +29,8 @@ public:
|
||||
{
|
||||
return singleAddressesForCategory( address );
|
||||
}
|
||||
|
||||
bool testDetectCyclicCalculation( int id, std::set<int>& ids ) const { return detectCyclicCalculation( id, ids ); }
|
||||
};
|
||||
|
||||
//--------------------------------------------------------------------------------------------------
|
||||
@@ -217,3 +220,15 @@ TEST( RimSummaryCalculation, SubstituteVariables_BlockLgr )
|
||||
EXPECT_EQ( 20, vars[0].summaryAddress.cellJ() );
|
||||
EXPECT_EQ( 30, vars[0].summaryAddress.cellK() );
|
||||
}
|
||||
|
||||
//--------------------------------------------------------------------------------------------------
|
||||
/// A calculation referring to a calculation that is already removed from the collection, as when the
|
||||
/// referenced calculation is being deleted, is not cyclic
|
||||
//--------------------------------------------------------------------------------------------------
|
||||
TEST( RimSummaryCalculation, DetectCyclicCalculation_MissingCalculation )
|
||||
{
|
||||
RimSummaryCalculationTester calculation;
|
||||
|
||||
std::set<int> ids;
|
||||
EXPECT_FALSE( calculation.testDetectCyclicCalculation( 9999, ids ) );
|
||||
}
|
||||
@@ -565,8 +565,7 @@ QList<QPointer<RiuPlotWidget>> RiuMultiPlotBook::visiblePlotWidgets() const
|
||||
QList<QPointer<RiuPlotWidget>> plotWidgets;
|
||||
for ( QPointer<RiuPlotWidget> plotWidget : m_plotWidgets )
|
||||
{
|
||||
CAF_ASSERT( plotWidget );
|
||||
if ( plotWidget->isChecked() )
|
||||
if ( plotWidget && plotWidget->isChecked() )
|
||||
{
|
||||
plotWidgets.push_back( plotWidget );
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user