Re: [PATCH] No uprobes on ARM autiasp instructions

Per discussion we are going to address this by somehow marking these cases
of probe specifications (wildcard instructions) to not consider a failure
to attach the probe as an error.  Specifically probing a given instruction
and failing to attach should always remain an error.

On Mon, Aug 11, 2025 at 06:13:07PM -0400, Kris Van Hees wrote:
> On Mon, Aug 11, 2025 at 04:52:43PM -0400, Eugene Loh wrote:
> > On 8/11/25 13:54, Kris Van Hees wrote:
> > 
> > > On Tue, Jun 10, 2025 at 05:10:42PM -0400, [email protected] wrote:
> > > > From: Eugene Loh <[email protected]>
> > > > 
> > > > New compilers emit autiasp instructions much more liberally.
> > > > A test like test/unittest/pid/tst.entry_off0.sh, which tries
> > > > to put a probe on each instruction, may fail.
> > > > 
> > > > Signed-off-by: Eugene Loh <[email protected]>
> > > > ---
> > > >   libdtrace/dt_pid.c | 23 +++++++++++++++++------
> > > >   1 file changed, 17 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
> > > > index e2d4e540d..833e9b647 100644
> > > > --- a/libdtrace/dt_pid.c
> > > > +++ b/libdtrace/dt_pid.c
> > > > @@ -279,12 +279,17 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
> > > >   		nmatches++;
> > > >   	} else if (glob) {
> > > > -#if defined(__amd64)
> > > >   		/*
> > > > -		 * We need to step through the instructions to find their
> > > > -		 * offsets.  This is difficult on x86, which has variable
> > > > -		 * instruction lengths.  We invoke the disassembler in
> > > > -		 * libopcodes.
> > > > +		 * We need the instructions for two reasons:
> > > > +		 * = On x86, instructions have varying lengths.  So,
> > > > +		 *   to step through the instructions, we need to
> > > > +		 *   disassemble them to know what they are.
> > > > +		 *   We invoke the disassembler in libopcodes.
> > > > +		 *   (On ARM, we step through 4 bytes at a time.)
> > > > +		 * = On both x86 and arm, we want to skip certain
> > > > +		 *   instructions.  So, again, we need to know what they are.
> > > > +		 */
> > > > +		/*
> > > >   		 *
> > > >   		 * We look for the Elf pointer.  It is already stored in
> > > >   		 * file_elf in file_info_t, but getting it back over here
> > > > @@ -298,7 +303,6 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
> > > >   		GElf_Shdr shdr;
> > > >   		Elf_Data *data;
> > > >   		size_t shstrndx, off;
> > > > -		disassembler_ftype disasm;
> > > >   		/* Set things up. */
> > > >   		fd = open(pp->dpp_fname, O_RDONLY);
> > > > @@ -344,12 +348,14 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
> > > >   		/* Get the instructions. */
> > > >   		data = elf_getdata(scn, NULL);
> > > > +#if defined(__amd64)
> > > >   		/*
> > > >   		 * "Disassemble" instructions just to get the offsets.
> > > >   		 *
> > > >   		 * Unfortunately, libopcodes's disassembler() has a different
> > > >   		 * interface in binutils versions before 2.29.
> > > >   		 */
> > > > +		disassembler_ftype disasm;
> > > >   #if defined(HAVE_DIS1) == defined(HAVE_DIS4)
> > > >   #error expect disassembler() to have 1 or else 4 arguments
> > > >   #endif
> > > > @@ -390,6 +396,11 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
> > > >   			/* Newer kernels do not allow uprobes on "hlt" instructions. */
> > > >   			if ((unsigned int)disasm_info.buffer[off] == 0xf4)
> > > >   				continue;
> > > > +#else
> > > > +			/* On ARM, we cannot place uprobes on "autiasp" instructions. */
> > > > +			if (*((unsigned int *)(data->d_buf + (sym.st_value + off - shdr.sh_addr)))
> > > > +			    == 0xd50323bf)
> > > Are there symbolic names we can use here?  From an include file concerning
> > > opcodes or (worst case) define one ourselves.  From the comment, I can assume
> > > that the 32-bit hex value you give must be that instruction.  But is it an
> > > actual 4-byte instruction without any values taht can be set for different
> > > uses, etc?  Perhaps a define and a comment explaining the value might be
> > > useful here.
> > > 
> > > And perhaps do the same for the 'hlt' x86 instruction mentioned above it?
> > 
> > Quite frankly, I'm not familiar with these instructions, and the situation
> > is getting worse.  There is another x86 instruction prefix that causes a
> > problem and needs to be added -- some "multi-byte nop 0x66 0x66" thing.
> > 
> > I'm tempted to go another route.  Instead of building a list of hlt,
> > autiasp, etc., just exclude the ones that don't work.  That is, if I ask for
> > pid$pid:$mod:$fun:*, silently ignore the offsets that don't work.
> > 
> > If I remember correctly, a challenge with this approach is that you don't
> > know until later that a particular offset will be a problem. Each offset
> > would entail an extra runtime check.  That seems unfortunate (and
> > unnecessary, if one could know in advance which offsets are okay), but maybe
> > it's not such a big deal if you have a lot of probes anyhow.
> > 
> > Anyhow, I don't know these instruction sets.  It might be nice to have a
> > solution that does not require specific knowledge of instruction sets (and
> > other info).
> 
> Unfortunately, the only source for this information is currently found here
> (for arm64):
> 	arch/arm64/kernel/probes/decode-insn.c function arm_probe_decode_insn
> 
> (for x86_64):
> 	arch/x86/kernel/uprobes.c function arch_uprobe_analyze_insn
> 
> So yes, um, good luck.  Interestingly, the arm64 case seems to be easier to
> handle than the x86_64 case, yet we do not seem to run into issues on the
> x86_64 side as much, probably because the unsupported cases are sufficiently
> rare in code.
> 
> Trial and error ahead of time is going to be expensive.  But we could see about
> adding some flag to a probe program that marks it as "ignore any failures for
> enabling any of the matching probes".  Also, it would be worth looking at
> whether there is a form of multiprobe support for uprobes in BPF so that we
> can get away with a single program being attached to all the probes (saves
> memory).  But that again would need to be able to ignore attaching to any
> instructions that cannot be supported.  Not sure if you can do that.
> 
> > > > +				continue;
> > > >   #endif
> > > >   			snprintf(offstr, sizeof(offstr), "%lx", off);
> > > > -- 
> > > > 2.43.5
> > > >