[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