Skip to content

ord: implement whitelist-based Tcl command logging#10686

Open
sparsh-karna wants to merge 7 commits into
The-OpenROAD-Project:masterfrom
sparsh-karna:feat/cmd-log-whitelist
Open

ord: implement whitelist-based Tcl command logging#10686
sparsh-karna wants to merge 7 commits into
The-OpenROAD-Project:masterfrom
sparsh-karna:feat/cmd-log-whitelist

Conversation

@sparsh-karna

Copy link
Copy Markdown
Contributor

Description

This PR implements Tcl command logging (Issue #3539) by logging every registered OpenROAD command executed by the user to the file specified by the -log flag, as cmd: <command> with its variables expanded.

To avoid the noise of internal implementation details (such as set, if, and internal proc bodies) flooding the log, this implementation utilizes a whitelist approach.

Key Changes

  • Command Whitelist: Captured the output of info commands before and after ord::initOpenRoad is called. The difference forms a robust whitelist of OpenROAD-registered commands, explicitly excluding Tcl builtins.
  • Tcl Object Trace: Used Tcl_CreateObjTrace to intercept commands before they are executed. If the command name is present in the whitelist, the full command string is reconstructed using Tcl_GetString on the already-expanded objv arguments and logged via logger->report().
  • Conditional Logging to Protect Tests: The command logging trace is strictly registered only when the -log parameter is provided (log_filename != nullptr). This ensures that standard CTest regression runs (which capture stdout via redirection rather than the -log flag) do not fail their compare_logfile checks due to unexpected cmd: lines.
  • Regression Testing: Added a dedicated regression test (cmd_log_trace) that spawns an OpenROAD subprocess with -log, executes a mix of whitelisted commands and internal Tcl builtins, and automatically reads back its own log to verify that only the correct commands are traced.

Prerequisites

  • Clang-format has been run.
  • Commits follow DCO compliance (git commit -s).
  • Build locally using ./etc/Build.sh
  • Ran related regression tests

Related Issues

Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>
@sparsh-karna sparsh-karna requested a review from a team as a code owner June 18, 2026 06:56
@sparsh-karna sparsh-karna requested a review from maliberty June 18, 2026 06:56

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

Copy link
Copy Markdown
Contributor

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 implements Tcl command tracing to log newly registered OpenRoad commands while filtering out standard Tcl built-ins, along with a corresponding integration test. The feedback highlights two important improvements: first, passing the TCL_ALLOW_INLINE_COMPILATION flag to Tcl_CreateObjTrace to avoid severe performance degradation caused by disabling bytecode compilation; second, stripping the leading :: namespace prefix from command names to ensure fully qualified commands are correctly matched against the whitelist.

Comment thread src/Main.cc Outdated
Comment on lines +173 to +174
Tcl_CreateObjTrace(
interp, 0, 0, logCommandCallback, trace_data, deleteCommandTrace);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Without the TCL_ALLOW_INLINE_COMPILATION flag, Tcl disables bytecode compilation for all commands executed in the interpreter while the trace is active. This can cause a severe performance degradation (often 10x or more) for Tcl scripts, especially those containing loops or heavy computations.

Since the callback only uses objc and objv (which are fully populated and valid even with inline compilation), enabling TCL_ALLOW_INLINE_COMPILATION is completely safe and preserves Tcl's execution performance.

  Tcl_CreateObjTrace(
      interp, 0, TCL_ALLOW_INLINE_COMPILATION, logCommandCallback, trace_data, deleteCommandTrace);

Comment thread src/Main.cc
Comment on lines +122 to +125
const char* cmd_name = Tcl_GetString(objv[0]);
if (cmd_name == nullptr || data->whitelist.count(cmd_name) == 0) {
return TCL_OK;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If a user invokes an OpenROAD command using a leading namespace qualifier (for example, ::read_lef instead of read_lef), Tcl_GetString(objv[0]) will return the fully qualified name with the leading colons. Since the whitelist contains the unqualified command names returned by info commands, the lookup data->whitelist.count(cmd_name) will fail, and the command will not be logged.

We can make this robust by stripping the leading :: prefix from the command name before performing the whitelist lookup.

  const char* cmd_name = Tcl_GetString(objv[0]);
  if (cmd_name == nullptr) {
    return TCL_OK;
  }
  if (std::strncmp(cmd_name, "::", 2) == 0) {
    cmd_name += 2;
  }
  if (data->whitelist.count(cmd_name) == 0) {
    return TCL_OK;
  }

Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>
Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>
Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>
Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>
Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>
Fixes a strict type mismatch in the macOS CI where Tcl_CreateObjTrace
requires the callback's objc parameter to be an int rather than a Tcl_Size.

Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>
@sparsh-karna

Copy link
Copy Markdown
Contributor Author

hey @maliberty could you please review this

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

openroad should write it's commands into the log

1 participant