ATen icon indicating copy to clipboard operation
ATen copied to clipboard

btrifact() may dereference nullptr

Open colesbury opened this issue 6 years ago • 3 comments

Here is the generated code:

std::tuple<Tensor,Tensor> CPUFloatType::btrifact(const Tensor & info, bool pivot, const Tensor & self) {
    auto result_ = new CPUFloatTensor(context);
    auto result = Tensor(result_,false);
    auto pivots_ = new CPUIntTensor(context);
    auto pivots = Tensor(pivots_,false);
    auto info_ = checked_cast<CPUIntTensor>(info.pImpl,"info",1, true);
    auto self_ = checked_cast<CPUFloatTensor>(self.pImpl,"self",3, false);
    THFloatTensor_btrifact(result_->tensor, pivots_->tensor, info_ ? info_->tensor : NULL, pivot, self_->tensor);
    bool maybe_scalar = info.dim() == 0 && self.dim() == 0;
    result_->maybeScalar(maybe_scalar);
    pivots_->maybeScalar(maybe_scalar);
    return std::tuple<Tensor, Tensor>(result, pivots);
}

The maybe_scalar calculation calls info.dim() but info is allowed to be an undefined tensor. I don't think info should affect the maybe_scalar calculation at all.

colesbury avatar Sep 14 '17 18:09 colesbury

Undefined Tensor means a TH tensor with no backing storage, but info itself is pointing to a valid TensorImpl? Just trying to understand the situation.

zdevito avatar Sep 14 '17 18:09 zdevito

I mean a tensor where pImpl is NULL. (i.e. tensor.defined() returns false)

This code will segfault:

Tensor matrix = CPU(kFloat).ones({1, 1, 1});
matrix.btrifact()

colesbury avatar Sep 14 '17 18:09 colesbury

I see, I think we need to mark it optional like @killeent did in some THNN functions.

zdevito avatar Sep 14 '17 18:09 zdevito