You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mxnet.apache.org by Anton Chernov <me...@gmail.com> on 2018/11/02 09:19:58 UTC
Coverity scan
Dear MXNet community,
I had investigated the possibility to adopt Coverity static analysis tools
for the MXNet project and it turned out that there is a tool provided by
Synopsys for open-source projects:
https://scan.coverity.com
The tool works nicely with GitHub [1] and I found that a scan for a fork
(from @apeforest) [2] was already set up. I can not tell how long ago the
scan was performed, but at the time of writing the project page shows 5
illegal memory access errors, that I think would be worth investigating.
If there is interest I would suggest that we would setup a Coverity scan
for the main repository instead of a fork and people that have interest
managing and fixing issues would request add them to the project.
I would appreciate feedback for this proposal and help from people having
rights for the main repository to set things up.
Best regards,
Anton
[1] https://scan.coverity.com/github
[2] https://scan.coverity.com/projects/apeforest-incubator-mxnet
Re: Coverity scan
Posted by Steffen Rochel <st...@gmail.com>.
+1, would be good to have Coverity run regularly. I think by now Python,
Java and other languages are supported.
FYI - If you ever run into problems with scan, let me know. I used to work
at Coverity (now Synopsys) before joining AWS.
Steffen
On Fri, Nov 2, 2018 at 3:43 PM Lin Yuan <ap...@gmail.com> wrote:
> Anton,
>
> Yes, I did a scan using Coverity on MXNet a few months ago. It did show
> some memory issues. I was later buried by other work with higher priority
> and would definitely like to see Coverity (or any other better memory scan)
> tool to be run regularly on MXNet backend.
>
> Let me know if you want to discuss further on this topic. I would like to
> provide as much help as I can.
>
> Best,
>
> Lin
>
> On Fri, Nov 2, 2018 at 3:15 PM kellen sunderland <
> kellen.sunderland@gmail.com> wrote:
>
> > Totally agree Pedro, reporting the data in a more accessible way would
> be a
> > huge improvement. For this reason alone I think it might be worthwhile
> > adopting coverity.
> >
> > On Fri, Nov 2, 2018, 11:38 AM Pedro Larroy <pedro.larroy.lists@gmail.com
> > wrote:
> >
> > > Thanks a lot, I think is very beneficial that we invest in these kind
> of
> > > tooling for code quality. As a developer I wonder, do we have
> actionable
> > > items for looking at / fixing these issues or right now is done in an
> > > informational / good will basis?
> > >
> > > Is there a way to colorize this output?
> > >
> > > Pedro.
> > >
> > > On Fri, Nov 2, 2018 at 5:10 PM kellen sunderland <
> > > kellen.sunderland@gmail.com> wrote:
> > >
> > > > Reference scan here (I believe I also count 5 memory violations):
> > > >
> > > >
> > >
> >
> http://jenkins.mxnet-ci.amazon-ml.com/blue/rest/organizations/jenkins/pipelines/incubator-mxnet/branches/master/runs/1856/nodes/104/log/?start=0
> > > >
> > > > -Kellen
> > > >
> > > > On Fri, Nov 2, 2018 at 9:07 AM kellen sunderland <
> > > > kellen.sunderland@gmail.com> wrote:
> > > >
> > > > > Hey Anton, can you provide a sample scan? I'm interested to see if
> > it
> > > > > catches different memory access violations, or if it gets the same
> > ones
> > > > > we've already seen reported by clang-tidy. For example are these
> > > > > violations in the reports:
> > > > > ------------------------------
> > > > >
> > "/work/mxnet/3rdparty/dmlc-core/include/dmlc/concurrentqueue.h:3443:24:
> > > > > warning: Access to field 'capacity' results in a dereference of a
> > null
> > > > > pointer (loaded from variable 'mainHash')
> > > > > [clang-analyzer-core.NullDereference]"
> > > > >
> > > > > ---------------------------
> > > > >
> > > > > /work/mxnet/3rdparty/mshadow/mshadow/./tensor.h:64:23: warning:
> > > Assigned
> > > > value is garbage or undefined
> > [clang-analyzer-core.uninitialized.Assign]
> > > > > this->shape_[i] = s[i];"
> > > > >
> > > > > -------------------------
> > > > >
> > > > >
> > > > >
> > > >
> > >
> >
> /usr/bin/../lib/gcc/x86_64-linux-gnu/8.0.1/../../../../include/c++/8.0.1/ext/atomicity.h:67:29:
> > > > warning: Use of memory after it is freed
> > > > [clang-analyzer-cplusplus.NewDelete]
> > > > >
> > > > > --------------------------
> > > > >
> > > > > -Kellen
> > > > >
> > > > >
> > > > >
> > > > > On Fri, Nov 2, 2018 at 2:20 AM Anton Chernov <me...@gmail.com>
> > > > wrote:
> > > > >
> > > > >> Dear MXNet community,
> > > > >>
> > > > >> I had investigated the possibility to adopt Coverity static
> analysis
> > > > tools
> > > > >> for the MXNet project and it turned out that there is a tool
> > provided
> > > by
> > > > >> Synopsys for open-source projects:
> > > > >>
> > > > >> https://scan.coverity.com
> > > > >>
> > > > >> The tool works nicely with GitHub [1] and I found that a scan for
> a
> > > fork
> > > > >> (from @apeforest) [2] was already set up. I can not tell how long
> > ago
> > > > the
> > > > >> scan was performed, but at the time of writing the project page
> > shows
> > > 5
> > > > >> illegal memory access errors, that I think would be worth
> > > investigating.
> > > > >>
> > > > >> If there is interest I would suggest that we would setup a
> Coverity
> > > scan
> > > > >> for the main repository instead of a fork and people that have
> > > interest
> > > > >> managing and fixing issues would request add them to the project.
> > > > >>
> > > > >> I would appreciate feedback for this proposal and help from people
> > > > having
> > > > >> rights for the main repository to set things up.
> > > > >>
> > > > >> Best regards,
> > > > >> Anton
> > > > >>
> > > > >> [1] https://scan.coverity.com/github
> > > > >> [2] https://scan.coverity.com/projects/apeforest-incubator-mxnet
> > > > >>
> > > > >
> > > >
> > >
> >
>
Re: Coverity scan
Posted by Lin Yuan <ap...@gmail.com>.
Anton,
Yes, I did a scan using Coverity on MXNet a few months ago. It did show
some memory issues. I was later buried by other work with higher priority
and would definitely like to see Coverity (or any other better memory scan)
tool to be run regularly on MXNet backend.
Let me know if you want to discuss further on this topic. I would like to
provide as much help as I can.
Best,
Lin
On Fri, Nov 2, 2018 at 3:15 PM kellen sunderland <
kellen.sunderland@gmail.com> wrote:
> Totally agree Pedro, reporting the data in a more accessible way would be a
> huge improvement. For this reason alone I think it might be worthwhile
> adopting coverity.
>
> On Fri, Nov 2, 2018, 11:38 AM Pedro Larroy <pedro.larroy.lists@gmail.com
> wrote:
>
> > Thanks a lot, I think is very beneficial that we invest in these kind of
> > tooling for code quality. As a developer I wonder, do we have actionable
> > items for looking at / fixing these issues or right now is done in an
> > informational / good will basis?
> >
> > Is there a way to colorize this output?
> >
> > Pedro.
> >
> > On Fri, Nov 2, 2018 at 5:10 PM kellen sunderland <
> > kellen.sunderland@gmail.com> wrote:
> >
> > > Reference scan here (I believe I also count 5 memory violations):
> > >
> > >
> >
> http://jenkins.mxnet-ci.amazon-ml.com/blue/rest/organizations/jenkins/pipelines/incubator-mxnet/branches/master/runs/1856/nodes/104/log/?start=0
> > >
> > > -Kellen
> > >
> > > On Fri, Nov 2, 2018 at 9:07 AM kellen sunderland <
> > > kellen.sunderland@gmail.com> wrote:
> > >
> > > > Hey Anton, can you provide a sample scan? I'm interested to see if
> it
> > > > catches different memory access violations, or if it gets the same
> ones
> > > > we've already seen reported by clang-tidy. For example are these
> > > > violations in the reports:
> > > > ------------------------------
> > > >
> "/work/mxnet/3rdparty/dmlc-core/include/dmlc/concurrentqueue.h:3443:24:
> > > > warning: Access to field 'capacity' results in a dereference of a
> null
> > > > pointer (loaded from variable 'mainHash')
> > > > [clang-analyzer-core.NullDereference]"
> > > >
> > > > ---------------------------
> > > >
> > > > /work/mxnet/3rdparty/mshadow/mshadow/./tensor.h:64:23: warning:
> > Assigned
> > > value is garbage or undefined
> [clang-analyzer-core.uninitialized.Assign]
> > > > this->shape_[i] = s[i];"
> > > >
> > > > -------------------------
> > > >
> > > >
> > > >
> > >
> >
> /usr/bin/../lib/gcc/x86_64-linux-gnu/8.0.1/../../../../include/c++/8.0.1/ext/atomicity.h:67:29:
> > > warning: Use of memory after it is freed
> > > [clang-analyzer-cplusplus.NewDelete]
> > > >
> > > > --------------------------
> > > >
> > > > -Kellen
> > > >
> > > >
> > > >
> > > > On Fri, Nov 2, 2018 at 2:20 AM Anton Chernov <me...@gmail.com>
> > > wrote:
> > > >
> > > >> Dear MXNet community,
> > > >>
> > > >> I had investigated the possibility to adopt Coverity static analysis
> > > tools
> > > >> for the MXNet project and it turned out that there is a tool
> provided
> > by
> > > >> Synopsys for open-source projects:
> > > >>
> > > >> https://scan.coverity.com
> > > >>
> > > >> The tool works nicely with GitHub [1] and I found that a scan for a
> > fork
> > > >> (from @apeforest) [2] was already set up. I can not tell how long
> ago
> > > the
> > > >> scan was performed, but at the time of writing the project page
> shows
> > 5
> > > >> illegal memory access errors, that I think would be worth
> > investigating.
> > > >>
> > > >> If there is interest I would suggest that we would setup a Coverity
> > scan
> > > >> for the main repository instead of a fork and people that have
> > interest
> > > >> managing and fixing issues would request add them to the project.
> > > >>
> > > >> I would appreciate feedback for this proposal and help from people
> > > having
> > > >> rights for the main repository to set things up.
> > > >>
> > > >> Best regards,
> > > >> Anton
> > > >>
> > > >> [1] https://scan.coverity.com/github
> > > >> [2] https://scan.coverity.com/projects/apeforest-incubator-mxnet
> > > >>
> > > >
> > >
> >
>
Re: Coverity scan
Posted by kellen sunderland <ke...@gmail.com>.
Totally agree Pedro, reporting the data in a more accessible way would be a
huge improvement. For this reason alone I think it might be worthwhile
adopting coverity.
On Fri, Nov 2, 2018, 11:38 AM Pedro Larroy <pedro.larroy.lists@gmail.com
wrote:
> Thanks a lot, I think is very beneficial that we invest in these kind of
> tooling for code quality. As a developer I wonder, do we have actionable
> items for looking at / fixing these issues or right now is done in an
> informational / good will basis?
>
> Is there a way to colorize this output?
>
> Pedro.
>
> On Fri, Nov 2, 2018 at 5:10 PM kellen sunderland <
> kellen.sunderland@gmail.com> wrote:
>
> > Reference scan here (I believe I also count 5 memory violations):
> >
> >
> http://jenkins.mxnet-ci.amazon-ml.com/blue/rest/organizations/jenkins/pipelines/incubator-mxnet/branches/master/runs/1856/nodes/104/log/?start=0
> >
> > -Kellen
> >
> > On Fri, Nov 2, 2018 at 9:07 AM kellen sunderland <
> > kellen.sunderland@gmail.com> wrote:
> >
> > > Hey Anton, can you provide a sample scan? I'm interested to see if it
> > > catches different memory access violations, or if it gets the same ones
> > > we've already seen reported by clang-tidy. For example are these
> > > violations in the reports:
> > > ------------------------------
> > > "/work/mxnet/3rdparty/dmlc-core/include/dmlc/concurrentqueue.h:3443:24:
> > > warning: Access to field 'capacity' results in a dereference of a null
> > > pointer (loaded from variable 'mainHash')
> > > [clang-analyzer-core.NullDereference]"
> > >
> > > ---------------------------
> > >
> > > /work/mxnet/3rdparty/mshadow/mshadow/./tensor.h:64:23: warning:
> Assigned
> > value is garbage or undefined [clang-analyzer-core.uninitialized.Assign]
> > > this->shape_[i] = s[i];"
> > >
> > > -------------------------
> > >
> > >
> > >
> >
> /usr/bin/../lib/gcc/x86_64-linux-gnu/8.0.1/../../../../include/c++/8.0.1/ext/atomicity.h:67:29:
> > warning: Use of memory after it is freed
> > [clang-analyzer-cplusplus.NewDelete]
> > >
> > > --------------------------
> > >
> > > -Kellen
> > >
> > >
> > >
> > > On Fri, Nov 2, 2018 at 2:20 AM Anton Chernov <me...@gmail.com>
> > wrote:
> > >
> > >> Dear MXNet community,
> > >>
> > >> I had investigated the possibility to adopt Coverity static analysis
> > tools
> > >> for the MXNet project and it turned out that there is a tool provided
> by
> > >> Synopsys for open-source projects:
> > >>
> > >> https://scan.coverity.com
> > >>
> > >> The tool works nicely with GitHub [1] and I found that a scan for a
> fork
> > >> (from @apeforest) [2] was already set up. I can not tell how long ago
> > the
> > >> scan was performed, but at the time of writing the project page shows
> 5
> > >> illegal memory access errors, that I think would be worth
> investigating.
> > >>
> > >> If there is interest I would suggest that we would setup a Coverity
> scan
> > >> for the main repository instead of a fork and people that have
> interest
> > >> managing and fixing issues would request add them to the project.
> > >>
> > >> I would appreciate feedback for this proposal and help from people
> > having
> > >> rights for the main repository to set things up.
> > >>
> > >> Best regards,
> > >> Anton
> > >>
> > >> [1] https://scan.coverity.com/github
> > >> [2] https://scan.coverity.com/projects/apeforest-incubator-mxnet
> > >>
> > >
> >
>
Re: Coverity scan
Posted by Pedro Larroy <pe...@gmail.com>.
Thanks a lot, I think is very beneficial that we invest in these kind of
tooling for code quality. As a developer I wonder, do we have actionable
items for looking at / fixing these issues or right now is done in an
informational / good will basis?
Is there a way to colorize this output?
Pedro.
On Fri, Nov 2, 2018 at 5:10 PM kellen sunderland <
kellen.sunderland@gmail.com> wrote:
> Reference scan here (I believe I also count 5 memory violations):
>
> http://jenkins.mxnet-ci.amazon-ml.com/blue/rest/organizations/jenkins/pipelines/incubator-mxnet/branches/master/runs/1856/nodes/104/log/?start=0
>
> -Kellen
>
> On Fri, Nov 2, 2018 at 9:07 AM kellen sunderland <
> kellen.sunderland@gmail.com> wrote:
>
> > Hey Anton, can you provide a sample scan? I'm interested to see if it
> > catches different memory access violations, or if it gets the same ones
> > we've already seen reported by clang-tidy. For example are these
> > violations in the reports:
> > ------------------------------
> > "/work/mxnet/3rdparty/dmlc-core/include/dmlc/concurrentqueue.h:3443:24:
> > warning: Access to field 'capacity' results in a dereference of a null
> > pointer (loaded from variable 'mainHash')
> > [clang-analyzer-core.NullDereference]"
> >
> > ---------------------------
> >
> > /work/mxnet/3rdparty/mshadow/mshadow/./tensor.h:64:23: warning: Assigned
> value is garbage or undefined [clang-analyzer-core.uninitialized.Assign]
> > this->shape_[i] = s[i];"
> >
> > -------------------------
> >
> >
> >
> /usr/bin/../lib/gcc/x86_64-linux-gnu/8.0.1/../../../../include/c++/8.0.1/ext/atomicity.h:67:29:
> warning: Use of memory after it is freed
> [clang-analyzer-cplusplus.NewDelete]
> >
> > --------------------------
> >
> > -Kellen
> >
> >
> >
> > On Fri, Nov 2, 2018 at 2:20 AM Anton Chernov <me...@gmail.com>
> wrote:
> >
> >> Dear MXNet community,
> >>
> >> I had investigated the possibility to adopt Coverity static analysis
> tools
> >> for the MXNet project and it turned out that there is a tool provided by
> >> Synopsys for open-source projects:
> >>
> >> https://scan.coverity.com
> >>
> >> The tool works nicely with GitHub [1] and I found that a scan for a fork
> >> (from @apeforest) [2] was already set up. I can not tell how long ago
> the
> >> scan was performed, but at the time of writing the project page shows 5
> >> illegal memory access errors, that I think would be worth investigating.
> >>
> >> If there is interest I would suggest that we would setup a Coverity scan
> >> for the main repository instead of a fork and people that have interest
> >> managing and fixing issues would request add them to the project.
> >>
> >> I would appreciate feedback for this proposal and help from people
> having
> >> rights for the main repository to set things up.
> >>
> >> Best regards,
> >> Anton
> >>
> >> [1] https://scan.coverity.com/github
> >> [2] https://scan.coverity.com/projects/apeforest-incubator-mxnet
> >>
> >
>
Re: Coverity scan
Posted by kellen sunderland <ke...@gmail.com>.
Reference scan here (I believe I also count 5 memory violations):
http://jenkins.mxnet-ci.amazon-ml.com/blue/rest/organizations/jenkins/pipelines/incubator-mxnet/branches/master/runs/1856/nodes/104/log/?start=0
-Kellen
On Fri, Nov 2, 2018 at 9:07 AM kellen sunderland <
kellen.sunderland@gmail.com> wrote:
> Hey Anton, can you provide a sample scan? I'm interested to see if it
> catches different memory access violations, or if it gets the same ones
> we've already seen reported by clang-tidy. For example are these
> violations in the reports:
> ------------------------------
> "/work/mxnet/3rdparty/dmlc-core/include/dmlc/concurrentqueue.h:3443:24:
> warning: Access to field 'capacity' results in a dereference of a null
> pointer (loaded from variable 'mainHash')
> [clang-analyzer-core.NullDereference]"
>
> ---------------------------
>
> /work/mxnet/3rdparty/mshadow/mshadow/./tensor.h:64:23: warning: Assigned value is garbage or undefined [clang-analyzer-core.uninitialized.Assign]
> this->shape_[i] = s[i];"
>
> -------------------------
>
>
> /usr/bin/../lib/gcc/x86_64-linux-gnu/8.0.1/../../../../include/c++/8.0.1/ext/atomicity.h:67:29: warning: Use of memory after it is freed [clang-analyzer-cplusplus.NewDelete]
>
> --------------------------
>
> -Kellen
>
>
>
> On Fri, Nov 2, 2018 at 2:20 AM Anton Chernov <me...@gmail.com> wrote:
>
>> Dear MXNet community,
>>
>> I had investigated the possibility to adopt Coverity static analysis tools
>> for the MXNet project and it turned out that there is a tool provided by
>> Synopsys for open-source projects:
>>
>> https://scan.coverity.com
>>
>> The tool works nicely with GitHub [1] and I found that a scan for a fork
>> (from @apeforest) [2] was already set up. I can not tell how long ago the
>> scan was performed, but at the time of writing the project page shows 5
>> illegal memory access errors, that I think would be worth investigating.
>>
>> If there is interest I would suggest that we would setup a Coverity scan
>> for the main repository instead of a fork and people that have interest
>> managing and fixing issues would request add them to the project.
>>
>> I would appreciate feedback for this proposal and help from people having
>> rights for the main repository to set things up.
>>
>> Best regards,
>> Anton
>>
>> [1] https://scan.coverity.com/github
>> [2] https://scan.coverity.com/projects/apeforest-incubator-mxnet
>>
>
Re: Coverity scan
Posted by kellen sunderland <ke...@gmail.com>.
Hey Anton, can you provide a sample scan? I'm interested to see if it
catches different memory access violations, or if it gets the same ones
we've already seen reported by clang-tidy. For example are these
violations in the reports:
------------------------------
"/work/mxnet/3rdparty/dmlc-core/include/dmlc/concurrentqueue.h:3443:24:
warning: Access to field 'capacity' results in a dereference of a null
pointer (loaded from variable 'mainHash')
[clang-analyzer-core.NullDereference]"
---------------------------
/work/mxnet/3rdparty/mshadow/mshadow/./tensor.h:64:23: warning:
Assigned value is garbage or undefined
[clang-analyzer-core.uninitialized.Assign]
this->shape_[i] = s[i];"
-------------------------
/usr/bin/../lib/gcc/x86_64-linux-gnu/8.0.1/../../../../include/c++/8.0.1/ext/atomicity.h:67:29:
warning: Use of memory after it is freed
[clang-analyzer-cplusplus.NewDelete]
--------------------------
-Kellen
On Fri, Nov 2, 2018 at 2:20 AM Anton Chernov <me...@gmail.com> wrote:
> Dear MXNet community,
>
> I had investigated the possibility to adopt Coverity static analysis tools
> for the MXNet project and it turned out that there is a tool provided by
> Synopsys for open-source projects:
>
> https://scan.coverity.com
>
> The tool works nicely with GitHub [1] and I found that a scan for a fork
> (from @apeforest) [2] was already set up. I can not tell how long ago the
> scan was performed, but at the time of writing the project page shows 5
> illegal memory access errors, that I think would be worth investigating.
>
> If there is interest I would suggest that we would setup a Coverity scan
> for the main repository instead of a fork and people that have interest
> managing and fixing issues would request add them to the project.
>
> I would appreciate feedback for this proposal and help from people having
> rights for the main repository to set things up.
>
> Best regards,
> Anton
>
> [1] https://scan.coverity.com/github
> [2] https://scan.coverity.com/projects/apeforest-incubator-mxnet
>