Re: clang warning: implicit conversion in intel_ddi.c:1481

2018-02-06 Thread Ruben Safir
> 
> We are interested 


who is we?

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: clang warning: implicit conversion in intel_ddi.c:1481

2018-02-03 Thread Knut Omang
On Fri, 2018-02-02 at 16:50 +0100, Greg KH wrote:
> On Fri, Feb 02, 2018 at 04:37:55PM +0200, Jani Nikula wrote:
> > On Fri, 02 Feb 2018, Greg KH  wrote:
> > > On Fri, Feb 02, 2018 at 12:44:38PM +0200, Jani Nikula wrote:
> > >> 
> > >> +Knut, Fengguang
> > >> 
> > >> On Fri, 02 Feb 2018, Greg KH  wrote:
> > >> >- If clang now builds the kernel "cleanly", yes, I want to take
> > >> >  warning fixes in the stable tree.  And even better yet, if you
> > >> >  keep working to ensure the tree is "clean", that would be
> > >> >  wonderful.
> > >> 
> > >> So we can run sparse using 'make C=1' and friends, or other static
> > >> analysis tools using 'make CHECK=foo C=1', as long as the passed command
> > >> line params work. There was work by Knut to extend this make checker
> > >> stuff [1]. Since mixing different HOSTCC's in a single workdir seems
> > >> like a bad idea, I wonder how hard it would be to make clang work like
> > >> this:
> > >> 
> > >> $ make CHECK=clang C=1
> > >> 
> > >> Or using Knut's wrapper. Feels like that could increase the use of clang
> > >> for static analysis of patches.
> > >
> > > Why not just build with clang itself:
> > >   make CC=clang
> > 
> > Same as HOSTCC, mixing different CC's in a single build dir seems like a
> > bad idea. Sure, everyone can setup a separate build dir for clang, but
> > IMHO having 'make CHECK=clang C=1' work has least resistance. YMMV.
> 
> "O=some_output_dir" is your friend.  If you aren't doing that already
> for your test builds, you don't know what you are missing :)

I use O= a lot myself - so good not to have all the output files "pollute" the 
source
tree, and to be able to switch branches and compile without having to recompile 
everything
by having multiple O= set up.

I think what my runchecks wrapper script brings in addition is the ability to 
to a number
of checks which may or may not pass, even return error codes, from the same 
'make' command
and configure what errors to fix now and what to postpone/ignore (and thus not 
fail from).

As an example, I just tried clang (on v4.15-rc6) with:

cd $HOME/src/kernel
make O=$HOME/build/kernel/clang
cd $HOME/build/kernel/clang
make

and it fails to compile for me in arch/x86/xen/mmu_pv.o. 

If I'd want to just make sure that some patches did not introduce new errors 
with clang, 
I would waste some time with unrelated errors, and there will be noise in the 
output, also
consuming personal "cycles".

I haven't really looked at the details of much of what clang outputs of errors 
yet, but I
can imagine that specific errors reported by clang might be useful to correct 
even in old
kernels, where some files inevitably will fail to compile like this.

This would be easy to handle with runchecks using a few exceptions for those
problems/files not yet fixed, allowing a run to easily detect (while compiling 
with gcc as
the main compiler) that no new clang errors were introduced of any other kind 
than those
suppressed.

Thanks,
Knut

> 
> thanks,
> 
> greg k-h
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: clang warning: implicit conversion in intel_ddi.c:1481

2018-02-02 Thread Lukas Bulwahn
On Fri, 2 Feb 2018, Jani Nikula wrote:

> Being brutally honest, please write shorter reports and shorter emails
> to the lists.
> 
> The static analysis reports are welcome, but only when 1) we didn't
> already fix it in linux-next, or 2) it reveals an actual bug, not just a
> warning, warranting a backport.

That will be our policy.

Lukas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: clang warning: implicit conversion in intel_ddi.c:1481

2018-02-02 Thread Ruben Safir
> 
> What is the goal of these types of emails?
> 

even more so on this mailing list.  It almost feels like guerilla
advertising for Clang.


> thanks,
> 
> greg k-h
> 
> ___
> Kernelnewbies mailing list
> kernelnewb...@kernelnewbies.org
> https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

-- 
So many immigrant groups have swept through our town
that Brooklyn, like Atlantis, reaches mythological
proportions in the mind of the world - RI Safir 1998
http://www.mrbrklyn.com 

