> The way I've implemented it (see the patch I sent to llvm-commits > yesterday), it's not just metadata: the intrinsic lowers to the > jumptable entry code given above. The CFI pass then generates a > function for each jump table; the function consists solely of these > intrinsic calls.Well, the intrinsic you proposed has no effect on the caller and has non-local effects on other specified functions. I'm not aware of any other intrinsic with similar behavior. On Fri, Mar 21, 2014 at 12:01:24PM -0700, Tom Roeder wrote:> On Fri, Mar 21, 2014 at 11:46 AM, Tom Roeder <tmroeder at google.com> wrote: > > On Thu, Mar 20, 2014 at 3:29 PM, Peter Collingbourne <peter at pcc.me.uk> wrote: > >> > >> > >> An alternative proposal: introduce a new function attribute named, say, > >> 'jumptable', which would cause the backend to emit a jump table entry and > >> redirect function references other than calls through the jump table. (Direct > >> function calls could call the original function directly, as an optimization.) > >> > >> The CFI pass could then consist of marking every address-taken function with > >> 'jumptable' and introducing the pointer safety checks at call sites. > > > > That's an interesting suggestion. I'll look into it. > > However, adding a new function attribute would require changing the > bitcode format, right? I thought that was to be avoided in general. > I'm not sure it makes sense to change the bitcode format to support > CFI.The bitcode format is not required to be kept stable. We've added attributes in the past to support the sanitizers and stack protection among other things, so I don't think it should be a problem to add an attribute for CFI. Thanks, -- Peter
On Fri, Mar 21, 2014 at 12:15 PM, Peter Collingbourne <peter at pcc.me.uk> wrote:>> The way I've implemented it (see the patch I sent to llvm-commits >> yesterday), it's not just metadata: the intrinsic lowers to the >> jumptable entry code given above. The CFI pass then generates a >> function for each jump table; the function consists solely of these >> intrinsic calls. > > Well, the intrinsic you proposed has no effect on the caller and has > non-local effects on other specified functions. I'm not aware of any other > intrinsic with similar behavior.I agree that it's not very similar to other intrinsics. But I don't exactly follow these statements. There are definitely intrinsics that have no effect on the caller, like llvm.var.annotation. And AFAIK, there is no non-local behavior: all the intrinsic does is lower to the labeled jump instruction; the changes to address-taken functions are done separately by the CFI pass. Note that in the patch I sent, the intrinsic only takes one argument: the function to jump to. Are there other effects in this case? So, maybe it would be better to call it something like @llvm.unconditional.jump(i8*)? I could then make it only lower to the jump and add an intrinsic that lowered to a function label as well.> > On Fri, Mar 21, 2014 at 12:01:24PM -0700, Tom Roeder wrote: >> On Fri, Mar 21, 2014 at 11:46 AM, Tom Roeder <tmroeder at google.com> wrote: >> > On Thu, Mar 20, 2014 at 3:29 PM, Peter Collingbourne <peter at pcc.me.uk> wrote: >> >> >> >> >> >> An alternative proposal: introduce a new function attribute named, say, >> >> 'jumptable', which would cause the backend to emit a jump table entry and >> >> redirect function references other than calls through the jump table. (Direct >> >> function calls could call the original function directly, as an optimization.) >> >> >> >> The CFI pass could then consist of marking every address-taken function with >> >> 'jumptable' and introducing the pointer safety checks at call sites. >> > >> > That's an interesting suggestion. I'll look into it. >> >> However, adding a new function attribute would require changing the >> bitcode format, right? I thought that was to be avoided in general. >> I'm not sure it makes sense to change the bitcode format to support >> CFI. > > The bitcode format is not required to be kept stable. We've added attributes > in the past to support the sanitizers and stack protection among other things, > so I don't think it should be a problem to add an attribute for CFI.OK, thanks.
On Fri, Mar 21, 2014 at 12:54:07PM -0700, Tom Roeder wrote:> On Fri, Mar 21, 2014 at 12:15 PM, Peter Collingbourne <peter at pcc.me.uk> wrote: > >> The way I've implemented it (see the patch I sent to llvm-commits > >> yesterday), it's not just metadata: the intrinsic lowers to the > >> jumptable entry code given above. The CFI pass then generates a > >> function for each jump table; the function consists solely of these > >> intrinsic calls. > > > > Well, the intrinsic you proposed has no effect on the caller and has > > non-local effects on other specified functions. I'm not aware of any other > > intrinsic with similar behavior. > > I agree that it's not very similar to other intrinsics. But I don't > exactly follow these statements. There are definitely intrinsics that > have no effect on the caller, like llvm.var.annotation.Yes but the purpose of such intrinsics is to communicate information about a specific value that may have an effect on analysis, optimization or code generation for that caller. On the other hand, the intrinsic you are proposing has nothing to do with the caller.> And AFAIK, > there is no non-local behavior: all the intrinsic does is lower to the > labeled jump instruction; the changes to address-taken functions are > done separately by the CFI pass. Note that in the patch I sent, the > intrinsic only takes one argument: the function to jump to. Are there > other effects in this case?The non-local effect is that the intrinsic describes the definition of a function in the global scope. Normally such definitions come from top-level entities.> So, maybe it would be better to call it something like > @llvm.unconditional.jump(i8*)? I could then make it only lower to the > jump and add an intrinsic that lowered to a function label as well.I'd imagine that might present more problems. For example, if you used either intrinsic in the middle of a regular function the behavior would not necessarily be well defined. It would be necessary to carefully document where and how these intrinsics may be used. Thanks, -- Peter