All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
To: Christopher Li <sparse-55XgFHCVCFZAfugRpC6u6w@public.gmane.org>
Cc: Matt Fleming
	<matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	"linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Linux-Sparse
	<linux-sparse-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"fengguang.wu"
	<fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Subject: Re: [efi:next 2/3] arch/x86/boot/compressed/eboot.c:26:16: sparse: incorrect type in return expression (different modifiers)
Date: Wed, 19 Nov 2014 12:31:08 +0100	[thread overview]
Message-ID: <CAKv+Gu93S6bRjAoOnWdvkAmqRtaSK2z3mzP0czdttVQPuC0sSQ@mail.gmail.com> (raw)
In-Reply-To: <CANeU7Q=uCR6P41F72J0X6c2=fgU5eefPj1NrzfnoYRtCuzYxYg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 19 November 2014 03:53, Christopher Li <sparse-55XgFHCVCFZAfugRpC6u6w@public.gmane.org> wrote:
> On Wed, Nov 19, 2014 at 12:38 AM, Ard Biesheuvel
> <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> Well, that is how sparse currently treats it, and I think that is correct.
>> If an extern declaration in a .h file specifies  __pure, then GCC may
>> legally emit fewer calls to that function than appear in the source.
>> So if the actual implementation in the .c file is not __pure, then you
>> have an error.
>
> OK. I just want to figure out what is the right thing to do here.
> It is ambiguous when the second time function declare without pure attribute,
> weather it should follow the incremental declare rule or conflicting error.
>
> If follow the C incremental declare rule, there is no error there.
> It just an incomplete declare, compiler will internally merge all the
> declare in the same scope and apply that to the final function.
>
> e.g.
> static __attribute__((noreturn)) void deadloop(void);
>
> static void deadloop(void)
> {
> }
>
> gcc will treat the second deadloop() function has noreturn attribute,
> even though it does not have one in the second time declare.
> That is why Gcc warns the function does return:
>  /tmp/noreturn.c: In function 'deadloop':
> /tmp/noreturn.c:6:1: warning: 'noreturn' function does return [enabled
> by default]
>  }
>
> I try it on gcc, gcc does not emit error for the pure attribute conflicting.
> That seems indicate gcc follow the incremental declare rule rather than
> conflicting error.
>

That may be the case, but GCC also does not warn if you are violating
the pureness rules.
For example, this function

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

is obviously not pure, but GCC does not warn about that. But yes, GCC
follows the incremental rule, so if you mark a function as 'pure' only
in the header file, but not in the definition, then it will still be
considered pure.

> Sparse has the symbol->same_symbol chain for that reason as well.
> It can be used for merging different declare of the same function.
> Some declare provide more detail specification, some did not.
>
>> """
>> static __attribute__((__pure__)) int pure2(void)
>> {
>> int i = 0;
>> return i;
>> }
>> static int pure2(void); // error here
>> """
>
> See above. I am not sure that is an error here if we follow
> the incremental declare rule.
>

I think you are right. It is fairly common practice to define
attributes for a function /definition/ by preceding it with a
definition. e.g.,

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

so this is obviously not an error.

So bottom line is that GCC does not warn about violating pureness
rules, and using the incremental declare rule seems appropriate, so
perhaps sparse should ignore pure altogether?

-- 
Ard.

  parent reply	other threads:[~2014-11-19 11:31 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <201411120724.PdeIjimc%fengguang.wu@intel.com>
     [not found] ` <201411120724.PdeIjimc%fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2014-11-12 10:31   ` [efi:next 2/3] arch/x86/boot/compressed/eboot.c:26:16: sparse: incorrect type in return expression (different modifiers) Ard Biesheuvel
     [not found]     ` <1415799312.14686.332.camel@mfleming-mobl1.ger.corp.intel.com>
     [not found]       ` <1415799312.14686.332.camel-ZqTwcBeJ+wsBof6jY8KHXm7IUlhRatedral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2014-11-12 15:22         ` Christopher Li
     [not found]           ` <CANeU7Qn8J2dn4V5Mx1WzUM9q+m=K66yUuEkN39bH_djRoBzqNA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-11-12 15:34             ` Ard Biesheuvel
     [not found]               ` <CAKv+Gu-L2EoqioZamh9arLSkXzQF4y=FDykk0YK1XvNkRGC-xg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-11-16 10:13                 ` Christopher Li
2014-11-16 17:58                   ` Ard Biesheuvel
     [not found]                     ` <CAKv+Gu8DB6tCrpTqiCSWDGU4cRO1u3hqO9-K-cON5ODDMgsx1Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-11-17  0:38                       ` Christopher Li
2014-11-17 15:35                         ` Christopher Li
     [not found]                           ` <CANeU7QkZKv+c1y-_9DsvV-EqKnmm+NjUEGxhZBqSF_5AJ+XT1g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-11-17 16:24                             ` Ard Biesheuvel
     [not found]                               ` <CAKv+Gu-c4qk-Snkc6Vs=LKwCnxVTMeBmgGjWT4qxapi-nTYX+w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-11-18 16:09                                 ` Christopher Li
2014-11-18 16:38                                   ` Ard Biesheuvel
     [not found]                                     ` <CAKv+Gu_2uwTdDT=7ghM6e2-=TpH652Q-JOOwF6oFsGFLxeKueg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-11-19  2:53                                       ` Christopher Li
     [not found]                                         ` <CANeU7Q=uCR6P41F72J0X6c2=fgU5eefPj1NrzfnoYRtCuzYxYg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-11-19 11:31                                           ` Ard Biesheuvel [this message]
2014-11-16 18:22                   ` Josh Triplett
2014-11-17  0:59                     ` Christopher Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKv+Gu93S6bRjAoOnWdvkAmqRtaSK2z3mzP0czdttVQPuC0sSQ@mail.gmail.com \
    --to=ard.biesheuvel-qsej5fyqhm4dnm+yrofe0a@public.gmane.org \
    --cc=fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-sparse-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=sparse-55XgFHCVCFZAfugRpC6u6w@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.