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 segmentation fault in shape inference logic. #50985

Merged
merged 2 commits into from Jul 28, 2021

Conversation

geetachavan1
Copy link
Contributor

When running shape functions, some functions (such as MutableHashTableShape)
produce extra output information in the form of a ShapeAndType struct. The
shapes embedded in this struct are owned by an inference context that is
cleaned up almost immediately; if the upstream code attempts to access this
shape information, it can trigger a segfault.

ShapeRefiner is mitigating this for normal output shapes by cloning them
(and thus putting the newly created shape under ownership of an inference
context that will not die), but we were not doing the same for shapes and
types. This commit fixes that by doing similar logic on output shapes and
types.

PiperOrigin-RevId: 384761124
Change-Id: I07c0c42d29dfbb55bfa13ec1f09ef825fb0a1a1d

When running shape functions, some functions (such as `MutableHashTableShape`)
produce extra output information in the form of a `ShapeAndType` struct.  The
shapes embedded in this struct are owned by an inference context that is
cleaned up almost immediately; if the upstream code attempts to access this
shape information, it can trigger a segfault.

`ShapeRefiner` is mitigating this for normal output shapes by cloning them
(and thus putting the newly created shape under ownership of an inference
context that will not die), but we were not doing the same for shapes and
types.  This commit fixes that by doing similar logic on output shapes and
types.

PiperOrigin-RevId: 384761124
Change-Id: I07c0c42d29dfbb55bfa13ec1f09ef825fb0a1a1d
@google-cla google-cla bot added the cla: yes label Jul 27, 2021
@gbaned gbaned self-assigned this Jul 28, 2021
@gbaned gbaned added comp:core issues related to core part of tensorflow size:S CL Change Size: Small labels Jul 28, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Jul 28, 2021
@gbaned gbaned assigned mihaimaruseac and unassigned gbaned Jul 28, 2021
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Jul 28, 2021
@mihaimaruseac mihaimaruseac merged commit 52ca09c into tensorflow:r2.3 Jul 28, 2021
PR Queue automation moved this from Approved by Reviewer to Merged Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:core issues related to core part of tensorflow size:S CL Change Size: Small
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

4 participants