Eliminate duplicate LifetimeStart in CreateAllocaBPFInit, dedupe hoisting code

[Birch-san]

Resolves #1787

Checklist

• Language changes are updated in docs/reference_guide.md
  • No language changes
• User-visible and non-trivial changes updated in CHANGELOG.md
  • Though this affects codegen, it eliminates a tautology, so BPF bytecode expected to be equivalent.
• The new behaviour is covered by tests
  • No new behaviour

[danobi]

LGTM with 1 nit, sorry about slow review

[Birch-san]

@danobi I've added a comment to hoist(), as requested.

@fbs I expect you'll be interested in the ELF differences, so I've run it:

sudo ./scripts/compare_tool_elf.sh `realpath ./build-b3df338/src/bpftrace` `realpath ./build-dedupe-alloca-init/src/bpftrace` llvm-objdump-8 `realpath ./tools`
[sudo] password for alex: 
Using version bpftrace v0.12.0-45-gb3df3 and bpftrace v0.12.0-46-gec42
Checking bashreadline
Checking biolatency
Checking biosnoop
Checking biostacks
Checking bitesize
Checking capable
Checking cpuwalk
Checking dcsnoop
Checking execsnoop
Checking gethostlatency
Checking killsnoop
Checking loads
Checking mdflush
Checking naptime
Checking oomkill
Checking opensnoop
Checking pidpersec
Checking runqlat
Checking runqlen
Checking setuids
Checking statsnoop
Checking swapin
Checking syncsnoop
Checking syscount
Checking tcpaccept
Checking tcpconnect
###############################
Change detected for script: tcpconnect
Checking tcpdrop
###############################
Change detected for script: tcpdrop
Checking tcplife
Checking tcpretrans
Checking tcpsynbl
Checking threadsnoop
Checking vfscount
Checking vfsstat
Checking writeback
Checking xfsdist

this isn't entirely surprising; tcpconnect.bt and tcpdrop.bt are noteworthy because they rely heavily on hoisted declarations:

https://github.com/iovisor/bpftrace/blob/master/tools/tcpconnect.bt#L38-L44

if ($inet_family == AF_INET) {
  $daddr = ntop($sk->__sk_common.skc_daddr);
  $saddr = ntop($sk->__sk_common.skc_rcv_saddr);
} else {
  $daddr = ntop($sk->__sk_common.skc_v6_daddr.in6_u.u6_addr8);
  $saddr = ntop($sk->__sk_common.skc_v6_rcv_saddr.in6_u.u6_addr8);
}

the variables $saddr / $daddr are declared for the first time in the positive branch of the condition, but since the variables are assigned-to in the negative branch also: the declaration needs to be hoisted to a block which is a common ancestor of both these blocks (and our easy solution is to hoist it to the top of the entry block).

so of course these tools are sensitive to changes in hoisting.
let's have a look at the diff:

tcpconnect.bt (left: master; right: branch):

tcpdrop.bt (left: master; right: branch):

In both cases, it looks like it's just a minor reshuffle of instructions.
r6 holds a zero at that time. so this is a memset(0) that's being moved earlier. none of the other instructions in this changed portion seem to have any dependency on this memory, so I think it can be freely moved.

I used the bpftrace built in my branch to run tcpconnect.bt, and it successfully traced a curl request that I issued.

So, I think the shuffle in the binary output is inconsequential.

[danobi]

LGTM but looks like it needs fixture regeneration

[Birch-san]

@danobi thanks; I've now rebased and regenerated fixtures.

[danobi]

Thanks for working on this!