DRM is THEFT - We are the STAKEHOLDERS - RI Safir 2002
http://www.nylxs.com - Leadership Development in Free Software
http://www2.mrbrklyn.com/resources - Unpublished Archive 
http://www.coinhangout.com - coins!
http://www.brooklyn-living.com 

Being so tracked is for FARM ANIMALS and and extermination camps, 
but incompatible with living as a free human being. -RI Safir 2013

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: clang warning: implicit conversion in intel_ddi.c:1481

2018-02-02 Thread Knut Omang
On Fri, 2018-02-02 at 12:44 +0200, Jani Nikula wrote:
> +Knut, Fengguang
> 
> On Fri, 02 Feb 2018, Greg KH  wrote:
> > - If clang now builds the kernel "cleanly", yes, I want to take
> >   warning fixes in the stable tree.  And even better yet, if you
> >   keep working to ensure the tree is "clean", that would be
> >   wonderful.
> 
> So we can run sparse using 'make C=1' and friends, or other static
> analysis tools using 'make CHECK=foo C=1', as long as the passed command
> line params work. There was work by Knut to extend this make checker
> stuff [1]. Since mixing different HOSTCC's in a single workdir seems
> like a bad idea, I wonder how hard it would be to make clang work like
> this:
> 
> $ make CHECK=clang C=1
> 
> Or using Knut's wrapper. Feels like that could increase the use of clang
> for static analysis of patches.

Yes, definitely a natural addition to the set of tools supported by
runchecks to also support using alternate compiler(s) as "checkers" - I guess
the same would apply for people compiling with clang - that they don't 
accidentally
generate warnings with gcc..

Thanks,
Knut

> BR,
> Jani.
> 
> 
> [1] 
> http://mid.mail-archive.com/cover.5b56d020b8e826a7da33b1823c059acd0c123f8b.151507278
> 2.git-series.knut.om...@oracle.com
> 
> 
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: clang warning: implicit conversion in intel_ddi.c:1481

2018-02-02 Thread Greg KH
On Fri, Feb 02, 2018 at 04:37:55PM +0200, Jani Nikula wrote:
> On Fri, 02 Feb 2018, Greg KH  wrote:
> > On Fri, Feb 02, 2018 at 12:44:38PM +0200, Jani Nikula wrote:
> >> 
> >> +Knut, Fengguang
> >> 
> >> On Fri, 02 Feb 2018, Greg KH  wrote:
> >> >  - If clang now builds the kernel "cleanly", yes, I want to take
> >> >warning fixes in the stable tree.  And even better yet, if you
> >> >keep working to ensure the tree is "clean", that would be
> >> >wonderful.
> >> 
> >> So we can run sparse using 'make C=1' and friends, or other static
> >> analysis tools using 'make CHECK=foo C=1', as long as the passed command
> >> line params work. There was work by Knut to extend this make checker
> >> stuff [1]. Since mixing different HOSTCC's in a single workdir seems
> >> like a bad idea, I wonder how hard it would be to make clang work like
> >> this:
> >> 
> >> $ make CHECK=clang C=1
> >> 
> >> Or using Knut's wrapper. Feels like that could increase the use of clang
> >> for static analysis of patches.
> >
> > Why not just build with clang itself:
> > make CC=clang
> 
> Same as HOSTCC, mixing different CC's in a single build dir seems like a
> bad idea. Sure, everyone can setup a separate build dir for clang, but
> IMHO having 'make CHECK=clang C=1' work has least resistance. YMMV.

"O=some_output_dir" is your friend.  If you aren't doing that already
for your test builds, you don't know what you are missing :)

thanks,

greg k-h
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: clang warning: implicit conversion in intel_ddi.c:1481

2018-02-02 Thread Jani Nikula
On Fri, 02 Feb 2018, Greg KH  wrote:
> On Fri, Feb 02, 2018 at 12:44:38PM +0200, Jani Nikula wrote:
>> 
>> +Knut, Fengguang
>> 
>> On Fri, 02 Feb 2018, Greg KH  wrote:
>> >- If clang now builds the kernel "cleanly", yes, I want to take
>> >  warning fixes in the stable tree.  And even better yet, if you
>> >  keep working to ensure the tree is "clean", that would be
>> >  wonderful.
>> 
>> So we can run sparse using 'make C=1' and friends, or other static
>> analysis tools using 'make CHECK=foo C=1', as long as the passed command
>> line params work. There was work by Knut to extend this make checker
>> stuff [1]. Since mixing different HOSTCC's in a single workdir seems
>> like a bad idea, I wonder how hard it would be to make clang work like
>> this:
>> 
>> $ make CHECK=clang C=1
>> 
>> Or using Knut's wrapper. Feels like that could increase the use of clang
>> for static analysis of patches.
>
> Why not just build with clang itself:
>   make CC=clang

