Re: [efi:next 2/3] arch/x86/boot/compressed/eboot.c:26:16: sparse: incorrect type in return expression (different modifiers)

[Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]

On 12 November 2014 16:22, Christopher Li <sparse-55XgFHCVCFZAfugRpC6u6w@public.gmane.org> wrote:
> On Wed, Nov 12, 2014 at 9:35 PM, Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>> (Pulling sparse list in)
>>
>> On Wed, 2014-11-12 at 11:31 +0100, Ard Biesheuvel wrote:
>>> > tree:   git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git next
>>> >>> arch/x86/boot/compressed/eboot.c:26:16: sparse: incorrect type in return expression (different modifiers)
>>> >    arch/x86/boot/compressed/eboot.c:26:16:    expected struct efi_config const [pure] *
>>> >    arch/x86/boot/compressed/eboot.c:26:16:    got struct efi_config *static [toplevel] efi_early
>>> >
>>>
>>> This smells like a sparse bug: __pure applies to functions only, so
>>> there is no way we could ever return something with the __pure
>>> modifier attached.
>
> That make sense.
>
>>
>> Yes, that warning does look a little strange.
>>
>> Christopher, Fengguang, what do you guys think?
>>
>
> Do you get me a small test function that reproduce the error with sparse?
>
> We can even add a test case for it.
>

Well, I spent some time playing around with this:

This one is accepted:

static __attribute__((__pure__)) int pure1(void)
{
    int i = 0;
    return i;
}

This one is not accepted:

static __attribute__((__pure__)) void *pure2(void)
{
    void *i = (void *)0;
    return i;
}

and produces

pure.c:11:16: warning: incorrect type in return expression (different modifiers)
pure.c:11:16:    expected void [pure] *
pure.c:11:16:    got void *i

I also noticed that the following triggers a warning as well:

static void*(*f)(void) = pure2;

(using the definition of pure2 as above)

pure.c:14:26: warning: incorrect type in initializer (different modifiers)
pure.c:14:26:    expected void *( *static [toplevel] f )( ... )
pure.c:14:26:    got void [pure] *( static [toplevel] *<noident> )( ... )

i.e., you are not allowed to assign a pointer to a pure function to a
non-pure function pointer, which is quite ok. The opposite should be
an error, though.

This patch

diff --git a/evaluate.c b/evaluate.c
index a5a830978bda..2e9ad8b7b31d 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -1375,7 +1375,7 @@ static int compatible_assignment_types(struct
expression *expr, struct symbol *t
                        goto Cast;
                }
                /* It's OK if the target is more volatile or const
than the source */
-               typediff = type_difference(&t->ctype, &s->ctype, 0, mod1);
+               typediff = type_difference(&t->ctype, &s->ctype, 0,
mod1 | MOD_PURE);
                if (typediff)
                        goto Err;
                return 1;

fixes the first issue, but I am not sure if it is an appropriate fix or not.

-- 
Ard.