[bandhambalaji]

Summary

When insert_buffer is invoked without -buffer_name or -net_name (e.g. buffering a single load pin or inferred nets), nullptr is forwarded through the Resizer layer into dbInsertBuffer. Assigning nullptr directly to new_buf_base_name_ overrides kDefaultBufBaseName and causes _dbBlock::makeNewName to execute fmt::string_view(nullptr), triggering a Segmentation Fault (Signal 11).

Solution

1. Fall back to kDefaultBufBaseName ("buf") and kDefaultNetBaseName ("net") in dbInsertBuffer::insertBufferSimple and dbInsertBuffer::insertBufferBeforeLoads when incoming names are null or empty.
2. Add defensive null checks in _dbBlock::makeNewName and dbBlock::getBaseName.

Fixes #12016.

[gemini-code-assist]

Code Review

This pull request adds null and empty-string checks for base names and full names in dbBlock.cpp and dbInsertBuffer.cpp to prevent potential null pointer dereferences. However, the feedback highlights that the silent fallbacks introduced in dbBlock.cpp (defaulting to 'unnamed' or returning an empty string) violate the OpenROAD style guide against hiding impossible states. It is recommended to use assertions instead of silent defaults for these invalid states.

[gemini-code-assist]

According to the OpenROAD style guide, we should avoid silent fallbacks (like defaulting to "unnamed") for impossible or invalid states. Since base_name being null is an invalid state that should be handled at the caller/API boundary (as you did in dbInsertBuffer), we should assert that base_name is not null here rather than silently falling back to "unnamed".

  assert(base_name != nullptr);
  buf.append(fmt::string_view(base_name));

References

1. Never suggest a guard that hides an impossible state: a null check, an empty, zero or negative check, or a missing-key check that returns a default, skips, clamps, continues or logs a warning and carries on. A null dereference that is reachable is reported with its triggering input; the fix is logger_->error(...) or an assert, never a silent default. ^(link)

[gemini-code-assist]

According to the OpenROAD style guide, we should avoid silent fallbacks (like returning "") for impossible or invalid states. If full_name being null is an invalid state, we should assert that it is not null rather than silently returning an empty string.

  assert(full_name != nullptr);

References

1. Never suggest a guard that hides an impossible state: a null check, an empty, zero or negative check, or a missing-key check that returns a default, skips, clamps, continues or logs a warning and carries on. A null dereference that is reachable is reported with its triggering input; the fix is logger_->error(...) or an assert, never a silent default. ^(link)

[github-actions]

Welcome to OpenROAD! Thanks for opening your first PR.
Before we review:

• Contribution Guide: https://openroad.readthedocs.io/en/latest/contrib/GettingInvolved.html
• Build Instructions: https://openroad.readthedocs.io/en/latest/user/Build.html

Please ensure:

• CI passes
• Code is properly formatted
• Tests are included where applicable
  A maintainer will review shortly!

[osamahammad21]

I think the original issue is that rsz sends a null name to odb. I'd rather fix that behavior in rsz and throw an error in odb when the name is not given. However, this is out of my scope so a review from @The-OpenROAD-Project/openroad-rsz would be helpful. @precisionmoon

[precisionmoon]

Thanks much for the contribution. Can you add a regression test to exercise the code fix please?

[bandhambalaji]

Added a regression test to src/odb/test/cpp/TestInsertBuffer.cpp exercising both single-load and multi-load buffer insertions with nullptr base names to ensure default name fallback without crashing. Ready for review.