Same as HOSTCC, mixing different CC's in a single build dir seems like a
bad idea. Sure, everyone can setup a separate build dir for clang, but
IMHO having 'make CHECK=clang C=1' work has least resistance. YMMV.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: clang warning: implicit conversion in intel_ddi.c:1481

2018-02-02 Thread Greg KH
On Fri, Feb 02, 2018 at 12:44:38PM +0200, Jani Nikula wrote:
> 
> +Knut, Fengguang
> 
> On Fri, 02 Feb 2018, Greg KH  wrote:
> > - If clang now builds the kernel "cleanly", yes, I want to take
> >   warning fixes in the stable tree.  And even better yet, if you
> >   keep working to ensure the tree is "clean", that would be
> >   wonderful.
> 
> So we can run sparse using 'make C=1' and friends, or other static
> analysis tools using 'make CHECK=foo C=1', as long as the passed command
> line params work. There was work by Knut to extend this make checker
> stuff [1]. Since mixing different HOSTCC's in a single workdir seems
> like a bad idea, I wonder how hard it would be to make clang work like
> this:
> 
> $ make CHECK=clang C=1
> 
> Or using Knut's wrapper. Feels like that could increase the use of clang
> for static analysis of patches.

Why not just build with clang itself:
make CC=clang

thanks,

greg k-h
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: clang warning: implicit conversion in intel_ddi.c:1481

2018-02-02 Thread Jani Nikula

+Knut, Fengguang

On Fri, 02 Feb 2018, Greg KH  wrote:
>   - If clang now builds the kernel "cleanly", yes, I want to take
> warning fixes in the stable tree.  And even better yet, if you
> keep working to ensure the tree is "clean", that would be
> wonderful.

So we can run sparse using 'make C=1' and friends, or other static
analysis tools using 'make CHECK=foo C=1', as long as the passed command
line params work. There was work by Knut to extend this make checker
stuff [1]. Since mixing different HOSTCC's in a single workdir seems
like a bad idea, I wonder how hard it would be to make clang work like
this:

$ make CHECK=clang C=1

Or using Knut's wrapper. Feels like that could increase the use of clang
for static analysis of patches.


BR,
Jani.


[1] 
http://mid.mail-archive.com/cover.5b56d020b8e826a7da33b1823c059acd0c123f8b.1515072782.git-series.knut.omang@oracle.com



-- 
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: clang warning: implicit conversion in intel_ddi.c:1481

2018-02-02 Thread Greg KH
On Fri, Feb 02, 2018 at 10:56:36AM +0100, Lukas Bulwahn wrote:
> On Fri, 2 Feb 2018, Jani Nikula wrote:
> 
> > Being brutally honest, please write shorter reports and shorter emails
> > to the lists.
> > 
> > The static analysis reports are welcome, but only when 1) we didn't
> > already fix it in linux-next, or 2) it reveals an actual bug, not just a
> > warning, warranting a backport.
> 
> That will be our policy.

Great!

Also a few other things to be aware of when working with the Linux
kernel community, and to try to answer some of your longer original
email querstions:

- don't scatter emails to tons of lists at the same time.  If
  you use scripts/get_maintainer.pl on a file, it will tell you
  exactly who and what list to notify of an issue found.

- when finding an issue, again, always check linux-next as that
  contains up to the past 3 months of work.  Don't duplicate
  stuff that others have already done, as that doesn't help
  anyone out.

- for stable kernel patches, please read:

https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
  as to how to report a patch to be included in a stable kernel
  release (hint, just send the git hash to the list when it is
  in Linus's tree and ask for it to be included, as well as what
  trees you think it should be included in.)

- If clang now builds the kernel "cleanly", yes, I want to take
  warning fixes in the stable tree.  And even better yet, if you
  keep working to ensure the tree is "clean", that would be
  wonderful.

