Closed Bug 1048589 Opened 6 years ago Closed 6 years ago

|X11Error| can use |first_error| uninitialized

Categories

(Core :: Widget: Gtk, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED INVALID

People

(Reporter: erahm, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: [CID 1225495])

It looks like it's possible for |first_error| to be used uninitialized in |X11Error| [1] if |MOZ_WIDGET_GTK| is defined. This could happen if |XQueryExtension| fails or |nExts == 0|.

Coverity analysis:

3. var_decl: Declaring variable first_error without initializer.
 43      int first_error;
   
4. Condition extNames, taking true branch
 44      if (extNames) {
   
5. Condition i < nExts, taking false branch
 45        for (int i = 0; i < nExts; ++i) {
 46          int major_opcode, first_event;
 47          if (XQueryExtension(tmpDisplay, extNames[i],
 48                              &major_opcode, &first_event, &first_error)
 49              && major_opcode == event->request_code) {
 50            message.Append(extNames[i]);
 51            message.Append('.');
 52            message.AppendInt(event->minor_code);
 53            break;
 54          }
 55        }
 56
 57        XFreeExtensionList(extNames);
 58      }
 59      XCloseDisplay(tmpDisplay);
 60
 61#if (MOZ_WIDGET_GTK == 2)
 62      // GDK2 calls XCloseDevice the devices that it opened on startup, but
 63      // the XI protocol no longer ensures that the devices will still exist.
 64      // If they have been removed, then a BadDevice error results.  Ignore
 65      // this error.
   
6. Condition message.EqualsLiteral("XInputExtension.4"), taking true branch
   
CID 1225495 (#1 of 1): Uninitialized scalar variable (UNINIT)7. uninit_use: Using uninitialized value first_error.
 66      if (message.EqualsLiteral("XInputExtension.4") &&
 67          event->error_code == first_error + 0) {
 68        return 0;
 69      }
 70#endif

[1] http://hg.mozilla.org/mozilla-central/annotate/71497ed2e0db/toolkit/xre/nsX11ErrorHandler.cpp#l67
Also note |first_error| could be an incorrect value as well if |XQueryExtension succeeds|, but |major_opcode != event->request_code|.
(In reply to Eric Rahm [:erahm] from comment #0)
> Coverity analysis:

> 6. Condition message.EqualsLiteral("XInputExtension.4"), taking true branch

|message| is only non-empty if XQueryExtension() returns non-zero and
|major_opcode != event->request_code|.

Is the bug that coverity is not analysing elements of arrays?
(In reply to Karl Tomlinson (:karlt) from comment #2)
> (In reply to Eric Rahm [:erahm] from comment #0)
> > Coverity analysis:
> 
> > 6. Condition message.EqualsLiteral("XInputExtension.4"), taking true branch
> 
> |message| is only non-empty if XQueryExtension() returns non-zero and
> |major_opcode != event->request_code|.
> 

Ugh I usually catch stuff like that, sorry!

> Is the bug that coverity is not analysing elements of arrays?

It's static analysis, which unfortunately has it's limits. I'll go ahead and close this, thanks for the quick feedback.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.