On Tue, Aug 25, 2020 at 5:49 PM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > On Tue, Aug 25, 2020 at 9:34 AM Kevin Sheldrake > <Kevin.Sheldrake@xxxxxxxxxxxxx> wrote: <SNIP> > > If I remove one section of code, unrelated to where the illegal jumps are, > reducing the overall size to 24480 instructions, the illegal jumps disappear. If > I reenable that section of code and remove a different section of code, also > unrelated to the illegal jumps, reducing the overall size to 47444 instructions, > the illegal jumps disappear again. > > I suspect it's a bug in llvm. It doesn't have a check that the branch target fits > into 16-bits. > It simply does: > llvm/lib/Target/BPF/MCTargetDesc/BPFAsmBackend.cpp > Value = (uint16_t)((Value - 8) / 8); > support::endian::write<uint16_t>(&Data[Fixup.getOffset() + 2], Value, > Endian); Could you add a log around that line to double > check that's the case? > May be you could send a patch to add an assert there? > It will help others avoid this debugging. > > In the past we've talked about extending BPF ISA with 32-bit unconditional > jump instruction. > But no one didn't come around to actually implementing it. > Once we have such insn llvm should be able to detect this 16-bit overflow > and use this new jmp insn. Hello Alexei Thank you for this insight - that's really helpful. I'll get on it when I'm back at work on Friday and I'll submit a patch. Thanks Kevin Sheldrake