Hope this helps!

greg k-h
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: clang warning: implicit conversion in intel_ddi.c:1481

2018-02-02 Thread Jani Nikula
On Thu, 01 Feb 2018, Lukas Bulwahn  wrote:
> Hi Greg,
>
> On Thu, 1 Feb 2018, Greg KH wrote:
>
>> On Thu, Feb 01, 2018 at 06:33:30PM +0100, Ozan Alpay wrote:
>> > Dear Rodrigo Vivi, Ville Syrjälä,
>> > 
>> > My name is Ozan Alpay, and I am a student mentored by Lukas Bulwahn. We 
>> > intend to use static analysis tools on the kernel source to identify, 
>> > analyze and report issues. As a very first step, we are looking into 
>> > clang compiler warnings and will then move to more sophisticated tools. 
>> >
>> > [...]
>> >
>> > Linux 4.15 is shipped with this clang warning, but we don't see the 
>> > crucial need to provide a backport commit to the stable branch for 4.15. 
>> > We just wanted to inform you about our analysis of this clang warning. 
>> > Ultimately the final call if you would like to address this clang warning 
>> > in 4.15 is yours.
>> 
>> Note, I have not taken "clang warning fixes" for stable kernel updates
>> in the past, and I doubt I will in the future, unless the tree "builds
>> clean" with clang.  If it eventually gets there, then yes, I will do
>> that.
>> 
>> Note, if you are going to email this out to everyone who fixes a warning
>> message, you might want to reconsider it.  That's going to be a lot of
>> work, and for people who have already fixed an issue, it's kind of
>> pointless to just remind them of work they have done in the past, right?
>> 
>> What is the goal of these types of emails?
>> 
>
> We are interested in providing useful information on potential bugs or bug 
> patterns that we get from static analysis tools, after we pre-assess them 
> and manually select them to send to the review process of the patch 
> submission.
>
> For me, the clang warnings were an easy starting point for a student to 
> set up and look at, compared to much more sophisticated tools, such as 
> coccinelle, sparse or new tools for the kernel development, such as CMBC 
> or facebook's Infer.
>
> Once we really understand which tools are useful and which information can 
> be quickly pre-assessed and sent out in a useful way without just creating 
> more noise in the discussion, I would have contacted the 0-day 
> infrastructure team or the kernelci.org team to continue the discussion 
> how to include our first setup into a proper semi-automated service.
>
> Using the clang warnings, I wanted to explore how this would even 
> potentially work.
>
> Considering clang, of course, currently, we cannot compile the whole 
> kernel with all possible kernel configurations with clang, but I believe 
> Nick Desaulniers, Matthias Kaehlcke and others are already working on 
> that and are getting close to this goal. Hence, assuming they will be 
> successful soon, I wanted to explore the next step of using static 
> analysis tools around the clang/LLVM compiler.
>
> Actually, v4.15 builds almost "cleanly" with clang: For defconfig, there 
> are only two clang compiler warnings and the one that we looked into 
> deeper here is already resolved in linux-next, so chances are actually 
> high that we might get to this "builds clean" soon-ish, at least for 
> defconfig.
>
> Concerning clang warnings and how to progress towards that goal of 
> building cleanly, my strategy is to identify when new clang compiler
> warnings are introduced and just point these warnings out as code smells 
> during the review discussion before they are merged into the 
> first maintainer tree. If we manually inspect these clang warnings
> to make sure that they are genuine code smells of some "imprecise 
> implementation" before we send them to the mailing list, I would hope that 
> they are of some value for the developer in the submission process and 
> he/she could address the warning in a patch v2 while he/she is reacting to 
> the other review comments from the human reviewers.
>
> At best, I always considered them as useful information during the review 
> process, but I certainly DO NOT want to start patching the kernel due to 
> clang warnings. The chances/risk that we just break more due to naively 
> fixing warnings without proper understanding is much higher than the  
> benefit of actually improving the situation. If I recall correctly, I 
> think this is also one of the lessons learned from motivating newcomers 
> to address warnings in previous kernel newbies activities.
>
> Greg, do you think it is worthwhile to invest some effort to move 
> towards the goal "kernel builds cleanly with clang"?
> Would you agree that providing information during the patch review is a 
> good way to move forward to this goal if we find a suitable manner to 
> provide this feedback quickly and cleanly at this very early stage of 
> development?
>
> If not, we will immediately stop to move in this direction and this is the 
> first and last email that you have seen of this kind, and we will have to 
> come up with better/other ideas around work on the Linux kernel.
>
> If so, we will continue in the 

