Fix CSE errors out on string tensor attributes by rajatmonga · Pull Request #28422 · microsoft/onnxruntime

rajatmonga · 2026-05-08T21:54:48Z

Description

The CSE optimizer crashed with ORT_ENFORCE when encountering a node with a string tensor attribute (e.g., LabelEncoder with keys_tensor of type STRING). This replaces the enforce with an early return of hash 0, treating such nodes as distinct — consistent with how AreScalarTensorAttributeEqual already handles string tensors.

Motivation and Context

Fixes #28413 — regression in 1.25.1 where models using LabelEncoder (or any op with string tensor attributes) crash during graph optimization.

Changes

Replace ORT_ENFORCE(data_type != STRING) with an early return in GetTensorAttributeHash
Updated comment to document string tensor handling

rajatmonga · 2026-05-08T22:11:17Z

@rajatmonga please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
@microsoft-github-policy-service agree [company="{your company}"]
Options:

(default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
(when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"
Contributor License Agreement

@microsoft-github-policy-service agree company="Microsoft"

Copilot

Pull request overview

Fixes a crash in the Common Subexpression Elimination (CSE) graph optimizer when encountering scalar STRING tensor attributes (e.g., LabelEncoder’s keys_tensor) by treating those attributes as unsupported for hashing instead of enforcing.

Changes:

Replaced an ORT_ENFORCE on STRING tensor attributes in GetTensorAttributeHash with an early return (hash 0) to avoid crashing during optimization.
Updated the local comment to document how STRING tensor attributes are handled.
Fixed a lifetime bug in a test utility by avoiding a std::string_view bound to a temporary string returned by GetProviderOptionPrefix.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
`onnxruntime/test/util/default_providers.cc`	Prevents dangling `std::string_view` by storing the provider option prefix as a `std::string` before comparing/stripping it.
`onnxruntime/core/optimizer/common_subexpression_elimination.cc`	Avoids CSE crash on scalar STRING tensor attributes by returning a neutral hash instead of enforcing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

yuslepukhin · 2026-05-12T19:07:23Z

Please, do not force-push, it makes it difficult to see what changes occurred in-between the iterations.

The CSE optimizer crashed with ORT_ENFORCE when encountering a node with a string tensor attribute (e.g., LabelEncoder with keys_tensor of type STRING). Replace the enforce with an early return of hash 0, treating such nodes as distinct (consistent with AreScalarTensorAttributeEqual). Fixes #28413 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…28413) Agent-Logs-Url: https://github.com/microsoft/onnxruntime/sessions/0ea57f90-11b2-48df-ad71-d540fdb088e2 Co-authored-by: rajatmonga <15679194+rajatmonga@users.noreply.github.com>

Use LabelEncoder (ai.onnx.ml) instead of Constant nodes so the STRING tensor attribute survives Model::Load and actually exercises GetTensorAttributeHash. Remove the .onnx test data file and generate.py additions that are no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 111 out of 111 changed files in this pull request and generated 2 comments.

github-actions

You can commit the suggested changes from lintrunner.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

onnxruntime/test/optimizer/cse_test.cc:307

This test hard-codes the ONNX-ML domain string in multiple places (opset import and AddNode). Using the existing kMLDomain constant would avoid typos and keep the domain value consistent across tests.

  Model model("CseStringTensorAttrTest", false, ModelMetaData(), PathString(),
              IOnnxRuntimeOpSchemaRegistryList(),
              {{kOnnxDomain, 21}, {"ai.onnx.ml", 4}}, {}, logger);
  auto& graph = model.MainGraph();

+TEST(CseTests, StringTensorAttr) {
+  // Regression test for https://github.com/microsoft/onnxruntime/issues/28413.
+  // CSE must not crash when it encounters a node with a STRING tensor attribute.
+  // We use LabelEncoder (ai.onnx.ml) because it retains its STRING tensor attribute
+  // through Model::Load (unlike Constant nodes which are converted to initializers).
+  const auto& logger = DefaultLoggingManager().DefaultLogger();
+  Model model("CseStringTensorAttrTest", false, ModelMetaData(), PathString(),
+              IOnnxRuntimeOpSchemaRegistryList(),
+              {{kOnnxDomain, 21}, {"ai.onnx.ml", 4}}, {}, logger);
+  auto& graph = model.MainGraph();


Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

+TEST(CseTests, StringTensorAttr) {
+  // Regression test for https://github.com/microsoft/onnxruntime/issues/28413.
+  // CSE must not crash when it encounters a node with a STRING tensor attribute.
+  // We use LabelEncoder (ai.onnx.ml) because it retains its STRING tensor attribute
+  // through Model::Load (unlike Constant nodes which are converted to initializers).
+  const auto& logger = DefaultLoggingManager().DefaultLogger();
+  Model model("CseStringTensorAttrTest", false, ModelMetaData(), PathString(),
+              IOnnxRuntimeOpSchemaRegistryList(),
+              {{kOnnxDomain, 21}, {"ai.onnx.ml", 4}}, {}, logger);


rajatmonga · 2026-05-13T19:01:55Z

Reverting PR as there are other issues. @Dmitri is submitting a better one.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

onnxruntime/test/optimizer/cse_test.cc:350

This comment is now misleading: the optimizer hashes STRING tensor attributes and AreScalarTensorAttributeEqual compares their string_data(), so string tensor attributes are not generally treated as distinct. In this specific test the LabelEncoder nodes remain distinct because ai.onnx.ml is not treated as a supported deterministic domain by CSE, not because of the string attribute handling.

  // CSE won't merge these nodes (STRING tensor attrs are treated as distinct),
  // but it must not crash when hashing them.

+TEST(CseTests, StringTensorAttr) {
+  // Regression test for https://github.com/microsoft/onnxruntime/issues/28413.
+  // CSE must not crash when it encounters a node with a STRING tensor attribute.
+  // We use LabelEncoder (ai.onnx.ml) because it retains its STRING tensor attribute
+  // through Model::Load (unlike Constant nodes which are converted to initializers).
+  const auto& logger = DefaultLoggingManager().DefaultLogger();
+  Model model("CseStringTensorAttrTest", false, ModelMetaData(), PathString(),
+              IOnnxRuntimeOpSchemaRegistryList(),
+              {{kOnnxDomain, 21}, {"ai.onnx.ml", 4}}, {}, logger);


+  if (utils::HasString(lhs_t)) {
+    return AreRangesEqual(lhs_t.string_data(), rhs_t.string_data());


yuslepukhin requested a review from Copilot May 8, 2026 23:25

Copilot started reviewing on behalf of yuslepukhin May 8, 2026 23:26 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

Comment thread onnxruntime/core/optimizer/common_subexpression_elimination.cc Outdated

rajatmonga force-pushed the rajatmonga/cse-string-crash-issue-28413 branch from e6ec7d4 to 8a81fb2 Compare May 8, 2026 23:49

Copilot started work on behalf of rajatmonga May 8, 2026 23:50 View session

Copilot finished work on behalf of rajatmonga May 9, 2026 00:00

rajatmonga force-pushed the rajatmonga/cse-string-crash-issue-28413 branch from b7f1cc2 to 9535d7f Compare May 11, 2026 19:56

rajatmonga self-assigned this May 11, 2026

rajatmonga requested a review from yuslepukhin May 12, 2026 00:41

yuslepukhin requested review from Copilot and removed request for yuslepukhin May 12, 2026 02:33

Copilot started reviewing on behalf of yuslepukhin May 12, 2026 02:34 View session

Copilot AI reviewed May 12, 2026

View reviewed changes

Comment thread onnxruntime/test/optimizer/cse_test.cc Outdated

Comment thread onnxruntime/test/testdata/transform/cse/generate.py Outdated

yuslepukhin changed the title ~~Fix CSE crash on string tensor attributes~~ Fix CSE errors out on string tensor attributes May 12, 2026

Rajat Monga and others added 3 commits May 12, 2026 14:47

Add regression test for CSE crash on STRING tensor attributes (issue #…

b9ffcec

…28413) Agent-Logs-Url: https://github.com/microsoft/onnxruntime/sessions/0ea57f90-11b2-48df-ad71-d540fdb088e2 Co-authored-by: rajatmonga <15679194+rajatmonga@users.noreply.github.com>

rajatmonga requested a review from Copilot May 12, 2026 22:22

Copilot started reviewing on behalf of rajatmonga May 12, 2026 22:23 View session

Copilot AI reviewed May 12, 2026

View reviewed changes

Comment thread onnxruntime/core/providers/cuda/plugin/cuda_plugin_ep.cc

Comment thread onnxruntime/core/optimizer/common_subexpression_elimination.cc Outdated

rajatmonga force-pushed the rajatmonga/cse-string-crash-issue-28413 branch from df1e17c to 96696b8 Compare May 12, 2026 22:42

rajatmonga marked this pull request as draft May 12, 2026 22:43

github-actions Bot reviewed May 12, 2026

View reviewed changes

Comment thread onnxruntime/test/optimizer/cse_test.cc Outdated

Comment thread onnxruntime/test/optimizer/cse_test.cc Outdated

Fix lint: align continuation indentation in CSE test

61efcea

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

rajatmonga requested a review from Copilot May 12, 2026 23:01

Copilot started reviewing on behalf of rajatmonga May 12, 2026 23:02 View session

Copilot AI reviewed May 12, 2026

View reviewed changes

yuslepukhin requested a review from Copilot May 13, 2026 18:04

Copilot started reviewing on behalf of yuslepukhin May 13, 2026 18:08 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

Support string attributes hashing and compare

ae0a640

yuslepukhin requested a review from Copilot May 13, 2026 18:56

Copilot started reviewing on behalf of yuslepukhin May 13, 2026 18:59 View session

rajatmonga closed this May 13, 2026

rajatmonga deleted the rajatmonga/cse-string-crash-issue-28413 branch May 13, 2026 19:02

Copilot AI reviewed May 13, 2026

View reviewed changes

		if (utils::HasString(lhs_t)) {
		return AreRangesEqual(lhs_t.string_data(), rhs_t.string_data());

Uh oh!

Conversation

rajatmonga commented May 8, 2026

Description

Motivation and Context

Changes

Uh oh!

rajatmonga commented May 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

yuslepukhin commented May 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

rajatmonga commented May 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants