On Sun, 2012-03-25 at 10:48 +0200, Johannes Berg wrote: > Hi, > > > struct anon_vma *page_lock_anon_vma(struct page *page) > > __cond_acquires(RCU) > > __cond_acquires(page_lock_anon_vma(page)->root->lock); > > > > Meaning that if the return value is true (non-zero), it would acquire > > that lock/context. One could of course add some shorter means of > > referring to the return value, but simply using the function in the > > expression should be simple enough. > > > > In order to implement this I guess we need to extend the > > __attribute__((context(expr,in,out))) thing. > > > > Currently in,out are explicit value constants, but I guess if we make > > them expressions we could evaluate them and get dynamic behaviour. > > > > Thus allowing something like: > > > > int spin_trylock(spinlock_t *lock) > > __attribute__((context(lock, 0, !!spin_trylock(lock)); > > > > meaning that the context would be incremented by 1 if the return value > > were true. > > > > Having only briefly looked at the sparse source, is this feasible to > > implement or do we get chicken/egg problems wrt using a function before > > its declaration is complete, and referring a return value before the > > function is part of an expression? > > > > If this yields problems, are there better ways of solving this issue? > > I once looked at all of this (which I suspect you saw, given that you're > CC'ing me) but all my changes ended up being reverted since they broke > things so maybe I'm not the right person to ask ... :-) Yeah, I looked at the git log of validation/context.c :-) > I played with having a "RETURN" builtin (or something like that) to use > inside here but it didn't really work out well, I don't think that was > what ended up going upstream though. > > However, I don't think using the function call etc. is a good idea, to > me that makes it look too much like you could put arbitrary code there, > but since this doesn't even exist at runtime ... Fair enough.. the explicit reference to the return value is indeed more convenient and as you show below makes it possible to do other things as well. > However, note that today sparse doesn't evaluate anything in the > context, it doesn't even look at the first argument. So another thing > you can't really annotate well is things like this: > > struct foo_object *get_locked_object(...); > > This is why I used RETURN to give the return value a name, so you could > write > __acquires(&RETURN->lock) Right, but if it doesn't actually evaluate the expression used in the context this is going to be problematic. > But I was also trying to make sparse actually evaluate the first > argument so it could tell the difference between two locks, which you > might not even care about ... (it would be nice though I think) Right, so what I thought we could maybe do is inject code in the callsites of these functions. So after the OP_CALL emit a piece of code that works like the __context__ stmt and can reference the return value that exists at that point. This also makes the conditional thing quite simple to do. -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