Re: clang warning: implicit conversion in intel_ddi.c:1481

2018-02-02 Thread Ozgur


01.02.2018, 21:03, "Greg KH" :
> On Thu, Feb 01, 2018 at 06:33:30PM +0100, Ozan Alpay wrote:
>>  Dear Rodrigo Vivi, Ville Syrjälä,
>>
>>  My name is Ozan Alpay, and I am a student mentored by Lukas Bulwahn. We

Hi Ozan,

why did you send e-mail to kernel development e-mail list?

>>  intend to use static analysis tools on the kernel source to identify,
>>  analyze and report issues. As a very first step, we are looking into
>>  clang compiler warnings and will then move to more sophisticated tools.
>>
>>  When compiling Linux 4.15 with clang, we have discovered that your commit
>>  2952cd6fb4cc ("drm/i915: Let's use more enum intel_dpll_id pll_id.")
>>  introduced the following warning:
>>
>>  drivers/gpu/drm/i915/intel_ddi.c:1481:30: warning: implicit conversion from 
>> enumeration type 'enum port' to different enumeration type 'enum 
>> intel_dpll_id' [-Wenum-conversion]
>>  enum intel_dpll_id pll_id = port;
>>
>>  To reproduce it, you can compile Linux 4.15 with clang with this command:
>>
>>  make HOSTCC=clang-5.0 defconfig && make -j32 HOSTCC=clang-5.0 CC=clang-5.0
>>
>>  If you don't have clang installed in your system, you can use this simple
>>  docker setup to compile the kernel with clang:
>>
>>  wget 
>> https://raw.githubusercontent.com/bulwahn/linux-kernel-analysis/master/docker/kernel-clang/Dockerfile
>>  && \
>>  docker build -t kernel-clang . && \
>>  docker run -v :/linux/ kernel-clang /bin/sh 
>> -c "cd linux && make CC=clang-5.0 clean && make HOSTCC=clang-5.0 defconfig 
>> && make -j32 HOSTCC=clang-5.0 CC=clang-5.0"
>>
>>  While we were doing our analysis on 4.15, we noticed that you already
>>  resolved this warning on linux-next with your work in commit bb911536f07e
>>  ("drm/i915: Eliminate pll->state usage from bxt_calc_pll_link()"). So,
>>  since it is resolved on linux-next and we expect that this commit will be
>>  merged in the merge window for 4.16, there is probably nothing further to
>>  do.
>>
>>  Linux 4.15 is shipped with this clang warning, but we don't see the
>>  crucial need to provide a backport commit to the stable branch for 4.15.
>>  We just wanted to inform you about our analysis of this clang warning.
>>  Ultimately the final call if you would like to address this clang warning
>>  in 4.15 is yours.
>
> Note, I have not taken "clang warning fixes" for stable kernel updates
> in the past, and I doubt I will in the future, unless the tree "builds
> clean" with clang. If it eventually gets there, then yes, I will do
> that.
>
> Note, if you are going to email this out to everyone who fixes a warning
> message, you might want to reconsider it. That's going to be a lot of
> work, and for people who have already fixed an issue, it's kind of
> pointless to just remind them of work they have done in the past, right?
>
> What is the goal of these types of emails?
>
> thanks,
>
> greg k-h

Ozgur
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: clang warning: implicit conversion in intel_ddi.c:1481

2018-02-02 Thread Lukas Bulwahn

Hi Greg,

On Thu, 1 Feb 2018, Greg KH wrote:

> On Thu, Feb 01, 2018 at 06:33:30PM +0100, Ozan Alpay wrote:
> > Dear Rodrigo Vivi, Ville Syrjälä,
> > 
> > My name is Ozan Alpay, and I am a student mentored by Lukas Bulwahn. We 
> > intend to use static analysis tools on the kernel source to identify, 
> > analyze and report issues. As a very first step, we are looking into 
> > clang compiler warnings and will then move to more sophisticated tools. 
> >
> > [...]
> >
> > Linux 4.15 is shipped with this clang warning, but we don't see the 
> > crucial need to provide a backport commit to the stable branch for 4.15. 
> > We just wanted to inform you about our analysis of this clang warning. 
> > Ultimately the final call if you would like to address this clang warning 
> > in 4.15 is yours.
> 
> Note, I have not taken "clang warning fixes" for stable kernel updates
> in the past, and I doubt I will in the future, unless the tree "builds
> clean" with clang.  If it eventually gets there, then yes, I will do
> that.
> 
> Note, if you are going to email this out to everyone who fixes a warning
> message, you might want to reconsider it.  That's going to be a lot of
> work, and for people who have already fixed an issue, it's kind of
> pointless to just remind them of work they have done in the past, right?
> 
> What is the goal of these types of emails?
> 

