| Message ID | 20170228173519.hyq3aihtg3zouoih@macpro.local (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi Luc, Thanks for taking the time to look into this and the fix! Regards Dibyendu On 28 February 2017 at 17:35, Luc Van Oostenryck <luc.vanoostenryck@gmail.com> wrote: > On Tue, Feb 28, 2017 at 06:03:05PM +0100, Luc Van Oostenryck wrote: >> On Tue, Feb 28, 2017 at 4:09 PM, Luc Van Oostenryck >> <luc.vanoostenryck@gmail.com> wrote: >> > There is indeed some problems regarding this, we looked a bit at this >> > some weeks ago. However I firmly believe that the information about >> > the type belong to the operations and not the values. >> >> I've taken a very quick look at this "mt->foo = (void *)0" >> The type info is perfectly present. >> If in sparse-llvm.c:output_op_store() you add somewhere something like: >> fprintf(stderr, "-> %s\n", show_typename(insn->type)); >> You will see that it display the expected type: "int *". >> This is all the type info needed: it's the type of insn->target (the >> value to be stored) >> and the type of the dereferencing of insn->src (the (base) address). >> >> The problem is that output_op_store() doesn't use this info, it tries to deduce >> this type via pseudo_to_value() but pseudo_to_value() wrongly assumes that all >> PSEUDO_VALUE-pseudo are integer. > > > Not very pretty and incomplete but the following patch allow sparse-llvm > to compile this: > struct mytype { > int *foo; > }; > > extern void init_mytype(struct mytype *mt); > void init_mytype(struct mytype *mt) > { > mt->foo = (int *)mt; > mt->foo = (void *)mt; > mt->foo = (int *)0; > mt->foo = (void *)0; > mt->foo = (void *)(long)0; > } > > It fail at " ... = (... *)1;" though. > > > diff --git a/sparse-llvm.c b/sparse-llvm.c > index 9f362b3ed..9e0450ae7 100644 > --- a/sparse-llvm.c > +++ b/sparse-llvm.c > @@ -306,6 +306,7 @@ static void pseudo_name(pseudo_t pseudo, char *buf) > static LLVMValueRef pseudo_to_value(struct function *fn, struct instruction *insn, pseudo_t pseudo) > { > LLVMValueRef result = NULL; > + LLVMTypeRef type; > > switch (pseudo->type) { > case PSEUDO_REG: > @@ -360,7 +361,21 @@ static LLVMValueRef pseudo_to_value(struct function *fn, struct instruction *ins > break; > } > case PSEUDO_VAL: > - result = LLVMConstInt(insn_symbol_type(fn->module, insn), pseudo->value, 1); > + type = insn_symbol_type(fn->module, insn); > + switch (LLVMGetTypeKind(type)) { > + case LLVMPointerTypeKind: > + assert(!pseudo->value); > + result = LLVMConstPointerNull(type); > + break; > + case LLVMIntegerTypeKind: > + result = LLVMConstInt(type, pseudo->value, 1); > + break; > + default: > + assert(0); > + } > break; > case PSEUDO_ARG: { > result = LLVMGetParam(fn->fn, pseudo->nr - 1); > @@ -626,6 +641,7 @@ static void output_op_store(struct function *fn, struct instruction *insn) > > addr = calc_memop_addr(fn, insn); > > target_in = pseudo_to_value(fn, insn, insn->target); > > /* perform store */ -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 28 February 2017 at 17:35, Luc Van Oostenryck <luc.vanoostenryck@gmail.com> wrote: > Not very pretty and incomplete but the following patch allow sparse-llvm > to compile this: > struct mytype { > int *foo; > }; > > extern void init_mytype(struct mytype *mt); > void init_mytype(struct mytype *mt) > { > mt->foo = (int *)mt; > mt->foo = (void *)mt; > mt->foo = (int *)0; > mt->foo = (void *)0; > mt->foo = (void *)(long)0; > } > > It fail at " ... = (... *)1;" though. > > > + type = insn_symbol_type(fn->module, insn); > + switch (LLVMGetTypeKind(type)) { > + case LLVMPointerTypeKind: > + assert(!pseudo->value); > + result = LLVMConstPointerNull(type); > + break; > + case LLVMIntegerTypeKind: > + result = LLVMConstInt(type, pseudo->value, 1); > + break; > + default: > + assert(0); > + } Following modified version should handle values than 0. LLVMTypeRef type = insn_symbol_type(fn->module, insn); switch (LLVMGetTypeKind(type)) { case LLVMPointerTypeKind: if (pseudo->value == 0) result = LLVMConstPointerNull(type); else result = LLVMConstIntToPtr(LLVMConstInt(LLVMIntType(bits_in_pointer), pseudo->value, 1), type); break; case LLVMIntegerTypeKind: result = LLVMConstInt(type, pseudo->value, 1); break; default: assert(0); } Regards Dibyendu -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 28, 2017 at 06:08:16PM +0000, Dibyendu Majumdar wrote: > > Following modified version should handle values than 0. Good, I'll cook a patch based on this. But I still have a fundamental problem with pseudo_to_value(). It's working with your example because we're using an OP_STORE and we're interesting in the type of the "target" and for this instruction (admittedly, like most instructions) insn->type is the type of insn->target. With insn->src or OP_SETNE, the result would have been incorrect. So while this patch is a little improvement and doesn't hurt, it's still not a good solution. What I last wrote on the subject the 29th Jan still holds (the "type" info should be given as argument to pseudo_to_value() instead of passing the pointer to the instruction) or at least something along this line. Luc Van Oostenryck -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/sparse-llvm.c b/sparse-llvm.c index 9f362b3ed..9e0450ae7 100644 --- a/sparse-llvm.c +++ b/sparse-llvm.c @@ -306,6 +306,7 @@ static void pseudo_name(pseudo_t pseudo, char *buf) static LLVMValueRef pseudo_to_value(struct function *fn, struct instruction *insn, pseudo_t pseudo) { LLVMValueRef result = NULL; + LLVMTypeRef type; switch (pseudo->type) { case PSEUDO_REG: @@ -360,7 +361,21 @@ static LLVMValueRef pseudo_to_value(struct function *fn, struct instruction *ins break; } case PSEUDO_VAL: - result = LLVMConstInt(insn_symbol_type(fn->module, insn), pseudo->value, 1); + type = insn_symbol_type(fn->module, insn); + switch (LLVMGetTypeKind(type)) { + case LLVMPointerTypeKind: + assert(!pseudo->value); + result = LLVMConstPointerNull(type); + break; + case LLVMIntegerTypeKind: + result = LLVMConstInt(type, pseudo->value, 1); + break; + default: + assert(0); + } break; case PSEUDO_ARG: { result = LLVMGetParam(fn->fn, pseudo->nr - 1); @@ -626,6 +641,7 @@ static void output_op_store(struct function *fn, struct instruction *insn) addr = calc_memop_addr(fn, insn);