DAOS-17681 object: fix bug in obj_ec_iom_merge/iom_recx_merge()

[liuxuezhao]

The original code possibly cause assertion or invalid memory access. Add a test case to reproduce the bug and verify the fix.

Steps for the author:

• Commit message follows the guidelines.
• Appropriate Features or Test-tag pragmas were used.
• Appropriate Functional Test Stages were run.
• At least two positive code reviews including at least one code owner from each category referenced in the PR.
• Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

• Gatekeeper requested (daos-gatekeeper added as a reviewer).

[github-actions]

Ticket title is 'daos container clone: daos_obj_fetch unexpectedly returns 0 sg_nr_out'
Status is 'Open'
Labels: 'ALCF,hpe_cluster'
https://daosio.atlassian.net/browse/DAOS-17681

[mchaarawi]

does that mean now we assert for the case for any EC array fetch with ioms != NULL and more than one iod (nr > 1 array iods)

orr_tgt_nr counts targets only, not targets * iod_nr ?
A per-iod counter array in obj_reasb_req would fix this i suppose

[liuxuezhao]

right, orr_tgt_nr counts targets only.
uses it to check it is the first or last reply.

[mchaarawi]

With multiple iods the truncation count of iod 0 leaks into iod 1's iom_nr_out.
Same issue as above; it needs to be per-iod.

[liuxuezhao]

you are right, I'll use copilot to fix it.

[mchaarawi]

suggested:

if (first)
        dst->iom_type = src->iom_type;
if (dst->iom_size == 0)
        dst->iom_size = src->iom_size;

[mchaarawi]

we should also reset
/* output field, the caller may pass it uninitialized */
dst->iom_nr_out = 0;
if first.

[liuxuezhao]

OK, I'll refresh it

[mchaarawi]

iom_nr_out can be unitialized from user, so assert is not favorable here. if the below initialization is fixed, then this is fine.

[liuxuezhao]

sure, let's set it to 0 for first.

[mchaarawi]

one mention from copilot review:

Not addressed, but adjacent:
on the -DER_FETCH_AGAIN retry path, cli_obj.c:5266 calls obj_reasb_io_fini(..., true) → obj_reasb_req_fini() which memsets the whole reasb_req, dropping orr_iom_realloc. The internally-allocated dst->iom_recxs survives on the user's iom, so the retry sees a non-NULL iom_recxs with orr_iom_realloc == 0 and will stop growing it — producing iom_nr_out > iom_nr on an iom DAOS allocated itself, which violates the documented contract. Worth handling since this patch is what makes that path observable.

[liuxuezhao]

@mchaarawi the fix and new test in the 2nd commit, thx

[wangshilong]

This is a important fix, shall we make it to 2.8.1?

[liuxuezhao]

This is a important fix, shall we make it to 2.8.1?

looks better to be in 2.8.1. let's try to backport if possible.