From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ard Biesheuvel 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 Message-ID: References: <201411120724.PdeIjimc%fengguang.wu@intel.com> <1415799312.14686.332.camel@mfleming-mobl1.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: Sender: linux-efi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Christopher Li Cc: Matt Fleming , "linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Linux-Sparse , "fengguang.wu" List-Id: linux-sparse@vger.kernel.org On 19 November 2014 03:53, Christopher Li wrote: > On Wed, Nov 19, 2014 at 12:38 AM, Ard Biesheuvel > 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.