| Message ID | 20190924070852.GA24834@mwanda (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | parse: Fix sign extension in casting enums | expand |
On Tue, Sep 24, 2019 at 12:09 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > The problem is the sign isn't extended properly when this casts an int > to a long. The expr->ctype has to be the original int ctype for the > cast_value() call so that the "value = get_longlong(old);" expansion > works correctly. What happens if you just remove the if (ctype->bit_size == base_type->bit_size) { expr->ctype = base_type; continue; } part entirely? IOW, leave just cast_value(expr, base_type, expr, ctype); expr->ctype = base_type; in place unconditionally? I _think- that should be the simpler correct fix, but I'll leave it to Luc to think about it more. That simpler alternate patch attached, Linus
On Tue, Sep 24, 2019 at 4:23 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > I _think- that should be the simpler correct > fix, but I'll leave it to Luc to think about it more. Side note: I don't think I've seen anything from Luc on the git list since April, and the last commit is early April too. It may be that sparse has lost its maintainer.. Linus
On Tue, Sep 24, 2019 at 04:23:34PM -0700, Linus Torvalds wrote: > On Tue, Sep 24, 2019 at 12:09 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > The problem is the sign isn't extended properly when this casts an int > > to a long. The expr->ctype has to be the original int ctype for the > > cast_value() call so that the "value = get_longlong(old);" expansion > > works correctly. > > What happens if you just remove the > > if (ctype->bit_size == base_type->bit_size) { > expr->ctype = base_type; > continue; > } > > part entirely? IOW, leave just > > cast_value(expr, base_type, expr, ctype); > expr->ctype = base_type; > > in place unconditionally? I _think- that should be the simpler correct > fix, but I'll leave it to Luc to think about it more. Yes, I agree. Dan's patch is the obvious one solving the problem (expr->ctype adjusted too early) but yours should be equivalent since bit_size check is also done in cast_value(). I still need to run the tests tough. That being said, I think that using cast_value() is error-prone: expr's ctype should probably be changed there. but this will be in a separate patch anyway. > That simpler alternate patch attached, Thanks. -- Luc
On Tue, Sep 24, 2019 at 04:26:48PM -0700, Linus Torvalds wrote: > On Tue, Sep 24, 2019 at 4:23 PM Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > I _think- that should be the simpler correct > > fix, but I'll leave it to Luc to think about it more. > > Side note: I don't think I've seen anything from Luc on the git list > since April, and the last commit is early April too. It may be that > sparse has lost its maintainer.. Sorry, I just had some issues and I'm stil catching up. -- Luc
diff --git a/parse.c b/parse.c index 7954343..5ced2f4 100644 --- a/parse.c +++ b/parse.c @@ -890,10 +890,12 @@ static void cast_enum_list(struct symbol_list *list, struct symbol *base_type) expr->ctype = &int_ctype; continue; } - expr->ctype = base_type; - if (ctype->bit_size == base_type->bit_size) + if (ctype->bit_size == base_type->bit_size) { + expr->ctype = base_type; continue; + } cast_value(expr, base_type, expr, ctype); + expr->ctype = base_type; } END_FOR_EACH_PTR(sym); }
The problem is the sign isn't extended properly when this casts an int to a long. The expr->ctype has to be the original int ctype for the cast_value() call so that the "value = get_longlong(old);" expansion works correctly. Fixes: 604a148a73af ("enum: fix cast_enum_list()") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- parse.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)