1477707 - [Static Analysis] infer errors in GeckoView

Status: RESOLVED FIXED

(GeckoView :: General, defect, P3)

Description

[Robert Bartlensky [:rbartlensky]]

// 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");

Comment 1

[Robert Bartlensky [:rbartlensky]]

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

Comment 2

[Robert Bartlensky [:rbartlensky]]

I assume 2 is a false positive, but what about 4? Should I ignore that one as well?

Comment 3

[Andi [:andi]]

Sebastian what do you think about these?

Comment 4

[Eugen Sawin [:esawin]]

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

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

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.

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

Clearing NI: Changes here are up to the GeckoView team.

Comment 7

[Robert Bartlensky [:rbartlensky]]

Attachment: Bug 1477707: Fix all infer errors in GeckoView.

This fixes some errors reported by infer on GeckoView that are not related to threading.

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 9002461 [details]
Bug 1477707: Fix all infer errors in GeckoView.

Jim Chen [:jchen] [:darchons] has approved the revision.

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/8e638a82d7da

Comment 10

[Chris Peterson [:cpeterson]]

status-firefox62=wontfix because we don't need to uplift these fixes to GV 62 Beta.