Skip to content

Fix crash when re-entering exhausted generator after earlier sections #3237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
akasakariko wants to merge 1 commit into
base: devel
Choose a base branch
from
+421 −29
Open

Fix crash when re-entering exhausted generator after earlier sections #3237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 58 additions & 9 deletions src/catch2/internal/catch_run_context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ namespace Catch {
// Filtered generator has moved to specific index due to
// a filter, it needs special handling of `countedNext()`
bool m_isFiltered = false;
// Tracks whether countedNext() returned false so we know
// m_generator is exhausted and cannot be dereferenced.
bool m_isExhausted = false;

GeneratorTracker(
TestCaseTracking::NameAndLocation&& nameAndLocation,
Expand Down Expand Up @@ -119,14 +122,39 @@ namespace Catch {
return nullptr;
}

if ( !tracker->isComplete() ) { tracker->open(); }
if ( !tracker->isComplete() ) {
tracker->open();
} else if ( tracker->m_isExhausted ) {
return nullptr;
} else {
ctx.setCurrentTracker( tracker );
}

return tracker;
}

void resetGenerator( GeneratorBasePtr&& generator ) {
m_generator = CATCH_MOVE( generator );
m_isExhausted = false;
m_ctx.setCurrentTracker( this );
}

void enterAsCompleted() {
m_runState = CompletedSuccessfully;
m_ctx.setCurrentTracker( this );
}

