1476565 - [Static Analysis] infer errors in dom/base/*

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Robert Bartlensky [:rbartlensky]]

dom/base/nsImageLoadingContent.cpp:450: error: DEAD_STORE
  The value written to &rv (type int) is never used.
  448.     }
  449.
  450. >   nsresult rv = NS_OK;
  451.     RefPtr<imgRequestProxy> currentReq;
  452.     if (mCurrentRequest) {

dom/base/nsObjectLoadingContent.cpp:729: error: DEAD_STORE
  The value written to &rv (type int) is never used.
  727.     }
  728.
  729. >   nsresult rv = NS_ERROR_FAILURE;
  730.     RefPtr<nsPluginHost> pluginHost = nsPluginHost::GetInst();
  731.

dom/base/nsGlobalWindowOuter.cpp:1696: error: DEAD_STORE
  The value written to &rv (type int) is never used.
  1694.                             GetCurrentInnerWindowInternal();
  1695.
  1696. >   nsresult rv = NS_OK;
  1697.
  1698.     // We set mDoc even though this is an outer window to avoid

dom/base/nsObjectLoadingContent.cpp:2154: error: DEAD_STORE
  The value written to &rv (type int) is never used.
  2152.     {
  2153.       MOZ_ASSERT_UNREACHABLE("Trying to load new plugin with existing content");
  2154. >     rv = NS_ERROR_UNEXPECTED;
  2155.       return NS_OK;
  2156.     }

dom/base/nsObjectLoadingContent.cpp:2162: error: DEAD_STORE
  The value written to &rv (type int) is never used.
  2160.     if (mType != eType_Null && !!mChannel != mChannelLoaded) {
  2161.       MOZ_ASSERT_UNREACHABLE("Trying to load with bad channel state");
  2162. >     rv = NS_ERROR_UNEXPECTED;
  2163.       return NS_OK;
  2164.     }

dom/base/nsContentUtils.cpp:2994: error: DEAD_STORE
  The value written to &index (type int) is never used.
  2992.
  2993.         // If in a form, add form name / index of form / index in form
  2994. >       int32_t index = -1;
  2995.         Element *formElement = control->GetFormElement();
  2996.         if (formElement) {

dom/base/nsGlobalWindowOuter.cpp:3651: error: DEAD_STORE
  The value written to &winLeft (type int) is never used.
  3649.         // convert those values to CSS pixels
  3650.         // XXX four separate retrievals of the prescontext
  3651. >       winLeft   = DevToCSSIntPixels(winLeft);
  3652.         winTop    = DevToCSSIntPixels(winTop);
  3653.         winWidth  = DevToCSSIntPixels(winWidth);

dom/base/nsGlobalWindowOuter.cpp:3652: error: DEAD_STORE
  The value written to &winTop (type int) is never used.
  3650.         // XXX four separate retrievals of the prescontext
  3651.         winLeft   = DevToCSSIntPixels(winLeft);
  3652. >       winTop    = DevToCSSIntPixels(winTop);
  3653.         winWidth  = DevToCSSIntPixels(winWidth);
  3654.         winHeight = DevToCSSIntPixels(winHeight);

dom/base/nsGlobalWindowInner.cpp:4898: error: DEAD_STORE
  The value written to &result (type _Bool) is never used.
  4896.     NS_ENSURE_SUCCESS(rv, rv);
  4897.
  4898. >   bool result = true;
  4899.     AutoJSAPI jsapi;
  4900.     result = jsapi.Init(this);

dom/base/nsGlobalWindowInner.cpp:4884: error: DEAD_STORE
  The value written to &rv (type int) is never used.
  4882.                  "Must be safe to run script here.");
  4883.
  4884. >   nsresult rv = NS_OK;
  4885.
  4886.     // Bail if the window is frozen.


dom/base/nsGlobalWindowInner.cpp:5625: error: UNINITIALIZED_VALUE
  The value read from lineno was never initialized.
  5623.         msg.Append(scriptLocation);
  5624.         msg.Append(':');
  5625. >       msg.AppendInt(lineno);
  5626.       }
  5627.     }

dom/base/nsContentUtils.cpp:6565: error: DEAD_STORE
  The value written to &rv (type int) is never used.
  6563.     }
  6564.
  6565. >   nsresult rv = NS_OK;
  6566.     JS::Rooted<JSObject*> scope(cx, JS::CurrentGlobalOrNull(cx));
  6567.     rv = sXPConnect->WrapNativeToJSVal(cx, scope, native, cache, aIID,

dom/base/nsGlobalWindowInner.cpp:6684: error: DEAD_STORE
  The value written to &rv (type int) is never used.
  6682.         options.setNoScriptRval(true);
  6683.         JS::Rooted<JSObject*> global(aes.cx(), FastGetGlobalJSObject());
  6684. >       nsresult rv = NS_OK;
  6685.         {
  6686.           nsJSUtils::ExecutionContext exec(aes.cx(), global);

dom/base/nsGlobalWindowInner.cpp:6732: error: UNINITIALIZED_VALUE
  The value read from nestingLevel was never initialized.
  6730.
  6731.     if (trackNestingLevel) {
  6732. >     TimeoutManager::SetNestingLevel(nestingLevel);
  6733.     }
  6734.

dom/base/nsGlobalWindowInner.cpp:7511: error: NULL_DEREFERENCE
  pointer `shell` last assigned on line 7508 could be null and is dereferenced at line 7511, column 7.
  7509.     MOZ_ASSERT(shell);
  7510.
  7511. >   if (shell->NeedStyleFlush() || shell->NeedLayoutFlush()) {
  7512.       // By the time our observer fired, something has already invalidated
  7513.       // style or layout - or perhaps we're still in the middle of a flush that

Comment 1

[Robert Bartlensky [:rbartlensky]]

Attachment: Bug 1476565: Fix DEAD_STORE errors in dom/base/*.

Review commit: https://reviewboard.mozilla.org/r/257808/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/257808/

Comment 2

[Andrea Marchesini [:baku]]

Comment on attachment 8992994 [details]
Bug 1476565: Fix DEAD_STORE errors in dom/base/*.

https://reviewboard.mozilla.org/r/257808/#review264742

::: dom/base/nsContentUtils.cpp:2942
(Diff revision 1)
>  
>        // Append the control type
>        KeyAppendInt(control->ControlType(), aKey);
>  
>        // If in a form, add form name / index of form / index in form
> -      int32_t index = -1;
> +      int32_t index;

I don't like this. Do this instead:

1. remove this variable here.
2. line 2953: int32_t index = htmlForm...
3. line 2994: int32_t index = htmlForm...

::: dom/base/nsImageLoadingContent.cpp:450
(Diff revision 1)
>  {
>    if (NS_WARN_IF(!aObserver)) {
>      return;
>    }
>  
> -  nsresult rv = NS_OK;
> +  nsresult rv;

here as well. Remove this line and at line 456 and line  465 do: nsresult rv = ...

::: dom/base/nsObjectLoadingContent.cpp:728
(Diff revision 1)
>    if (!thisContent->GetPrimaryFrame()) {
>      LOG(("OBJLC [%p]: Not instantiating plugin with no frame", this));
>      return NS_OK;
>    }
>  
> -  nsresult rv = NS_ERROR_FAILURE;
> +  nsresult rv;

move this to line 745.

Comment 3

[Robert Bartlensky [:rbartlensky]]

Comment on attachment 8992994 [details]
Bug 1476565: Fix DEAD_STORE errors in dom/base/*.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/257808/diff/1-2/

Comment 4

[Andrea Marchesini [:baku]]

Comment on attachment 8992994 [details]
Bug 1476565: Fix DEAD_STORE errors in dom/base/*.

https://reviewboard.mozilla.org/r/257808/#review264790

Thanks!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4f24738dbf7
Fix DEAD_STORE errors in dom/base/*. r=baku

Comment 6

[Eliza Balazs [:ebalazs_]]

Backed out changeset f4f24738dbf7 (bug 1476565) for bustage in /builds/worker/workspace/build/src/dom/base/nsGlobalWindowOuter.cpp

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f4f24738dbf743d0a244fa00a296d78417c72047&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable&selectedJob=188953943

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=188953943&repo=autoland&lineNumber=1071

Backout push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4adf441211197568b07dde0bec5e916582024928&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable

Comment 7

[Robert Bartlensky [:rbartlensky]]

Comment on attachment 8992994 [details]
Bug 1476565: Fix DEAD_STORE errors in dom/base/*.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/257808/diff/2-3/

Comment 8

[Robert Bartlensky [:rbartlensky]]

(In reply to Eliza Balazs [:ebalazs_] from comment #6)
> Backed out changeset f4f24738dbf7 (bug 1476565) for bustage in
> /builds/worker/workspace/build/src/dom/base/nsGlobalWindowOuter.cpp
> 
> Push with failures:
> https://treeherder.mozilla.org/#/
> jobs?repo=autoland&revision=f4f24738dbf743d0a244fa00a296d78417c72047&filter-
> resultStatus=testfailed&filter-resultStatus=busted&filter-
> resultStatus=exception&filter-resultStatus=usercancel&filter-
> resultStatus=runnable&selectedJob=188953943
> 
> Failure log:
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=188953943&repo=autoland&lineNumber=1071
> 
> Backout push:
> https://treeherder.mozilla.org/#/
> jobs?repo=autoland&revision=4adf441211197568b07dde0bec5e916582024928&filter-
> resultStatus=testfailed&filter-resultStatus=busted&filter-
> resultStatus=exception&filter-resultStatus=usercancel&filter-
> resultStatus=runnable

Fixed, waiting for approval.

Comment 9

[Robert Bartlensky [:rbartlensky]]

I moved the variable to the top because I didn't realize that the build was not working properly. Now it does: 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=430ada42af0e39b9adf9dc9fe488279912d43edc

Comment 10

[Andrea Marchesini [:baku]]

good to me.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/25e31aa41583
Fix DEAD_STORE errors in dom/base/*. r=baku

Comment 12

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/25e31aa41583