This is an archive of the discontinued LLVM Phabricator instance.

[ObjC] type method metadata `_imp`, messenger routine at callsite with program address space
ClosedPublic

Authored by mhjacobson on Oct 19 2021, 5:15 PM.

Details

Summary

On targets with non-default program address space (e.g., Harvard architectures), clang crashes (example backtrace below) when emitting Objective-C method metadata, because the address of the method IMP cannot be bitcast to i8*. It similarly crashes at messenger callsite with a failed bitcast.

Define the _imp field instead as i8 addrspace(1)* (or whatever the target's program address space is). And in getMessageSendInfo(), create signatureType by specifying the program address space.

Add a regression test using the AVR target. Test failed previously and passes now. Checked codegen of the test for x86_64-apple-darwin19.6.0 and saw no difference, as expected.

Backtrace (when emitting method metadata):

Assertion failed: (CastInst::castIsValid(Instruction::BitCast, C, DstTy) && "Invalid constantexpr bitcast!"), function getBitCast, file /Users/matt/src/llvm/llvm/lib/IR/Constants.cpp, line 2224.

...

7  libsystem_c.dylib        0x00007fff6ec09ac6 err + 0
8  clang-14                 0x0000000101800518 llvm::ConstantExpr::getBitCast(llvm::Constant*, llvm::Type*, bool) + 120
9  clang-14                 0x000000010394ca70 clang::CodeGen::ConstantAggregateBuilderBase::addBitCast(llvm::Constant*, llvm::Type*) + 48
10 clang-14                 0x0000000103999ac2 (anonymous namespace)::CGObjCNonFragileABIMac::emitMethodConstant(clang::CodeGen::ConstantArrayBuilder&, clang::ObjCMethodDecl const*, bool) + 402
11 clang-14                 0x0000000103999102 (anonymous namespace)::CGObjCNonFragileABIMac::emitMethodList(llvm::Twine, (anonymous namespace)::(anonymous namespace)::MethodListType, llvm::ArrayRef<clang::ObjCMethodDecl const*>) + 1074
12 clang-14                 0x000000010399a4c0 (anonymous namespace)::CGObjCNonFragileABIMac::BuildClassRoTInitializer(unsigned int, unsigned int, unsigned int, clang::ObjCImplementationDecl const*) + 1376
13 clang-14                 0x0000000103990b9b (anonymous namespace)::CGObjCNonFragileABIMac::GenerateClass(clang::ObjCImplementationDecl const*) + 1995
14 clang-14                 0x0000000103bb87d1 clang::CodeGen::CodeGenModule::EmitTopLevelDecl(clang::Decl*) + 2065
15 clang-14                 0x0000000103db5bf2 (anonymous namespace)::CodeGeneratorImpl::HandleTopLevelDecl(clang::DeclGroupRef) + 146
16 clang-14                 0x0000000103b61014 clang::BackendConsumer::HandleTopLevelDecl(clang::DeclGroupRef) + 228
17 clang-14                 0x00000001070d7425 clang::ParseAST(clang::Sema&, bool, bool) + 533

Diff Detail

Event Timeline

mhjacobson created this revision.Oct 19 2021, 5:15 PM
mhjacobson requested review of this revision.Oct 19 2021, 5:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 19 2021, 5:15 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Ping. Is there anyone more suited to reviewing this patch?

dylanmckay accepted this revision.Dec 1 2021, 3:50 AM

LGTM, nice work.

This revision is now accepted and ready to land.Dec 1 2021, 3:50 AM
rjmccall accepted this revision.Dec 2 2021, 8:26 PM

Well, I don't envy you the amount of work you're going to have to do to get ObjC working on a Harvard architecture, but this patch LGTM as progress.

Is AVR really going to be using the Darwin ObjC runtime, though? Are you planning to fork an Apple source drop?

Well, I'm hoping it doesn't prove to be too much work. 🙂 With this fix and D112049, most things are working quite well, though I'm sure I'll uncover more interesting things as I go.

As for the runtime, I've built my own: https://github.com/mhjacobson/avr-objc. It conforms to the Mac OS X non-fragile-ivars ABI.

By the way, I don't have write privs to LLVM, so assuming this is good to go, could someone commit this for me? Thanks for all your help!

Happy new year! If this change is acceptable, could someone please merge it for me? (And if not, could someone point out what needs more work?) Thanks for your help!

This revision was landed with ongoing or failed builds.Aug 4 2022, 2:42 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2022, 2:42 AM