We are interested in providing useful information on potential bugs or bug 
patterns that we get from static analysis tools, after we pre-assess them 
and manually select them to send to the review process of the patch 
submission.

For me, the clang warnings were an easy starting point for a student to 
set up and look at, compared to much more sophisticated tools, such as 
coccinelle, sparse or new tools for the kernel development, such as CMBC 
or facebook's Infer.

Once we really understand which tools are useful and which information can 
be quickly pre-assessed and sent out in a useful way without just creating 
more noise in the discussion, I would have contacted the 0-day 
infrastructure team or the kernelci.org team to continue the discussion 
how to include our first setup into a proper semi-automated service.

Using the clang warnings, I wanted to explore how this would even 
potentially work.

Considering clang, of course, currently, we cannot compile the whole 
kernel with all possible kernel configurations with clang, but I believe 
Nick Desaulniers, Matthias Kaehlcke and others are already working on 
that and are getting close to this goal. Hence, assuming they will be 
successful soon, I wanted to explore the next step of using static 
analysis tools around the clang/LLVM compiler.

Actually, v4.15 builds almost "cleanly" with clang: For defconfig, there 
are only two clang compiler warnings and the one that we looked into 
deeper here is already resolved in linux-next, so chances are actually 
high that we might get to this "builds clean" soon-ish, at least for 
defconfig.

Concerning clang warnings and how to progress towards that goal of 
building cleanly, my strategy is to identify when new clang compiler
warnings are introduced and just point these warnings out as code smells 
during the review discussion before they are merged into the 
first maintainer tree. If we manually inspect these clang warnings
to make sure that they are genuine code smells of some "imprecise 
implementation" before we send them to the mailing list, I would hope that 
they are of some value for the developer in the submission process and 
he/she could address the warning in a patch v2 while he/she is reacting to 
the other review comments from the human reviewers.

At best, I always considered them as useful information during the review 
process, but I certainly DO NOT want to start patching the kernel due to 
clang warnings. The chances/risk that we just break more due to naively 
fixing warnings without proper understanding is much higher than the  
benefit of actually improving the situation. If I recall correctly, I 
think this is also one of the lessons learned from motivating newcomers 
to address warnings in previous kernel newbies activities.

Greg, do you think it is worthwhile to invest some effort to move 
towards the goal "kernel builds cleanly with clang"?
Would you agree that providing information during the patch review is a 
good way to move forward to this goal if we find a suitable manner to 
provide this feedback quickly and cleanly at this very early stage of 
development?

If not, we will immediately stop to move in this direction and this is the 
first and last email that you have seen of this kind, and we will have to 
come up with better/other ideas around work on the Linux kernel.

If so, we will continue in the direction sketched above, and I think I 
just have to point out and apologize for the two obvious things that we 
did wrong in this specific case here:

  - We noticed that there were further changes in linux-next, but we

clang warning: implicit conversion in intel_ddi.c:1481

2018-02-01 Thread Ozan Alpay
Dear Rodrigo Vivi, Ville Syrjälä,

My name is Ozan Alpay, and I am a student mentored by Lukas Bulwahn. We 
intend to use static analysis tools on the kernel source to identify, 
analyze and report issues. As a very first step, we are looking into 
clang compiler warnings and will then move to more sophisticated tools. 

When compiling Linux 4.15 with clang, we have discovered that your commit 
2952cd6fb4cc ("drm/i915: Let's use more enum intel_dpll_id pll_id.") 
introduced the following warning:

drivers/gpu/drm/i915/intel_ddi.c:1481:30: warning: implicit conversion from 
enumeration type 'enum port' to different enumeration type 'enum intel_dpll_id' 
[-Wenum-conversion] 
enum intel_dpll_id pll_id = port;

