vsk added inline comments.
================ Comment at: lib/CodeGen/CodeGenFunction.cpp:445 + auto *GV = new llvm::GlobalVariable(CGM.getModule(), Addr->getType(), + /*isConstant=*/false, + llvm::GlobalValue::PrivateLinkage, Addr); ---------------- pcc wrote: > vsk wrote: > > pcc wrote: > > > This can be constant I think. > > Can the value of the global change if a dynamic library is unloaded? I'm > > unsure about this case: A.dylib and B.dylib both have linkonce_odr > > definitions of X, dyld picks the definition from A, and A is unloaded. > I don't think it is possible to get into a situation where a global's address > can change if a library is unloaded. In your scenario, suppose that B takes > the address of X and then A is dlclosed. In that scenario the program would > rightly expect the address to continue to be valid; to do otherwise would > break the C++ standard's guarantee that each global has a unique address. > That I believe is why POSIX does not provide a mechanism to require that a > DSO is unloaded, only to request it to be unloaded; see the specification of > dlclose [0]. > > The Linux manpage for dlclose is more specific: "If the reference count > drops to zero and no other loaded libraries use symbols in it, then the > dynamic library is unloaded." I believe that any references to X in B would > count as a use of X in A. > > [0] http://pubs.opengroup.org/onlinepubs/009695399/functions/dlclose.html I see, thanks for explaining! ================ Comment at: lib/CodeGen/CodeGenFunction.cpp:463 + auto *PCRelAsInt = + Builder.CreatePtrToInt(EncodedAddr, IntPtrTy, "encoded_addr.int"); + auto *FuncAsInt = Builder.CreatePtrToInt(F, IntPtrTy, "func_addr.int"); ---------------- pcc wrote: > vsk wrote: > > vsk wrote: > > > pcc wrote: > > > > pcc wrote: > > > > > Maybe use `Int32Ty` (here and below). That should be sufficient under > > > > > the small code model. > > > > Sorry, I meant that the difference could be truncated to `Int32Ty`, and > > > > stored as an integer, not a pointer. > > > I tried this out but it resulted in a mysterious dyld crash after the > > > indirect callee returns, which I've yet to understand. I'll have to try > > > using a Debug build of dyld to see what's happening. > > @pcc It turned out that the jump we encode in getUBSanFunctionSignature not > > correct if we truncate a 64-bit address to a 32-bit address. If we jump to > > +8 instead of +12, we can make the truncation work :). There is no dyld bug. > > > > That said, I think we should store the pc-rel address as an IntPtrTy to > > keep things a bit simpler. We could avoid logic to truncate/sign-extend > > conditionally, and not emit those instructions at all. Wdyt? > The problem is that COFF does not have a relocation that can express a 64-bit > relative reference. If we want something that will work with all object > formats, 32-bit is probably best. Got it. https://reviews.llvm.org/D37597 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
