[Static Analysis] infer errors in GeckoView

RESOLVED FIXED in Firefox 63

Status

()

P3
normal
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: rbartlensky, Assigned: rbartlensky)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 63
Points:
---

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox-esr60 wontfix, firefox61 wontfix, firefox62 wontfix, firefox63 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 months ago
// 1
mobile/android/geckoview/src/main/java/org/mozilla/gecko/process/GeckoProcessManager.java:178: error: NULL_DEREFERENCE
  object returned by `GeckoProcessManager.mConnections.get("tab").bind()` could be null and is dereferenced at line 178.
  176.       public void crashChild() {
  177.           try {
  178. >             mConnections.get("tab").bind().crash();
  179.           } catch (RemoteException e) {
  180.           }

// 2
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoInputConnection.java:213: error: NULL_DEREFERENCE
  object returned by `getText(view.getContext())` could be null and is dereferenced by call to `commitText(...)` at line 213.
  211.                   break;
  212.               case android.R.id.paste:
  213. >                 commitText(Clipboard.getText(view.getContext()), 1);
  214.                   break;
  215.               case android.R.id.copy:

// 3
mobile/android/geckoview/src/main/java/org/mozilla/gecko/process/GeckoProcessManager.java:249: error: NULL_DEREFERENCE
  object `ipcPfd` last assigned on line 215 could be null and is dereferenced at line 249.
  247.                   crashPfd.close();
  248.               }
  249. >             ipcPfd.close();
  250.           } catch (final IOException e) {
  251.           }

// 4
mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java:289: error: NULL_DEREFERENCE
  object returned by `getRootException(e)` could be null and is dereferenced by call to `getExceptionStackTrace(...)` at line 289.
  287.       @WrapForJNI(exceptionMode = "ignore")
  288.       private static String getExceptionStackTrace(Throwable e) {
  289. >         return CrashHandler.getExceptionStackTrace(CrashHandler.getRootException(e));
  290.       }
  291.   

// 5
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java:329: error: NULL_DEREFERENCE
  object `matchedList` last assigned on line 327 could be null and is dereferenced by call to `listToCategory(...)` at line 329.
  327.                       final String matchedList = message.getString("matchedList");
  328.                       delegate.onTrackerBlocked(GeckoSession.this, uri,
  329. >                         TrackingProtection.listToCategory(matchedList));
  330.                   }
  331.               }

// 6
mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoThread.java:335: error: NULL_DEREFERENCE
  object `profile` last assigned on line 334 could be null and is dereferenced at line 335.
  333.   
  334.           final GeckoProfile profile = getProfile();
  335. >         if (profile.isCustomProfile()) {
  336.               args.add("-profile");
  337.               args.add(profile.getDir().getAbsolutePath());

// 7
mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoProfile.java:805: error: NULL_DEREFERENCE
  object `sections` last assigned on line 804 could be null and is dereferenced at line 805.
  803.               final INIParser parser = GeckoProfileDirectories.getProfilesINI(mMozillaDir);
  804.               final Hashtable<String, INISection> sections = parser.getSections();
  805. >             for (Enumeration<INISection> e = sections.elements(); e.hasMoreElements();) {
  806.                   final INISection section = e.nextElement();
  807.                   String name = section.getStringProperty("Name");
(Assignee)

Comment 1

5 months ago
I am not sure if 2, 5, and 7 are correct or not, could someone help me with figuring that out?
(Assignee)

Comment 2

5 months ago
I assume 2 is a false positive, but what about 4? Should I ignore that one as well?
Sebastian what do you think about these?
Flags: needinfo?(s.kaspari)
(Assignee)

Updated

5 months ago
Priority: -- → P3
We could add a null check for #5, but we would never reach this callback with matchedList being null [1].

[1] https://searchfox.org/mozilla-central/source/mobile/android/modules/geckoview/GeckoViewTrackingProtection.jsm#36
4 is a false positive, though you can probably add `@NonNull` to the `Throwable e` argument to clear the warning.

2 and 7 can technically happen, but I don't think we have seen them in practice.
Clearing NI: Changes here are up to the GeckoView team.
Flags: needinfo?(s.kaspari)
(Assignee)

Comment 7

4 months ago
Created attachment 9002461 [details]
Bug 1477707: Fix all infer errors in GeckoView.

This fixes some errors reported by infer on GeckoView that are not related to threading.
Comment on attachment 9002461 [details]
Bug 1477707: Fix all infer errors in GeckoView.

Jim Chen [:jchen] [:darchons] has approved the revision.
Attachment #9002461 - Flags: review+

Comment 9

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8e638a82d7da
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
status-firefox62=wontfix because we don't need to uplift these fixes to GV 62 Beta.
status-firefox61: --- → wontfix
status-firefox62: --- → wontfix
status-firefox-esr52: --- → wontfix
status-firefox-esr60: --- → wontfix
You need to log in before you can comment on or make changes to this bug.