On Sun, Jan 29, 2017 at 12:26:40AM +0000, Dibyendu Majumdar wrote: > Hi Luc, > > On 27 January 2017 at 20:43, Luc Van Oostenryck > <luc.vanoostenryck@xxxxxxxxx> wrote: > > To get the type of malloc()'s arg and this is done by calling > > pseudo_to_value() with the malloc instruction and the pseudo > > for the size (but no info about this pseudo being malloc()'s arg). > > LLVMConstInt() is then called with the constant value and the type > > is given by calling insn_symbol_type() with the malloc instruction > > but again, it's not possible to get the right type without specifying > > we try to get the type of the first argument of the called function > > and not the type of the result. > > > > I think that there is a problem wherever pseudo_to_value() is being > used and the pseudo is an integer constant. Indeed, and possibly in other cases too. > Firstly the logic for > determining the size of the constant needs to cover all cases and > secondly depending upon the context the constant may need to be cast > to a pointer. So while the patch you mentioned before tries to solve > this for comparison operations, I think that the solution needs to > cater for all use cases not just those. The handling of arguments is > an example of this. Absolutely, the patch I wrote was only to fix a sparse-llvm breakage I saw after some changes with sparse dealing of comparisons. I didn't realized that the problem was bigger. > > My suggestion is that pseudo_to_value() for PSEUDO_VAL should always > return an integer constant of type 'long long' and the caller of > pseudo_to_value() should adjust the constant to right size (by > truncating or extending) or to pointer type if necessary as the caller > has more information. For instance, in the handling of OP_CALL, the > function output_op_call() knows when the call is for an argument, etc. > Currently pseudo_to_value() tries to work out the integer size, but > cannot do this correctly due to lack of information, and also even if > it did work out the size, the cast to pointer would be missed I think. > > Does this make sense? Yes, it make a lot of sense but I'm not so sure about the approach. * I'm not sure how easy it is to adjust the constant size/type. * adjusting the constant size/type instead of gettign directly the right type will create a superfluous cast (not a big deal as LLVM will optimize it away later but well ...). * I think that in general passing the pointer to the instruction to pseudo_to_value() and insn_symbol_type() is an error since what is really needed is the type, thus those function can only make assumptions about the type, maybe most of the time a correct one, but they can't do for all cases. * I think that this problem would be better handled if the caller could directly give the type information to pseudo_to_value() instead of adjusting it after the fact (but I have no idea how difficult and how much work it will be). * I have no idea about the constant-to-pointer case. * I think you will have a problem with void pointer since in this case sparse doesn't have the information about the real type (but maybe it will be easy to solve, I have no idea). Note: I known next to nothing about LLVM Note: I've added in CC the authors of sparse-llvm.c Regards, Luc Van Oostenryck -- 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