To reproduce it, you can compile Linux 4.15 with clang with this command: 

make HOSTCC=clang-5.0 defconfig && make -j32 HOSTCC=clang-5.0 CC=clang-5.0

If you don't have clang installed in your system, you can use this simple 
docker setup to compile the kernel with clang:

wget 
https://raw.githubusercontent.com/bulwahn/linux-kernel-analysis/master/docker/kernel-clang/Dockerfile
 && \
docker build -t kernel-clang . && \
docker run -v :/linux/ kernel-clang /bin/sh -c 
"cd linux && make CC=clang-5.0 clean && make HOSTCC=clang-5.0 defconfig && make 
-j32 HOSTCC=clang-5.0 CC=clang-5.0"
 
While we were doing our analysis on 4.15, we noticed that you already 
resolved this warning on linux-next with your work in commit bb911536f07e 
("drm/i915: Eliminate pll->state usage from bxt_calc_pll_link()"). So, 
since it is resolved on linux-next and we expect that this commit will be 
merged in the merge window for 4.16, there is probably nothing further to 
do.

Linux 4.15 is shipped with this clang warning, but we don't see the 
crucial need to provide a backport commit to the stable branch for 4.15. 
We just wanted to inform you about our analysis of this clang warning. 
Ultimately the final call if you would like to address this clang warning 
in 4.15 is yours.

Best regards,

Ozan & Lukas___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: clang warning: implicit conversion in intel_ddi.c:1481

2018-02-01 Thread Greg KH
On Thu, Feb 01, 2018 at 06:33:30PM +0100, Ozan Alpay wrote:
> Dear Rodrigo Vivi, Ville Syrjälä,
> 
> My name is Ozan Alpay, and I am a student mentored by Lukas Bulwahn. We 
> intend to use static analysis tools on the kernel source to identify, 
> analyze and report issues. As a very first step, we are looking into 
> clang compiler warnings and will then move to more sophisticated tools. 
> 
> When compiling Linux 4.15 with clang, we have discovered that your commit 
> 2952cd6fb4cc ("drm/i915: Let's use more enum intel_dpll_id pll_id.") 
> introduced the following warning:
> 
> drivers/gpu/drm/i915/intel_ddi.c:1481:30: warning: implicit conversion from 
> enumeration type 'enum port' to different enumeration type 'enum 
> intel_dpll_id' [-Wenum-conversion] 
>   enum intel_dpll_id pll_id = port;
> 
> To reproduce it, you can compile Linux 4.15 with clang with this command: 
> 
> make HOSTCC=clang-5.0 defconfig && make -j32 HOSTCC=clang-5.0 CC=clang-5.0
> 
> If you don't have clang installed in your system, you can use this simple 
> docker setup to compile the kernel with clang:
> 
> wget 
> https://raw.githubusercontent.com/bulwahn/linux-kernel-analysis/master/docker/kernel-clang/Dockerfile
>  && \
> docker build -t kernel-clang . && \
> docker run -v :/linux/ kernel-clang /bin/sh -c 
> "cd linux && make CC=clang-5.0 clean && make HOSTCC=clang-5.0 defconfig && 
> make -j32 HOSTCC=clang-5.0 CC=clang-5.0"
>  
> While we were doing our analysis on 4.15, we noticed that you already 
> resolved this warning on linux-next with your work in commit bb911536f07e 
> ("drm/i915: Eliminate pll->state usage from bxt_calc_pll_link()"). So, 
> since it is resolved on linux-next and we expect that this commit will be 
> merged in the merge window for 4.16, there is probably nothing further to 
> do.
> 
> Linux 4.15 is shipped with this clang warning, but we don't see the 
> crucial need to provide a backport commit to the stable branch for 4.15. 
> We just wanted to inform you about our analysis of this clang warning. 
> Ultimately the final call if you would like to address this clang warning 
> in 4.15 is yours.

Note, I have not taken "clang warning fixes" for stable kernel updates
in the past, and I doubt I will in the future, unless the tree "builds
clean" with clang.  If it eventually gets there, then yes, I will do
that.

Note, if you are going to email this out to everyone who fixes a warning
message, you might want to reconsider it.  That's going to be a lot of
work, and for people who have already fixed an issue, it's kind of
pointless to just remind them of work they have done in the past, right?

What is the goal of these types of emails?

thanks,

greg k-h
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel