diff mbox

[tip,v6,2/5] kbuild: Add option to turn incompatible pointer check into error

Message ID 1453992270-4688-3-git-send-email-daniel.wagner@bmw-carit.de (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Wagner Jan. 28, 2016, 2:44 p.m. UTC 
With the introduction of the simple wait API we have two very
similar APIs in the kernel. For example wake_up() and swake_up()
is only one character away. Although the compiler will warn
happily the wrong usage it keeps on going an even links the kernel.
Thomas and Peter would rather like to see early missuses reported
as error early on.

In a first attempt we tried to wrap all swait and wait calls
into a macro which has an compile time type assertion. The result
was pretty ugly and wasn't able to catch all wrong usages.
woken_wake_function(), autoremove_wake_function() and wake_bit_function()
are assigned as function pointers. Wrapping them with a macro around is
not possible. Prefixing them with '_' was also not a real option
because there some users in the kernel which do use them as well.
All in all this attempt looked to intrusive and too ugly.

An alternative is to turn the pointer type check into an error which
catches wrong type uses. Obviously not only the swait/wait ones. That
isn't a bad thing either.

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 Makefile | 3 +++
 1 file changed, 3 insertions(+)

Comments

Daniel Wagner Jan. 29, 2016, 12:17 p.m. UTC | #1 
On 01/28/2016 03:44 PM, Daniel Wagner wrote:
> +# enforce correct pointer usage
> +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
> +

As it turns out there are a few fallouts by that one. I'll send fixes
for it.
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Gortmaker Jan. 29, 2016, 6:55 p.m. UTC | #2 
[Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 29/01/2016 (Fri 13:17) Daniel Wagner wrote:

> On 01/28/2016 03:44 PM, Daniel Wagner wrote:
> > +# enforce correct pointer usage
> > +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
> > +
> 
> As it turns out there are a few fallouts by that one. I'll send fixes
> for it.

Did you try non-x86 builds with this applied?   I'd be really surprised
if there were just a few, once you did allyesconfig/allmodconfig for
ARM, MIPS, PPC, etc.

P.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Wagner Feb. 1, 2016, 6:49 a.m. UTC | #3 
On 01/29/2016 07:55 PM, Paul Gortmaker wrote:
> [Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 29/01/2016 (Fri 13:17) Daniel Wagner wrote:
> 
>> On 01/28/2016 03:44 PM, Daniel Wagner wrote:
>>> +# enforce correct pointer usage
>>> +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
>>> +
>>
>> As it turns out there are a few fallouts by that one. I'll send fixes
>> for it.
> 
> Did you try non-x86 builds with this applied?   I'd be really surprised
> if there were just a few, once you did allyesconfig/allmodconfig for
> ARM, MIPS, PPC, etc.

I have tried this with non-x86 builds and apart of a few problems all
looked fine. As it turns out I was using too old cross tools from
kernel.org [1]. Luckily Fengguang's kbuild robot did catch a bunch of
them (see the patches in this series).

Since Thomas was also surprised that only a bunch of them showed up,
I'll better give it another go with more recent compilers.

cheers,
daniel

[1] https://www.kernel.org/pub/tools/crosstool
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Wagner Feb. 5, 2016, 8:16 a.m. UTC | #4 
On 02/01/2016 07:49 AM, Daniel Wagner wrote:
> On 01/29/2016 07:55 PM, Paul Gortmaker wrote:
>> [Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 29/01/2016 (Fri 13:17) Daniel Wagner wrote:
>>
>>> On 01/28/2016 03:44 PM, Daniel Wagner wrote:
>>>> +# enforce correct pointer usage
>>>> +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
>>>> +
>>>
>>> As it turns out there are a few fallouts by that one. I'll send fixes
>>> for it.
>>
>> Did you try non-x86 builds with this applied?   I'd be really surprised
>> if there were just a few, once you did allyesconfig/allmodconfig for
>> ARM, MIPS, PPC, etc.
> 
> I have tried this with non-x86 builds and apart of a few problems all
> looked fine. As it turns out I was using too old cross tools from
> kernel.org [1]. Luckily Fengguang's kbuild robot did catch a bunch of
> them (see the patches in this series).

It turns out this week was particular bad for doing anything productive.
Anyway, I found some time to fire up some cross compilers and it looks
promising.

I used the cross compiler version 5.2.1 shipped by Fedora 23
and run allyesconfig/allmodconfig for ARM, ARM64, MIPS64, PPC64
(swait-v7 and 4.5-rc2). No new errors popped up.

With some luck I get some more architectures covered soon.

cheers,
daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Gortmaker Feb. 7, 2016, 4:39 a.m. UTC | #5 
[Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 05/02/2016 (Fri 09:16) Daniel Wagner wrote:

> On 02/01/2016 07:49 AM, Daniel Wagner wrote:
> > On 01/29/2016 07:55 PM, Paul Gortmaker wrote:
> >> [Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 29/01/2016 (Fri 13:17) Daniel Wagner wrote:
> >>
> >>> On 01/28/2016 03:44 PM, Daniel Wagner wrote:
> >>>> +# enforce correct pointer usage
> >>>> +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
> >>>> +
> >>>
> >>> As it turns out there are a few fallouts by that one. I'll send fixes
> >>> for it.
> >>
> >> Did you try non-x86 builds with this applied?   I'd be really surprised
> >> if there were just a few, once you did allyesconfig/allmodconfig for
> >> ARM, MIPS, PPC, etc.
> > 
> > I have tried this with non-x86 builds and apart of a few problems all
> > looked fine. As it turns out I was using too old cross tools from
> > kernel.org [1]. Luckily Fengguang's kbuild robot did catch a bunch of
> > them (see the patches in this series).
> 
> It turns out this week was particular bad for doing anything productive.
> Anyway, I found some time to fire up some cross compilers and it looks
> promising.
> 
> I used the cross compiler version 5.2.1 shipped by Fedora 23
> and run allyesconfig/allmodconfig for ARM, ARM64, MIPS64, PPC64
> (swait-v7 and 4.5-rc2). No new errors popped up.

SOunds good ; guess my gut feeling about this causing more fallout was
off the mark.

P.
--

> 
> With some luck I get some more architectures covered soon.
> 
> cheers,
> daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Wagner Feb. 17, 2016, 1:04 p.m. UTC | #6 
On 02/07/2016 05:39 AM, Paul Gortmaker wrote:
>> I used the cross compiler version 5.2.1 shipped by Fedora 23
>> and run allyesconfig/allmodconfig for ARM, ARM64, MIPS64, PPC64
>> (swait-v7 and 4.5-rc2). No new errors popped up.
> 
> SOunds good ; guess my gut feeling about this causing more fallout was
> off the mark.

The fallouts I found are on the way to mainline or are already
in mainline:

fbdev: https://git.kernel.org/tomba/c/206fc20598157ce15597822cf01b94377e30075b
mips:  https://git.kernel.org/torvalds/c/f4d3d504198d464e406171cfa554a59bd4773d79

There was also a hickup on alpha. The patch is on the way. 

Here is the list of archs and config I tried out. First run was with the
config listed, followed by a allyesconfig and allmodconfig build. 

#    name      ARCH     CROSS_COMPILE        config
declare -a config=(
    "alpha     alpha    alpha-linux-gnu-     defconfig"
    "arm32     arm      arm-linux-gnu-       versatile_defconfig"
    "arm64     arm64    aarch64-linux-gnu-   defconfig"
    "cris10    cris     cris-linux-gnu-      etrax-100lx_defconfig"
    "frv       frv      frv-linux-gnu-       defconfig"
    "ia64      ia64     ia64-linux-gnu-      generic_defconfig"
    "m68k      m68k     m68k-linux-gnu-      amiga_defconfig"
    "mips32    mips     mips64-linux-gnu-    ar7_defconfig"
    "mips64    mips     mips64-linux-gnu-    bigsur_defconfig"
    "ppc64     powerpc  powerpc64-linux-gnu- pseries_defconfig"
    "s390      s390     s390x-linux-gnu-     defconfig"
    "sparc32   sparc    sparc64-linux-gnu-   sparc32_defconfig"
    "sparc64   sparc    sparc64-linux-gnu-   sparc64_defconfig"
    "um_x86_64 um       gcc                  defconfig"
    "x86_64    x86_64   gcc                  defconfig"

# known broken configurations on F23
#   "alpha     alpha    alpha-linux-gnu-     allyesconfig"
#   "avr32     avr32    avr32-linux-gnu-     defconfig"
#   "blackfin  blackfin bfin-linux-gnu-      BF518F-EZBRD_defconfig"
#   "cris32    cris     cris-linux-gnu-      etraxfs_defconfig"
#   "m32r      m32r     m32r-linux-gnu-      mappi3.smp_defconfig"
#   "parisc    parisc   hppa-linux-gnu-      default_defconfig"
#   "parisc64  parisc   hppa-linux-gnu-      a500_defconfig"
#   "ppc32     powerpc  powerpc64-linux-gnu- mpc512x_defconfig"
#   "sh32      sh       sh-linux-gnu-        defconfig"
#   "sh64      sh64     sh64-linux-gnu-      defconfig"
#   "tile      tile     tile-linux-gnu-      defconfig"
#   "xtensa    xtensa   xtensa-linux-gnu-    common_defconfig"
)

cheers,
daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Makefile b/Makefile
index 9d94ade..653fd08 100644
--- a/Makefile
+++ b/Makefile
@@ -767,6 +767,9 @@  KBUILD_CFLAGS   += $(call cc-option,-Werror=strict-prototypes)
 # Prohibit date/time macros, which would make the build non-deterministic
 KBUILD_CFLAGS   += $(call cc-option,-Werror=date-time)
 
+# enforce correct pointer usage
+KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
+
 # use the deterministic mode of AR if available
 KBUILD_ARFLAGS := $(call ar-option,D)