// TrackerBase interface
bool isGeneratorTracker() const override { return true; }
void close() override {
if ( m_runState == CompletedSuccessfully ) {
while ( &m_ctx.currentTracker() != this ) {
m_ctx.currentTracker().close();
}
m_ctx.setCurrentTracker( m_parent );
m_ctx.completeCycle();
return;
}
TrackerBase::close();
// If a generator has a child (it is followed by a section)
// and none of its children have started, then we must wait
Expand Down Expand Up @@ -183,12 +211,17 @@ namespace Catch {
// value, but we do not want to invoke the side-effect if
// this generator is still waiting for any child to start.
assert( m_generator && "Tracker without generator" );
if ( should_wait_for_child
|| ( m_runState == CompletedSuccessfully
&& !m_isFiltered // filtered generators cannot meaningfully move forward, as they would get past the filter
&& m_generator->countedNext() ) ) {
if ( should_wait_for_child ) {
m_children.clear();
m_runState = Executing;
} else if ( m_runState == CompletedSuccessfully &&
!m_isFiltered ) { // filtered generators cannot meaningfully move forward, as they would get past the filter
if ( m_generator->countedNext() ) {
m_children.clear();
m_runState = Executing;
} else {
m_isExhausted = true;
}
}
}

Expand Down Expand Up @@ -542,15 +575,26 @@ namespace Catch {
SourceLineInfo lineInfo,
Generators::GeneratorBasePtr&& generator ) {

auto nameAndLoc = TestCaseTracking::NameAndLocation( static_cast<std::string>( generatorName ), lineInfo );
auto nameAndLocRef = TestCaseTracking::NameAndLocationRef( generatorName, lineInfo );
auto& currentTracker = m_trackerContext.currentTracker();
assert(
currentTracker.nameAndLocation() != nameAndLoc &&
!( currentTracker.nameAndLocation() == nameAndLocRef ) &&
"Trying to create tracker for a generator that already has one" );

if ( auto* existing = currentTracker.findChild( nameAndLocRef ) ) {
assert( existing->isGeneratorTracker() );
assert( existing->isComplete() );
auto* genTracker =
static_cast<Generators::GeneratorTracker*>( existing );
assert( genTracker->m_isExhausted );
genTracker->resetGenerator( CATCH_MOVE( generator ) );
return genTracker;
}

auto newTracker =
Catch::Detail::make_unique<Generators::GeneratorTracker>(
CATCH_MOVE( nameAndLoc ),
TestCaseTracking::NameAndLocation(
static_cast<std::string>( generatorName ), lineInfo ),
m_trackerContext,
&currentTracker,
CATCH_MOVE( generator ) );
Expand All @@ -568,9 +612,14 @@ namespace Catch {
}

auto returnPtr = newTracker.get();
const bool parentAlreadyComplete = currentTracker.isComplete();
currentTracker.addChild( CATCH_MOVE( newTracker ) );

returnPtr->open();
if ( parentAlreadyComplete ) {
returnPtr->enterAsCompleted();
} else {
returnPtr->open();
}
return returnPtr;
}

Expand Down
1 change: 1 addition & 0 deletions tests/SelfTest/Baselines/automake.sw.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Nor would this
:test-result: PASS #2152 - ULP checks between differently signed values were wrong - double
:test-result: PASS #2152 - ULP checks between differently signed values were wrong - float
:test-result: XFAIL #2615 - Throwing in constructor generator fails test case but does not abort
:test-result: PASS #2809 - Sections taking more cycles than following GENERATE
:test-result: PASS #3001: Enum-based bitfields can be captured
:test-result: XFAIL #748 - captures with unexpected exceptions
:test-result: PASS #809
Expand Down
1 change: 1 addition & 0 deletions tests/SelfTest/Baselines/automake.sw.multi.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
:test-result: PASS #2152 - ULP checks between differently signed values were wrong - double
:test-result: PASS #2152 - ULP checks between differently signed values were wrong - float
:test-result: XFAIL #2615 - Throwing in constructor generator fails test case but does not abort
:test-result: PASS #2809 - Sections taking more cycles than following GENERATE
:test-result: PASS #3001: Enum-based bitfields can be captured
:test-result: XFAIL #748 - captures with unexpected exceptions
:test-result: PASS #809
Expand Down
10 changes: 8 additions & 2 deletions tests/SelfTest/Baselines/compact.sw.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ Matchers.tests.cpp:<line number>: passed: smallest_non_zero, !WithinULP( -smalle
Matchers.tests.cpp:<line number>: passed: smallest_non_zero, WithinULP( -smallest_non_zero, 2 ) for: 0.0f is within 2 ULPs of -1.40129846e-45f ([-4.20389539e-45, 1.40129846e-45])
Matchers.tests.cpp:<line number>: passed: smallest_non_zero, !WithinULP( -smallest_non_zero, 1 ) for: 0.0f not is within 1 ULPs of -1.40129846e-45f ([-2.80259693e-45, -0.00000000e+00])
Generators.tests.cpp:<line number>: failed: unexpected exception with message: 'failure to init'; expression was: {Unknown expression after the reported line}
PartTracker.tests.cpp:<line number>: passed:
PartTracker.tests.cpp:<line number>: passed: (g == true || g == false) for: true
PartTracker.tests.cpp:<line number>: passed:
PartTracker.tests.cpp:<line number>: passed: (g == true || g == false) for: true
PartTracker.tests.cpp:<line number>: passed:
PartTracker.tests.cpp:<line number>: passed: (g == true || g == false) for: true
Compilation.tests.cpp:<line number>: passed: bf.e == 1 for: 1 == 1
Compilation.tests.cpp:<line number>: passed: 1 == bf.e for: 1 == 1
Exception.tests.cpp:<line number>: failed: unexpected exception with message: 'expected exception'; expression was: {Unknown expression after the reported line}
Expand Down Expand Up @@ -3036,7 +3042,7 @@ InternalBenchmark.tests.cpp:<line number>: passed: med == 18. for: 18.0 == 18.0
InternalBenchmark.tests.cpp:<line number>: passed: q3 == 23. for: 23.0 == 23.0
Misc.tests.cpp:<line number>: passed:
Misc.tests.cpp:<line number>: passed:
test cases: 456 | 336 passed | 96 failed | 6 skipped | 18 failed as expected
assertions: 2444 | 2243 passed | 158 failed | 43 failed as expected
test cases: 457 | 337 passed | 96 failed | 6 skipped | 18 failed as expected
assertions: 2450 | 2249 passed | 158 failed | 43 failed as expected


10 changes: 8 additions & 2 deletions tests/SelfTest/Baselines/compact.sw.multi.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ Matchers.tests.cpp:<line number>: passed: smallest_non_zero, !WithinULP( -smalle
Matchers.tests.cpp:<line number>: passed: smallest_non_zero, WithinULP( -smallest_non_zero, 2 ) for: 0.0f is within 2 ULPs of -1.40129846e-45f ([-4.20389539e-45, 1.40129846e-45])
Matchers.tests.cpp:<line number>: passed: smallest_non_zero, !WithinULP( -smallest_non_zero, 1 ) for: 0.0f not is within 1 ULPs of -1.40129846e-45f ([-2.80259693e-45, -0.00000000e+00])
Generators.tests.cpp:<line number>: failed: unexpected exception with message: 'failure to init'; expression was: {Unknown expression after the reported line}
PartTracker.tests.cpp:<line number>: passed:
PartTracker.tests.cpp:<line number>: passed: (g == true || g == false) for: true
PartTracker.tests.cpp:<line number>: passed:
PartTracker.tests.cpp:<line number>: passed: (g == true || g == false) for: true
PartTracker.tests.cpp:<line number>: passed:
PartTracker.tests.cpp:<line number>: passed: (g == true || g == false) for: true
Compilation.tests.cpp:<line number>: passed: bf.e == 1 for: 1 == 1
Compilation.tests.cpp:<line number>: passed: 1 == bf.e for: 1 == 1
Exception.tests.cpp:<line number>: failed: unexpected exception with message: 'expected exception'; expression was: {Unknown expression after the reported line}
Expand Down Expand Up @@ -3025,7 +3031,7 @@ InternalBenchmark.tests.cpp:<line number>: passed: med == 18. for: 18.0 == 18.0
InternalBenchmark.tests.cpp:<line number>: passed: q3 == 23. for: 23.0 == 23.0
Misc.tests.cpp:<line number>: passed:
Misc.tests.cpp:<line number>: passed:
test cases: 456 | 336 passed | 96 failed | 6 skipped | 18 failed as expected
assertions: 2444 | 2243 passed | 158 failed | 43 failed as expected
test cases: 457 | 337 passed | 96 failed | 6 skipped | 18 failed as expected
assertions: 2450 | 2249 passed | 158 failed | 43 failed as expected


4 changes: 2 additions & 2 deletions tests/SelfTest/Baselines/console.std.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1743,6 +1743,6 @@ due to unexpected exception with message:
Why would you throw a std::string?

===============================================================================
test cases: 456 | 354 passed | 76 failed | 7 skipped | 19 failed as expected
assertions: 2422 | 2243 passed | 136 failed | 43 failed as expected
test cases: 457 | 355 passed | 76 failed | 7 skipped | 19 failed as expected
assertions: 2428 | 2249 passed | 136 failed | 43 failed as expected

66 changes: 64 additions & 2 deletions tests/SelfTest/Baselines/console.sw.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,68 @@ Generators.tests.cpp:<line number>: FAILED:
due to unexpected exception with message:
failure to init

-------------------------------------------------------------------------------
#2809 - Sections taking more cycles than following GENERATE
a
aa
-------------------------------------------------------------------------------
PartTracker.tests.cpp:<line number>
...............................................................................

PartTracker.tests.cpp:<line number>: PASSED:

-------------------------------------------------------------------------------
#2809 - Sections taking more cycles than following GENERATE
-------------------------------------------------------------------------------
PartTracker.tests.cpp:<line number>
...............................................................................

PartTracker.tests.cpp:<line number>: PASSED:
REQUIRE( (g == true || g == false) )
with expansion:
true

-------------------------------------------------------------------------------
#2809 - Sections taking more cycles than following GENERATE
a
ab
-------------------------------------------------------------------------------
PartTracker.tests.cpp:<line number>
...............................................................................

PartTracker.tests.cpp:<line number>: PASSED:

-------------------------------------------------------------------------------
#2809 - Sections taking more cycles than following GENERATE
-------------------------------------------------------------------------------
PartTracker.tests.cpp:<line number>
...............................................................................

PartTracker.tests.cpp:<line number>: PASSED:
REQUIRE( (g == true || g == false) )
with expansion:
true

-------------------------------------------------------------------------------
#2809 - Sections taking more cycles than following GENERATE
b
-------------------------------------------------------------------------------
PartTracker.tests.cpp:<line number>
...............................................................................

PartTracker.tests.cpp:<line number>: PASSED:

-------------------------------------------------------------------------------
#2809 - Sections taking more cycles than following GENERATE
-------------------------------------------------------------------------------
PartTracker.tests.cpp:<line number>
...............................................................................

PartTracker.tests.cpp:<line number>: PASSED:
REQUIRE( (g == true || g == false) )
with expansion:
true

-------------------------------------------------------------------------------
#3001: Enum-based bitfields can be captured
-------------------------------------------------------------------------------
Expand Down Expand Up @@ -20371,6 +20433,6 @@ Misc.tests.cpp:<line number>
Misc.tests.cpp:<line number>: PASSED:

===============================================================================
test cases: 456 | 336 passed | 96 failed | 6 skipped | 18 failed as expected
assertions: 2444 | 2243 passed | 158 failed | 43 failed as expected
test cases: 457 | 337 passed | 96 failed | 6 skipped | 18 failed as expected
assertions: 2450 | 2249 passed | 158 failed | 43 failed as expected

66 changes: 64 additions & 2 deletions tests/SelfTest/Baselines/console.sw.multi.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,68 @@ Generators.tests.cpp:<line number>: FAILED:
due to unexpected exception with message:
failure to init

-------------------------------------------------------------------------------
#2809 - Sections taking more cycles than following GENERATE
a
aa
-------------------------------------------------------------------------------
PartTracker.tests.cpp:<line number>
...............................................................................

PartTracker.tests.cpp:<line number>: PASSED:

-------------------------------------------------------------------------------
#2809 - Sections taking more cycles than following GENERATE
-------------------------------------------------------------------------------
PartTracker.tests.cpp:<line number>
...............................................................................

PartTracker.tests.cpp:<line number>: PASSED:
REQUIRE( (g == true || g == false) )
with expansion:
true

-------------------------------------------------------------------------------
#2809 - Sections taking more cycles than following GENERATE
a
ab
-------------------------------------------------------------------------------
PartTracker.tests.cpp:<line number>
...............................................................................

PartTracker.tests.cpp:<line number>: PASSED:

-------------------------------------------------------------------------------
#2809 - Sections taking more cycles than following GENERATE
-------------------------------------------------------------------------------
PartTracker.tests.cpp:<line number>
...............................................................................

PartTracker.tests.cpp:<line number>: PASSED:
REQUIRE( (g == true || g == false) )
with expansion:
true

-------------------------------------------------------------------------------
#2809 - Sections taking more cycles than following GENERATE
b
-------------------------------------------------------------------------------
PartTracker.tests.cpp:<line number>
...............................................................................

PartTracker.tests.cpp:<line number>: PASSED:

-------------------------------------------------------------------------------
#2809 - Sections taking more cycles than following GENERATE
-------------------------------------------------------------------------------
PartTracker.tests.cpp:<line number>
...............................................................................

PartTracker.tests.cpp:<line number>: PASSED:
REQUIRE( (g == true || g == false) )
with expansion:
true

-------------------------------------------------------------------------------
#3001: Enum-based bitfields can be captured
-------------------------------------------------------------------------------
Expand Down Expand Up @@ -20360,6 +20422,6 @@ Misc.tests.cpp:<line number>
Misc.tests.cpp:<line number>: PASSED:

===============================================================================
test cases: 456 | 336 passed | 96 failed | 6 skipped | 18 failed as expected
assertions: 2444 | 2243 passed | 158 failed | 43 failed as expected
test cases: 457 | 337 passed | 96 failed | 6 skipped | 18 failed as expected
assertions: 2450 | 2249 passed | 158 failed | 43 failed as expected

Loading