1264476 - Coverity: mobile/android/base/java code no longer appears & open issues were mass-fixed

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Testing, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Context via IRC:

05:04 <abpostelnicu> in this snapshot https://scan4.coverity.com/reports.htm#v24646/p11134/fileInstanceId=9310760&defectInstanceId=2064431&mergedDefectId=123699, many issues reported previously under /mobile/base/java.org/* have been marked as fixed
05:04 <abpostelnicu> i think something changed in the way how we build fennec
...
12:02 <abpostelnicu> Hello Michael definetely it's a problem with what coverity analysis. In fact it doesn't include in the build nothing from mobile/android/Java
15:14 <mcomella> So the artifact builds are not broken, as best I can tell, and afaik, we didn't change how it's built
15:14 <mcomella> I did update some dependencies to the builders
15:14 <mcomella> For front-end builds, but we haven't seen any issues from that, as best i can tell
15:15 <mcomella> I vaguely remember something about the directory where we store the artifacts got changed
15:15 <mcomella> Perhaps we're trying to upload the wrong APK or an APK that doesn't exist?
...
00:40 <abpostelnicu> the thing is we didn't change anything to our build system, or at least for my knowledge we didn't
00:41 <abpostelnicu> this is the link for the jenkins project
00:41 <abpostelnicu> http://relman-ci.mozilla.org/view/coverity%20jobs/job/fennec-coverity/
---

I think the first step here is to find out what commands Jenkins runs to create its jobs.

Andi, do you know? I can't seem to find it in the interface.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Looking at one of the build outputs, it looks like we proguarded:

Optimizing...
  Number of finalized classes:                 2434
...
  Number of removed write-only fields:         624

Maybe Coverity can't do its thing because the code is obfuscated. It does static analysis, but maybe it does it on the bytecode. I think it'll be hard to debug without knowing what commands we run.

Comment 2

[Andi [:andi]]

This is the script that Jenkins uses to build for coverity-fennec:

>>set -e 
>>rm -rf obj-arm-linux-androideabi cov-build.tar.gz cov-int/  obj-arm-linux-androideabi
>>export PATH=$PATH:/opt/cov-analysis/bin/
>>echo "Cleanup previous report"
>>rm -rf reports/
>>rm -rf obj-arm-linux-androideabi cov-build.tar.gz cov-int/
>>
>>echo "Cleanup"
>>./mach clobber
>>
>>cat > .mozconfig <<EOF
>># Build Firefox for Android:
>>ac_add_options --enable-application=mobile/android
>>ac_add_options --target=arm-linux-androideabi
>>ac_add_options --enable-debug
>>
>># With the following Android SDK and NDK:
>>ac_add_options --with-android-sdk="/var/lib/jenkins/.mozbuild/android-sdk-linux"
>>ac_add_options --with-android-ndk="/var/lib/jenkins/.mozbuild/android-ndk-r10e"
>>
>>#ac_add_options --enable-artifact-builds
>>
>>mk_add_options MOZ_MAKE_FLAGS="-j24"
>>EOF
>>
>>echo "Apply patches on top of the hg repo"
>>hg qpop -a
>>hg qpush -a
>>
>>echo "Rebuild all"
>>cov-configure --java --force-debug
>>cov-build --dir cov-int  ./mach --log-no-times build -v
>>
>>tar zcvf cov-build.tar.gz cov-int/
>>
>>#minimumsize=90000
>>minimumsize=80000
>>
>>actualsize=$(du -k cov-build.tar.gz | cut -f 1)
>>if [ $actualsize -ge $minimumsize ]; then
>>        sh /opt/upload-to-coverity-fennec.sh
>>else
>>	echo "Unexpected filesize. Should be at least $minimumsize (found $actualize)"
>>	exit 1
>>fi
>>
>>./mach clobber
>>
>>rm -rf obj-arm-linux-androideabi cov-build.tar.gz cov-int/  obj-arm-linux-androideabi
>>exit 0

I agree with you that having the code obfuscated can lead to this issue. If you look at the way how Coverity dismissed bugs you can see that it excluded files from mobile/android/base/java/org/mozilla. It is like when it's builds it's analysis db coverity doesn't include these files or code from these files.
if you look at coverity snapsho-  21/03/2016 you can see that the number of bugs marked as resoved is  and almost all of them from mobile/android/base/java/org/mozilla

