On 09/14/2017 09:46 PM, Philippe Mathieu-Daudé wrote:
>> +static bool cap_disas(disassemble_info *info, uint64_t pc, size_t size)
>
> I'd rather use:
>
> ..,, target_ulong code, ...
>> +{
>
> uint64_t pc = (uint64_t)code;
Why?
>
>> + bool ret = false;
>
> Isn't it cleaner to have a stubs/disas_capstone.c?
>
>> +#ifdef CONFIG_CAPSTONE
If this one function warranted a separate file of its own, maybe, but certainly
not without that.
>> + cap_mode += (info->endian == BFD_ENDIAN_BIG ? CS_MODE_BIG_ENDIAN
>> + : CS_MODE_LITTLE_ENDIAN);
>> +
>
> assert(size); ?
The existing disassembler doesn't have it. So, no?
> err = cs_open(info->cap_arch, cap_mode, &handle);
> if (err != CS_ERR_OK) {
> (*info->fprintf_func)(info->stream, "Capstone: %s\n",
> cs_strerror(err));
No. Or not without extra limits. We don't want 100k instances of this error
as we dump all of the hunks for "-d in_asm".
We'd be assuming the distro didn't do anything silly wrt compiling with "diet"
mode or with only the host cpu enabled. I really don't know what to do about
such an eventuality except continue to fall back to the binutils code.
>> + cbuf = buf = g_malloc(size);
>
> if (buf == NULL) {
> goto err2;
> }
g_malloc cannot fail.
>
> cs_free(insn, 1);
> err2:
>
>> + g_free(buf);
>> + err1:
Oops, yes.
>> +
>> + if (s.info.cap_arch >= 0 && cap_disas(&s.info, (uintptr_t)code, size)) {
>
> (target_ulong)(uintptr_t)code, ?
Why? Even if I did change the type of the argument?
The extra cast would be implied by the language.
>> + /* ??? Capstone requires that we copy the data into a host-addressable
>> + buffer first and has no call-back to read more. Therefore we need
>> + an estimate of buffer size. This will work for most RISC, but we'll
>> + need to figure out something else for variable-length ISAs. */
>> + if (s.info.cap_arch >= 0 && cap_disas(&s.info, pc, 4 * nb_insn)) {
>
> .., MIN(16 * nb_insn, TARGET_PAGE_SIZE))) ?
That's no better than what I have; it simply prints more. I need a *real*
solution. It will probably involve not re-using cap_disas but writing code
just for the monitor.
r~
