[lld] r211921 - [mach-o] refactor x86_64 relocation handling.

Tim Northover t.p.northover at gmail.com
Mon Jun 30 04:54:08 PDT 2014


Hi Nick,

Couple of things I noticed while looking through. Feel free to ignore:

>    // Debug logging of symbols.
>    //for (const Symbol *sym : symbols)
> -  //  llvm::errs() << "sym: " << sym->value << ", " << sym->name << "\n";
> +  //  llvm::errs() << "  sym: "
> +  //    << llvm::format("0x%08llx ", (uint64_t)sym->value)
> +  //    << ", " << sym->name << "\n";

Should we put this in DEBUG or something, and make it official? I made
use of it the other day too.

> +      case atomizeCFString:
> +        // Break section up into compact unwind entries.
> +        size = is64 ? 32 : 16;
> +        break;

This looks like a comment copy/paste error.

> +    MachODefinedAtom *inAtom = file.findAtomCoveringAddress(section,
> +                                                            reloc.offset,
> +                                                            &offsetInAtom);
> +    if (!inAtom)
> +      return make_dynamic_error_code(Twine("no atom at r_address"));

Isn't this more of an assertion-failure error? It seems like a gappy
section shouldn't even be able to exist in an ill-formed MachO file.

>  bool KindHandler_x86_64::isPointer(const Reference &ref) {
> -  return (ref.kindValue() == X86_64_RELOC_UNSIGNED);
> +  if (ref.kindNamespace() != Reference::KindNamespace::mach_o)
> +    return false;
> +  assert(ref.kindArch() == Reference::KindArch::x86_64);
> +  return (ref.kindValue() == pointer64);

Isn't pointer64Anon also a pointer, at least for the purposes
"isPointer" gets used for

Cheers.

Tim.



More information about the llvm-commits mailing list