As an example this is the latest build from our history that seemed to work properly with Coverity, we've had newer builds but for no reason they are gone from the history, i think they've been deleted: http://relman-ci.mozilla.org/job/fennec-coverity/53/consoleFull

If you look at below these lines from the build log:

>>+ tar zcvf cov-build.tar.gz cov-int/
>>cov-int/

you can see that there are many more files in cov-int/ than in the latest build: http://relman-ci.mozilla.org/job/fennec-coverity/135/consoleFull

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I see you don't have an explicit objdir (i.e. `mk_add_options MOZ_OBJDIR=...` – perhaps the default objdir got changed?

In any case, it'd be more robust to set an explicit objdir.

Comment 4

[Andi [:andi]]

I did the changes to mozconfig and now it looks like:

>># Build Firefox for Android:
>>ac_add_options --enable-application=mobile/android
>>ac_add_options --target=arm-linux-androideabi
>>ac_add_options --enable-debug

>># With the following Android SDK and NDK:
>>ac_add_options --with-android-sdk="/var/lib/jenkins/.mozbuild/android-sdk-linux"
>>#ac_add_options --with-android-ndk="/var/lib/jenkins/.mozbuild/android-ndk-r10e"

>># Front end path 
>>mk_add_options MOZ_OBJDIR=./objdir-frontend

>>ac_add_options --enable-artifact-builds

>>mk_add_options MOZ_MAKE_FLAGS="-j24"

It doesn't matter if we use build artifacts or not the outcome is the same, what troubles me is the content of cov-int that dropped from builds greater than 53.

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

07:25 <abpostelnicu> hello Michael i've tried to disable  proguard on the latest fennect coverity build -  http://relman-ci.mozilla.org/view/coverity%20jobs/job/fennec-coverity/148/
07:26 <abpostelnicu> i've added in mozconfig  export MOZ_DISABLE_PROGUARD=1 since it seems that MOZ_DISABLE_PROGUARD is used in https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/Makefile.in#225
07:26 <abpostelnicu> but i didn't have much luck since still proguard did one pass of optimization

---

Hey Andi, I've flagged this for triage but I think it'd be good to take a step back. afaik, the Android developers don't actively look into this dashboard so it forms the question: what value do we get from Coverity?

I've seen from other static analysis tools that without accountability (e.g. when someone causes a regression, they fix it), the issues these tools point out continue to regress and don't serve much value. Since no devs look at the coverity dashboard currently, there's no accountability.

I've noticed you've been fixing issues (which we really appreciate!) so what value does Coverity provide to your team? fwiw, it'd probably be more efficient for the devs to fix their regressions (e.g. from a context switching perspective) so we should try to drive towards a solution with developer accountability.

Ideally, imo, new Coverity regressions either burn the tree on treeherder or are filed as bugs with someone assigned, but we can look into faster solutions if that's not feasible.

Comment 6

[Andi [:andi]]

Hello Michael,

For the moment we are testing the checkers provided by Coverity, using a free version of it, in order to analyze the consistency between the bugs that it reports and the ones that are false positive. If we conclude that this tool is worth considering i think we should continue monitoring it's output as it would provide help by reporting issues that are not yet visible to the end-user.
For example we started using Coverity mainly on our Gecko code, and many of the checkers where about uninitialized scalars, or pointer fields in classes. Of course in many situations this is not a big deal, but in some cases this can lead to unexpected behavior, worst of them being crashes. So for this we've started working on a clang-plugin, see bug 525063, that will be integrated in the building pipeline and will detect these kinds on un-initialization variables and asserts them as errors during compile time thus forcing the engineer to take action.
This being said I can take ownership of this bug and find a solution for this problem.

Comment 7

[:Margaret Leibovic]

There's nothing for us to track for a specific Fennec release here, but we do support fixing this.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Thanks for the feedback, Andi. If we could build it into our build system like it sounds like you're doing for Gecko, that'd be great! My biggest concern is that things will regress but if we can make it part of the build system, that should solve that issue.

(In reply to Andi-Bogdan Postelnicu from comment #6)
> This being said I can take ownership of this bug and find a solution for
> this problem.

Great, thanks for taking this over! If you have questions, please feel free to NI me and I'll get back to you as quick as I can!

Comment 9

[Andi [:andi]]

This is no longer the case so closing it.