Skip to content

Commit

Permalink
Fixes deterministic crash that's node ID dependent (#647)
Browse files Browse the repository at this point in the history
We had a deterministic crash in the alias allocator that was triggered based on the exact bit pattern of the NodeID.

This PR fixes the crash, and adds a unit test that verifies that we can successfully allocate a full cycle of 4096 aliases.

===

* Fix crash in alias allocator when the generator sequence hits the zero alias.

* Adds regression test for aliases crashing the node dependeing on node ID.
  • Loading branch information
balazsracz authored Aug 27, 2022
1 parent e5796e2 commit ab5cee2
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/openlcb/AliasAllocator.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,10 @@ NodeAlias AliasAllocator::get_new_seed()
{
NodeAlias ret = seed_;
next_seed();
if (!ret)
{
continue;
}
LOG(VERBOSE, "(%p) alias test seed is %03X (next %03X)", this, ret,
seed_);
if (if_can()->local_aliases()->lookup(ret))
Expand Down
45 changes: 45 additions & 0 deletions src/openlcb/AliasAllocator.cxxtest
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "openlcb/BulkAliasAllocator.hxx"
#include "openlcb/CanDefs.hxx"
#include "utils/async_if_test_helper.hxx"
#include "os/FakeClock.hxx"

namespace openlcb
{
Expand Down Expand Up @@ -369,6 +370,50 @@ TEST_F(AsyncAliasAllocatorTest, DifferentGenerated)
wait();
}

TEST_F(AsyncAliasAllocatorTest, SequenceCrash)
{
FakeClock clk;
alias_allocator_.~AliasAllocator();
NodeID id = 0x050101011415;
new (&alias_allocator_) AliasAllocator(id, ifCan_.get());
clear_expect(false); // any packet is okay.
for (int i = 0; i < 25; i++)
{
fprintf(stderr, ".");
mainBufferPool->alloc(&b_);
alias_allocator_.send(b_);
wait();
clk.advance(MSEC_TO_NSEC(250));
wait();
}
twait();
}

TEST_F(AsyncAliasAllocatorTest, SequenceCrashLong)
{
FakeClock clk;
alias_allocator_.~AliasAllocator();
NodeID id = 0x090099DD0002;
new (&alias_allocator_) AliasAllocator(id, ifCan_.get());
clear_expect(false); // any packet is okay.
// This wraps around more than once the entire sequence
for (int i = 0; i < 6000; i++)
{
if (i % 50 == 0)
{
LOG(INFO, " %d", i);
}
fprintf(stderr, ".");
mainBufferPool->alloc(&b_);
alias_allocator_.send(b_);
wait();
clk.advance(MSEC_TO_NSEC(250));
wait();
}
twait();
}


TEST_F(AsyncAliasAllocatorTest, BulkFew)
{
RX(EXPECT_EQ(0u, ifCan_->alias_allocator()->num_reserved_aliases()));
Expand Down

0 comments on commit ab5cee2

Please sign in to comment.