yangfly commented on a change in pull request #13351: Fix
contrib.bipartite_matching malloc defect
URL: https://github.com/apache/incubator-mxnet/pull/13351#discussion_r237119068
##########
File path: src/operator/contrib/bounding_box-inl.h
##########
@@ -785,7 +785,7 @@ void BipartiteMatchingForward(const nnvm::NodeAttrs& attrs,
.get_with_shape<xpu, 2, DType>(Shape2(batch_size, col), s);
Shape<1> sort_index_shape = Shape1(dshape.Size());
index_t workspace_size = sort_index_shape.Size();
- workspace_size += ((sort_index_shape.Size() * sizeof(int32_t) - 1) /
sizeof(DType)) * 2;
+ workspace_size += ((sort_index_shape.Size() * sizeof(int32_t) - 1) /
sizeof(DType) + 1) * 2;
Review comment:
- Just work like [this
line](https://github.com/apache/incubator-mxnet/blob/b6907321f9e49e77e18aeacab329fdd2018a5f80/src/operator/contrib/bounding_box-inl.h#L426).
```
// ceil up when sizeof(DType) is larger than sizeof(DType)
// #L426
index_t int32_offset = (int32_size * sizeof(int32_t) - 1) / sizeof(DType) +
1;
// #788
workspace_size += ((sort_index_shape.Size() * sizeof(int32_t) - 1) /
sizeof(DType) + 1) * 2;
```
In use case, raw_workspace_size = sort_index_shape.Size() * (sizeof(int32_t)
+ sizeof(DType) * 2,
as workspace is allocated as tensor of Dtype, raw_workspace_size should be
ceiled. Hence **plus 1** is necessary or sometime **out-of-range**.
- CI failure is due to build failure. That should not be caused by this
change. BTW, I'm not familiar with CI and might miss some details.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
