Skip to content
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.

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

Fix checkfail in UnicodeEncode Op #63398

Merged
merged 1 commit into from
Apr 2, 2024
Merged

Conversation

SuryanarayanaY
Copy link
Collaborator

@SuryanarayanaY SuryanarayanaY commented Mar 11, 2024

The Op UnicodeEncode segfaults when passed 2D tensor to input_splits.

It has the below check in SetShapeFn which supposed to raise exception if rank !=1 AFAIk. This seems not working for reason unknown to me.

ShapeHandle splits_shape = c->input(1);
TF_RETURN_IF_ERROR(c->WithRank(splits_shape, 1, &unused));

Same with input_values argument also.

Added an explicit check in Op.

Ref issue #63379

The Op UnicodeEncode segfaults when passed 2D tensor to `input_splits`.

It has the below check in SetShapeFn which supposed to raise exception if rank !=1 AFAIk. This seems not working for reason unknown to me.

https://github.com/tensorflow/tensorflow/blob/6f64ad5d767a034df45a5eaab8b36fd688cd1217/tensorflow/core/ops/string_ops.cc#L316-L317

Same with input_values argument also.
 
Added an explicit check in Op.
@google-ml-butler google-ml-butler bot added the size:XS CL Change Size: Extra Small label Mar 11, 2024
@gbaned gbaned added the comp:core issues related to core part of tensorflow label Mar 12, 2024
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Mar 12, 2024
@gbaned gbaned requested a review from sagunb March 12, 2024 06:52
@google-ml-butler google-ml-butler bot added the awaiting review Pull request awaiting review label Mar 12, 2024
Copy link
Member

@sagunb sagunb left a comment

Choose a reason for hiding this comment

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

Thank you for the changes. It is odd this check was missing when the comment above explicitly states the same thing.

@google-ml-butler google-ml-butler bot added the kokoro:force-run Tests on submitted change label Mar 14, 2024
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Mar 14, 2024
@google-ml-butler google-ml-butler bot added the ready to pull PR ready for merge process label Mar 14, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Mar 14, 2024
@google-ml-butler google-ml-butler bot added the kokoro:force-run Tests on submitted change label Mar 14, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Mar 14, 2024
@gbaned gbaned added ready to pull PR ready for merge process and removed awaiting review Pull request awaiting review ready to pull PR ready for merge process labels Apr 1, 2024
copybara-service bot pushed a commit that referenced this pull request Apr 2, 2024
FUTURE_COPYBARA_INTEGRATE_REVIEW=#63398 from tensorflow:Surya_UnicodeEncode 176048a
PiperOrigin-RevId: 621054940
copybara-service bot pushed a commit that referenced this pull request Apr 2, 2024
Imported from GitHub PR #63398

The Op UnicodeEncode segfaults when passed 2D tensor to `input_splits`.

It has the below check in `SetShapeFn` which supposed to raise exception if `rank !=1` AFAIk. This seems not working for reason unknown to me.

https://github.com/tensorflow/tensorflow/blob/6f64ad5d767a034df45a5eaab8b36fd688cd1217/tensorflow/core/ops/string_ops.cc#L316-L317

Same with `input_values` argument also.

Added an explicit check in Op.

Ref issue #63379
Copybara import of the project:

--
176048a by Surya <116063290+SuryanarayanaY@users.noreply.github.com>:

Fix checkfail in UnicodeEncode Op

The Op UnicodeEncode segfaults when passed 2D tensor to `input_splits`.

It has the below check in SetShapeFn which supposed to raise exception if rank !=1 AFAIk. This seems not working for reason unknown to me.

https://github.com/tensorflow/tensorflow/blob/6f64ad5d767a034df45a5eaab8b36fd688cd1217/tensorflow/core/ops/string_ops.cc#L316-L317

Same with input_values argument also.

Added an explicit check in Op.

Merging this change closes #63398

FUTURE_COPYBARA_INTEGRATE_REVIEW=#63398 from tensorflow:Surya_UnicodeEncode 176048a
PiperOrigin-RevId: 620612470
copybara-service bot pushed a commit that referenced this pull request Apr 2, 2024
FUTURE_COPYBARA_INTEGRATE_REVIEW=#63398 from tensorflow:Surya_UnicodeEncode 176048a
PiperOrigin-RevId: 620398840
copybara-service bot pushed a commit that referenced this pull request Apr 2, 2024
FUTURE_COPYBARA_INTEGRATE_REVIEW=#63398 from tensorflow:Surya_UnicodeEncode 176048a
PiperOrigin-RevId: 620397151
copybara-service bot pushed a commit that referenced this pull request Apr 2, 2024
FUTURE_COPYBARA_INTEGRATE_REVIEW=#63398 from tensorflow:Surya_UnicodeEncode 176048a
PiperOrigin-RevId: 620335988
@copybara-service copybara-service bot merged commit 233305a into master Apr 2, 2024
13 of 15 checks passed
PR Queue automation moved this from Approved by Reviewer to Merged Apr 2, 2024
@copybara-service copybara-service bot deleted the Surya_UnicodeEncode branch April 2, 2024 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:core issues related to core part of tensorflow ready to pull PR ready for merge process size:XS CL Change Size: Extra Small
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

5 participants