[PATCH] Add MCSymbolizer for disassembled instruction symbolization/annotation.

[Ahmed Bougacha]

> On May 17, 2013, at 1:36 PM, Quentin Colombet <qcolombet at apple.com> wrote:
>  In my opinion, the diff would have been simpler if you did not change the
> order of the arguments of tryAddingSymbolicOperand.
>  What was the rational for changing that order?

This was done for consistency: the X86 and ARM versions of the
functions had a different order. At least this settles that!

>  Generally speaking the patch looks good to me, but I am concerned about the
> feature we lose for ARM.
>
>  Jim, what do you think?

On Fri, May 17, 2013 at 1:37 PM, Jim Grosbach <grosbach at apple.com> wrote:
>
> Yes, it’s very important to not lose the handling of the low/high ARM
> relocations. Their variant kinds should definitely stay in the
> target-specific code, as they have no relation to anything but ARM. In
> general, the symbolizer needs to be able to interface to target specific
> code to handle things like this.

I updated the patch, adding the MCRelocationInfo class, which is
mostly useful for improved symbolization on relocatable objects.
There are implementations for both ELF and Mach-O X86-64, and a
minimal ARM one (supporting the llvm-c/Disassembler.h VariantKinds. I
sadly can't really test this, as this is only used in otool).

Symbolization now also works (to some extent) for all formats, as it
mostly relies on libObject APIs.

Thanks for reviewing!

> -Jim
>
>
>
> ================
> Comment at: lib/MC/MCDisassembler.cpp:26
> @@ +25,3 @@
> +  assert(ctx != 0 && "No MCContext given for symbolic disassembly");
> +  setSymbolizer(new MCExternalSymbolizer(*ctx, getOpInfo, symbolLookUp,
> +                                         disInfo));
> ----------------
> I guess that this method is called only once, but in case, there is the
> delete counterpart of this new?
>
> ================
> Comment at: lib/MC/MCExternalSymbolizer.cpp:117
> @@ +116,3 @@
> +  // variantkinds to MCSymbolRefExpr?
> +
> +  //if (SymbolicOp.VariantKind == LLVMDisassembler_VariantKind_ARM_HI16)
> ----------------
> As you noted it in your initial comment, this is a regression compared to
> what we had.
> ARM is an important target and I am not sure this is acceptable.
>
>
> http://llvm-reviews.chandlerc.com/D801
>
>