memory corruption fixes in zone cut

Several operations were not safe to call on empty pack and would return invalid memory. If the pack would have reserved space, but would be empty (length = 0), it's head would be NULL but tail would be array address (pack->at + 0). This is mostly checked by caller, but it wasn't in several places (object deletion). The address election would then crash like this:

#0  pack_obj_len (it=0x2 <error: Cannot access memory at address 0x2>) at /cfsetup_build/knot-resolver/tmp/build/knot-resolver/lib/generic/pack.h:112
#1  eval_addr_set (score=<optimized out>, addr=0x7ffea96f7120, opts=0x56376fc7beb6, ctx=<optimized out>, ctx=<optimized out>, addr_set=<optimized out>, addr_set=<optimized out>) at lib/nsrep.c:98
#2  0x00007f38b0549e6e in kr_nsrep_elect_addr (qry=0x7ffea96f7060, qry@entry=0x56376fc7bea0, ctx=0xa) at lib/nsrep.c:374
#3  0x00007f38b054d2a4 in kr_resolve_produce (request=request@entry=0x56376ea0f370, dst=dst@entry=0x56376ea0f850, type=type@entry=0x7ffea96f725c, packet=<optimized out>) at lib/resolve.c:1430
#4  0x000056376c6ffdfb in qr_task_step (task=0x56376ea0f7f8, packet_source=0x7ffea96f7370, packet=0x56376e5409a0) at daemon/worker.c:1590

I think it's better to return invalid pointers in this case, instead of indeterminate pointers as it's at least analyzable.

Hmm, I fixed the main part of this yesterday in a private branch, but in a different way, i.e. always returning &pack.at[pack.len] – that will always be either NULL or a valid location (i.e. inside allocated or just one byte past). Are you convinced that's wrong?

assigned to @vcunat

That will fix this problem too, but it's also more dangerous as it breaks assumptions like:

uint8_t *it = pack_head(pack);
if (it) {
   pack_obj_len(it)
}

In general, I think it's safer (and easier for static analyzer) to return invalid pointers when they are invalid and not pointing to indeterminate memory. Semantically, a packed array doesn't have a first element if it's empty, so it doesn't feel right to return it.

Hmm, yes, it should catch more bad usage that way; I'll revisit this later.

added 139 commits

• abebad58...7b33fb61 - 137 commits from branch master
• 270d9964 - lib/nsrep: refactored copypasta
• 7617bea1 - lib/generic/pack: switch to NULL on empty pack iterator

Compare with previous version

enabled an automatic merge when the pipeline for 7617bea1 succeeds

I still kept some non-NULL assertions, so it's a kind of hybrid of our two approaches.

merged

mentioned in commit d0e32c6f