Skip to content

Corruption on returning composite ops from functions #95

@feltech

Description

@feltech

A function such as the following (with b and I being Tensors), which returns a composite op, cannot safely be assigned to a Tensor because the branches of the op tree go out of scope and are destroyed after the function returns.

auto const sigma = [](
	Scalar const J, auto const & b,
	Scalar const lambda, Scalar const mu) {
	return (mu / J) * (b - I) + (lambda / J) * log(J) * I;
};

It seems the construction of the op tree relies on const references (expression_t) to the branches, as opposed to moves or copies. So outside the function, those const references are now (probably) invalid and the op segfaults when evaluated.

I noticed the FASTOR_COPY_EXPR flag is available, and seems to solve the problem. But of course that's a global flag and there's presumably good performance reasons why we should use references whenever possible.

A better solution for the above case in particular is perhaps changing expression_t from

conditional_t_<is_arithmetic_v_<T>, const T, const T&>

to

conditional_t_<(is_expression_v<T> || is_arithmetic_v_<T>), const T, const T&>

so that ops are copied but tensors at the leafs remain references. This of course doesn't work if the function above was returning an op that relied on a temporary tensor created inside the function. In that case we would want to copy the tensor rather than attempting to retain a reference to it. I'm not sure how to detect that case.

We can additionally add operator overloads that accept r-value references in binary_arithmetic_ops.h. I tried that and added some logging (since debugger is useless for macros) and they do get hit. Presumably this could ensure op branches are moved into their parent rather than copied. Though I'm not sure it's worth it - there isn't much data and the compiler can probably elide perfectly well on it's own.

Annoyingly, trying to reproduce this problem in a minimal example isn't so easy (on gcc 7.5), presumably because the memory is still safe to access for some limited time, despite it going out of scope (I've seen this "feature" of gcc before).

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions