Skip to content

LibertyReader: guard null FuncExpr/members on malformed Liberty #373

Open
ranaumarnadeem wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
ranaumarnadeem:master
Open

LibertyReader: guard null FuncExpr/members on malformed Liberty #373
ranaumarnadeem wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
ranaumarnadeem:master

Conversation

@ranaumarnadeem

Copy link
Copy Markdown

Fix three null-dereference crashes in LibertyReader on malformed Liberty

Reading a Liberty file with certain malformed port attributes crashes OpenSTA
with a SIGSEGV. Three sites in LibertyReader dereference a pointer that can
legitimately be null on malformed input. This adds the missing null guards
(matching the existing guarded sibling path) so the reader emits a warning and
continues, plus a focused regression test.

reoroduce via this

A few lines of Liberty + read_liberty:

  1. function references an undeclared pin (e.g. a typo):
    pin (Z) { direction : output; function : "A & NOPIN"; }
  2. three_state references an undeclared pin:
    pin (Z) { direction : output; three_state : "BADPIN"; function : "A"; }
  3. bundle group with no members:
    bundle (B) { direction : output; }

Root cause

  • LibertyReader::makePortFuncs() calls parseFunc() for function and
    three_state. parseFunc() returns nullptr for an expression that parses
    but references an unknown port (LibExprReader::makeFuncExprPort warns and
    returns null). The results were dereferenced unconditionally
    (func_expr->checkSize(port), tri_disable_expr->invert()).
  • LibertyReader::makeBundlePort() calls findComplexAttr("members"), which
    returns nullptr when the attribute is absent, then dereferences it in the
    range-for (member_attr->values()) and leaks the allocated ConcretePortSeq.

The correct pattern already exists in the same file: makeSeqFunc() guards with
if (expr && expr->checkSize(size)). These three sites simply missed it.

Fix

  • function: if (func_expr && func_expr->checkSize(port)) (mirrors makeSeqFunc).
  • three_state: wrap the tristate handling in if (tri_disable_expr) { ... }.
  • bundle: if members is absent, warn(1315, ...) "bundle <name> missing members." and return (also avoids the leak).

Valid Liberty is unaffected — the guarded expressions are non-null there. On
malformed input the reader now warns and continues; the cell loads minus the
bad attribute.

Tests

New regression liberty/test/liberty_malformed_ports.{lib,tcl} (+ golden .ok),
registered in liberty/test/CMakeLists.txt. One read_liberty exercises all
three paths; the golden pins the warnings and confirms recovery

Verification

  • All three reproducers now exit 0 instead of SIGSEGV (exit 139).
  • ctest -R tcl.liberty: 33/33 pass (32 existing + the new test).
  • Full ctest suite: 6083/6083 pass, zero regressions.

Files changed

  • liberty/LibertyReader.cc — three null guards (+ new warning id 1315)
  • liberty/test/liberty_malformed_ports.lib — new (test input)
  • liberty/test/liberty_malformed_ports.tcl — new (test driver)
  • liberty/test/liberty_malformed_ports.ok — new (golden)
  • liberty/test/CMakeLists.txt — register the new test

@CLAassistant

CLAassistant commented Jun 19, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request fixes potential null-pointer dereferences in LibertyReader.cc by adding null checks for member_attr in makeBundlePort, and for func_expr and tri_disable_expr in makePortFuncs. It also adds a new test case malformed_ports to ensure that malformed port attributes trigger warnings instead of causing crashes. There are no review comments to evaluate, so no further feedback is provided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants