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

Segfault when calling TF_OperationGetAttrTensor on malformed tensor #7394

Closed
malmaud opened this issue Feb 9, 2017 · 8 comments
Closed

Segfault when calling TF_OperationGetAttrTensor on malformed tensor #7394

malmaud opened this issue Feb 9, 2017 · 8 comments
Assignees

Comments

@malmaud
Copy link
Contributor

malmaud commented Feb 9, 2017

On 1.0.0-rc1 (and earlier versions), this segfaults:

void dealloc(void* data, size_t len, void* arg) {
}

int main() {
  TF_Tensor* empty = TF_NewTensor(TF_FLOAT, NULL, 0, NULL, 0, dealloc, NULL);
  TF_Graph* graph = TF_NewGraph();
  TF_OperationDescription* desc = TF_NewOperation(graph, "Const", "empty");
  TF_Status* status = TF_NewStatus();
  TF_SetAttrTensor(desc, "value", empty, status);
  TF_SetAttrType(desc, "dtype", TF_FLOAT);
  TF_Operation* op = TF_FinishOperation(desc, status);
  TF_Tensor* value;
  TF_OperationGetAttrTensor(op, "value", &value, status); //Segfaults
  return 0;
}

Note that TF_Message(status) is TF_OK after the calls to TF_SetAttrTensor and TF_FinishOperation.

@asimshankar
Copy link
Contributor

We should probably error our more safely, but could you elabore on what you mean by an "empty" tensor? A tensor with ndims=0 is intended for representing a scalar, so a TF_FLOAT tensor with ndims=0 should be backed by 4 bytes of storage. So, the empty TF_Tensor in the snippet above seems malformed.

Perhaps TF_NewTensor should return nullptr in that case and we should update the documentation for it. Does that make sense? Or is there something about an empty tensor that I'm missing?

@asimshankar asimshankar added the stat:awaiting response Status - Awaiting response from author label Feb 9, 2017
@malmaud
Copy link
Contributor Author

malmaud commented Feb 9, 2017

Yes thanks, that makes sense. It just seems problematic that a malformed tensor should cause TF_OperationGetAttrTensorto crash instead of safely error-ing out.

@aselle aselle removed the stat:awaiting response Status - Awaiting response from author label Feb 9, 2017
@malmaud malmaud changed the title Segfault when calling TF_OperationGetAttrTensor on empty tensor Segfault when calling TF_OperationGetAttrTensor on malformed tensor Feb 10, 2017
@yifeif
Copy link
Contributor

yifeif commented Jun 16, 2017

@asimshankar do we plan to add a check or this is expected?

@tensorflowbutler
Copy link
Member

It has been 14 days with no activity and this issue has an assignee.Please update the label and/or status accordingly.

1 similar comment
@tensorflowbutler
Copy link
Member

It has been 14 days with no activity and this issue has an assignee.Please update the label and/or status accordingly.

@tensorflowbutler
Copy link
Member

Nagging Assignee: It has been 14 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.

@gunan
Copy link
Contributor

gunan commented Feb 8, 2018

ping @asimshankar is this resolved?
Should we close the issue?

@asimshankar
Copy link
Contributor

Ah, I hadn't gotten to this, but it seems like a simple fix. I'll try to get this out soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants