Skip to content

Fix UB in dcalc tests: remove fake Pin* pointers from LoadPinIndexMap #346

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
gonsolo wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix UB in dcalc tests: remove fake Pin* pointers from LoadPinIndexMap #346

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
24 changes: 9 additions & 15 deletions dcalc/test/cpp/TestDcalc.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -802,22 +802,23 @@ TEST_F(StaDcalcTest, AllCalcsCopyDestroy) {
}

// Test UnitDelayCalc with non-empty load_pin_index_map
// Note: LoadPinIndexMap uses PinIdLess which calls network_->id(pin),
// so we cannot use fake Pin* pointers. Test with an empty map
// (load sizing is already covered by ArcDcalcResultTest).
TEST_F(StaDcalcTest, UnitDelayCalcGateDelayWithLoads) {
Copy link
Copy Markdown
Contributor

@jhkim-pii jhkim-pii Apr 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a non-empty load_pin_index_map does not look trivial.
I think removing this TEST_F (and the others below) is better because there is a similar UnitDelayCalcGateDelay test w/ empty load_pin_index_map.

Copy link
Copy Markdown
Author

@gonsolo gonsolo Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as my package still works, I'm happy:
NixOS/nixpkgs@63a954c

ArcDelayCalc *calc = makeDelayCalc("unit", sta_);
ASSERT_NE(calc, nullptr);
LoadPinIndexMap load_pin_index_map(sta_->network());
// Use dummy pin pointers for the index map
int dummy1 = 1, dummy2 = 2;
const Pin *pin1 = reinterpret_cast<const Pin*>(&dummy1);
const Pin *pin2 = reinterpret_cast<const Pin*>(&dummy2);
load_pin_index_map[pin1] = 0;
load_pin_index_map[pin2] = 1;
ArcDcalcResult result = calc->gateDelay(nullptr, nullptr, 0.0, 0.0,
nullptr, load_pin_index_map,
nullptr, nullptr);
EXPECT_GE(delayAsFloat(result.gateDelay()), 0.0f);
// UnitDelayCalc may leave uninitialized subnormal floats for wire delays;
// use EXPECT_NEAR with a tolerance to avoid flakiness.
// Verify wire delay / load slew accessors via ArcDcalcResult directly.
result.setLoadCount(2);
result.setWireDelay(0, 0.0);
result.setWireDelay(1, 0.0);
result.setLoadSlew(0, 0.0);
result.setLoadSlew(1, 0.0);
EXPECT_NEAR(delayAsFloat(result.wireDelay(0)), 0.0f, 1e-10f);
EXPECT_NEAR(delayAsFloat(result.wireDelay(1)), 0.0f, 1e-10f);
EXPECT_NEAR(delayAsFloat(result.loadSlew(0)), 0.0f, 1e-10f);
Expand All @@ -832,14 +833,10 @@ TEST_F(StaDcalcTest, UnitDelayCalcGateDelaysWithLoads) {
ArcDcalcArgSeq args;
args.push_back(ArcDcalcArg());
LoadPinIndexMap load_pin_index_map(sta_->network());
int dummy1 = 1;
const Pin *pin1 = reinterpret_cast<const Pin*>(&dummy1);
load_pin_index_map[pin1] = 0;
ArcDcalcResultSeq results = calc->gateDelays(args, load_pin_index_map,
nullptr, nullptr);
EXPECT_EQ(results.size(), 1u);
EXPECT_GE(delayAsFloat(results[0].gateDelay()), 0.0f);
EXPECT_FLOAT_EQ(delayAsFloat(results[0].wireDelay(0)), 0.0f);
delete calc;
}

Expand All @@ -848,9 +845,6 @@ TEST_F(StaDcalcTest, UnitDelayCalcInputPortDelayWithLoads) {
ArcDelayCalc *calc = makeDelayCalc("unit", sta_);
ASSERT_NE(calc, nullptr);
LoadPinIndexMap load_pin_index_map(sta_->network());
int dummy1 = 1;
const Pin *pin1 = reinterpret_cast<const Pin*>(&dummy1);
load_pin_index_map[pin1] = 0;
ArcDcalcResult result = calc->inputPortDelay(nullptr, 1e-10, nullptr,
nullptr, load_pin_index_map,
nullptr, nullptr);
Expand Down