Open Bug 1405142 Opened 7 years ago Updated 2 years ago

fsanitize=enum (ubsan) runtime errors for GtkStateFlags

Categories

(Core :: Widget: Gtk, defect, P3)

All
Linux
defect

Tracking

()

People

(Reporter: arthur, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [tor])

When I run automated tests of linux64-asan (clang) with -fsanitize=enum, I get the following errors: widget/gtk/WidgetStyleCache.cpp:1236:23 87 load of value 2048, which is not a valid value for type 'GtkStateFlags' widget/gtk/WidgetStyleCache.cpp:1252:21 1698 load of value 128, which is not a valid value for type 'GtkStateFlags' widget/gtk/WidgetStyleCache.cpp:1261:7 1698 load of value 128, which is not a valid value for type 'GtkStateFlags' widget/gtk/gtk3drawing.cpp:1039:57 11 load of value 2048, which is not a valid value for type 'GtkStateFlags' widget/gtk/gtk3drawing.cpp:1901:46 20 load of value 2048, which is not a valid value for type 'GtkStateFlags' widget/gtk/gtk3drawing.cpp:1913:55 20 load of value 2048, which is not a valid value for type 'GtkStateFlags' widget/gtk/gtk3drawing.cpp:343:50 2 load of value 2048, which is not a valid value for type 'GtkStateFlags' widget/gtk/gtk3drawing.cpp:346:40 71 load of value 2048, which is not a valid value for type 'GtkStateFlags' widget/gtk/gtk3drawing.cpp:506:42 77 load of value 128, which is not a valid value for type 'GtkStateFlags' widget/gtk/gtk3drawing.cpp:508:41 77 load of value 128, which is not a valid value for type 'GtkStateFlags' widget/gtk/nsLookAndFeel.cpp:184:37 1698 load of value 128, which is not a valid value for type 'GtkStateFlags' widget/gtk/nsLookAndFeel.cpp:204:50 1698 load of value 128, which is not a valid value for type 'GtkStateFlags' Here's an example stack trace: [task 2017-10-02T07:13:26.510Z] 07:13:26 INFO - GECKO(1092) | #0 0x7fecfcad4582 in GetStyleContext(WidgetNodeType, GtkTextDir\ ection, GtkStateFlags, unsigned int) /builds/worker/workspace/build/src/widget/gtk/WidgetStyleCache.cpp:1252:21^M [task 2017-10-02T07:13:26.528Z] 07:13:26 INFO - GECKO(1092) | #1 0x7fecfcb1a598 in nsLookAndFeel::EnsureInit() /builds/worker\ /workspace/build/src/widget/gtk/nsLookAndFeel.cpp:832:13^M [task 2017-10-02T07:13:26.530Z] 07:13:26 INFO - GECKO(1092) | #2 0x7fecfcb1cd16 in nsLookAndFeel::NativeGetColor(mozilla::Loo\ kAndFeel::ColorID, unsigned int&) /builds/worker/workspace/build/src/widget/gtk/nsLookAndFeel.cpp:226:5^M [task 2017-10-02T07:13:26.531Z] 07:13:26 INFO - GECKO(1092) | #3 0x7fecfca883a1 in nsXPLookAndFeel::GetColorImpl(mozilla::Loo\ kAndFeel::ColorID, bool, unsigned int&) /builds/worker/workspace/build/src/widget/nsXPLookAndFeel.cpp:855:27^M [task 2017-10-02T07:13:26.532Z] 07:13:26 INFO - GECKO(1092) | #4 0x7fed0080e14c in nsWebBrowser::Create() /builds/worker/work\ space/build/src/toolkit/components/browser/nsWebBrowser.cpp:1210:3^M [task 2017-10-02T07:13:26.564Z] 07:13:26 INFO - GECKO(1092) | #5 0x7fecfc2ba9c0 in mozilla::dom::TabChild::Init() /builds/wor\ ker/workspace/build/src/dom/ipc/TabChild.cpp:586:15^M [task 2017-10-02T07:13:26.570Z] 07:13:26 INFO - GECKO(1092) | #6 0x7fecfc31f9aa in mozilla::dom::nsIContentChild::RecvPBrowse\ rConstructor(mozilla::dom::PBrowserChild*, mozilla::dom::IdType<mozilla::dom::TabParent> const&, mozilla::dom::IdType<mozilla::dom::T\ abParent> const&, mozilla::dom::IPCTabContext const&, unsigned int const&, mozilla::dom::IdType<mozilla::dom::ContentParent> const&, \ bool const&) /builds/worker/workspace/build/src/dom/ipc/nsIContentChild.cpp:97:7^M [task 2017-10-02T07:13:26.587Z] 07:13:26 INFO - GECKO(1092) | #7 0x7fecfc242fa3 in mozilla::dom::ContentChild::RecvPBrowserCo\ nstructor(mozilla::dom::PBrowserChild*, mozilla::dom::IdType<mozilla::dom::TabParent> const&, mozilla::dom::IdType<mozilla::dom::TabP\ arent> const&, mozilla::dom::IPCTabContext const&, unsigned int const&, mozilla::dom::IdType<mozilla::dom::ContentParent> const&, boo\ l const&) /builds/worker/workspace/build/src/dom/ipc/ContentChild.cpp:1839:27^M [task 2017-10-02T07:13:26.623Z] 07:13:26 INFO - GECKO(1092) | #8 0x7fecf7793abc in mozilla::dom::PContentChild::OnMessageRece\ ived(IPC::Message const&) /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/PContentChild.cpp:5025:20^M [task 2017-10-02T07:13:26.640Z] 07:13:26 INFO - GECKO(1092) | #9 0x7fecf6fefbeb in mozilla::ipc::MessageChannel::DispatchAsyn\ cMessage(IPC::Message const&) /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:2119:25^M [task 2017-10-02T07:13:26.642Z] 07:13:26 INFO - GECKO(1092) | #10 0x7fecf6fecaff in mozilla::ipc::MessageChannel::DispatchMes\ sage(IPC::Message&&) /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:2049:17^M
And one more representative stack trace: [task 2017-10-02T07:26:41.501Z] 07:26:41 INFO - GECKO(1344) | /builds/worker/workspace/build/src/widget/gtk/nsLookAndFeel.cpp:204\ :50: runtime error: load of value 128, which is not a valid value for type 'GtkStateFlags'^M [task 2017-10-02T07:26:41.503Z] 07:26:41 INFO - GECKO(1344) | #0 0x7f6d3511ff0e in GetBorderColors /builds/worker/workspace/b\ uild/src/widget/gtk/nsLookAndFeel.cpp:204:50^M [task 2017-10-02T07:26:41.507Z] 07:26:41 INFO - GECKO(1344) | #1 0x7f6d3511ff0e in GetBorderColors(_GtkStyleContext*, unsigne\ d int*, unsigned int*) /builds/worker/workspace/build/src/widget/gtk/nsLookAndFeel.cpp:217^M [task 2017-10-02T07:26:41.511Z] 07:26:41 INFO - GECKO(1344) | #2 0x7f6d3511bdd8 in nsLookAndFeel::EnsureInit() /builds/worker\ /workspace/build/src/widget/gtk/nsLookAndFeel.cpp:986:9^M [task 2017-10-02T07:26:41.513Z] 07:26:41 INFO - GECKO(1344) | #3 0x7f6d3511cd16 in nsLookAndFeel::NativeGetColor(mozilla::Loo\ kAndFeel::ColorID, unsigned int&) /builds/worker/workspace/build/src/widget/gtk/nsLookAndFeel.cpp:226:5^M [task 2017-10-02T07:26:41.518Z] 07:26:41 INFO - GECKO(1344) | #4 0x7f6d350883a1 in nsXPLookAndFeel::GetColorImpl(mozilla::Loo\ kAndFeel::ColorID, bool, unsigned int&) /builds/worker/workspace/build/src/widget/nsXPLookAndFeel.cpp:855:27^M [task 2017-10-02T07:26:41.522Z] 07:26:41 INFO - GECKO(1344) | #5 0x7f6d38e0e14c in nsWebBrowser::Create() /builds/worker/work\ space/build/src/toolkit/components/browser/nsWebBrowser.cpp:1210:3^M
The offending values are being produced by the GTK function gtk_style_context_get_state(style); According to the latest gtk source https://github.com/GNOME/gtk/blob/master/gtk/gtkenums.h GTK_STATE_FLAG_DIR_LTR = 1 << 7, // 128 GTK_STATE_FLAG_CHECKED = 1 << 11, // 2048 Maybe that means we are somehow including an old version of gtkenums.h that doesn't have these values? Karl, do you have any insight? Thanks in advance.
Flags: needinfo?(karlt)
Builds are typically against older versions, CentOS 6 gtk+ 3.4 IIRC, so that they run against newer versions. It won't work the other way. Perhaps your analysis can be run on a build on the same system? These are really C enums. The equivalence of C++ enums with C is dependent on implementation, but all implementations provide equivalence so that C++ can be used with C. What is the warning protecting against?
Flags: needinfo?(karlt)
In C++, although the underlying type is implementation defined, the conversion from int to enum is undefined if the value is not within the range of enumeration values. The range of enumeration values depends only on the enumerators, not on the underlying type. In C, the compatible type is still implementation defined, but there is no similar concept of a different range of enumeration values, and so no similar *undefined* behavior. C++ requires "linkage" to C functions. Perhaps that means that behavior is implementation defined when C code loads a value outside the range into storage declared in C++? The fact that GTK+ has added enumerators to increase the minimum precision of the compatible type would seem to imply that it is assuming that compilers were not using char or 8-bit integer compatible types. GtkStateFlags was always passed by value in the GTK+ API, but code compiled against GTK versions with 8-bit enumerators are meant to successfully store > 8-bit values when receiving GtkStateFlags from newer GTK versions so that they can be passed back to GTK. N4296 5.2.9 Static cast 10 A value of integral or enumeration type can be explicitly converted to a complete enumeration type. The value is unchanged if the original value is within the range of the enumeration values (7.2). Otherwise, the behavior is undefined. 7.2 Enumeration declarations 7 For an enumeration whose underlying type is not fixed, the underlying type is an integral type that can represent all the enumerator values defined in the enumeration. If no integral type can represent all the enumerator values, the enumeration is ill-formed. It is implementation-defined which integral type is used as the underlying type except that the underlying type shall not be larger than int unless the value of an enumerator cannot fit in an int or unsigned int 8 For an enumeration whose underlying type is fixed, the values of the enumeration are the values of the underlying type. Otherwise, for an enumeration where e min is the smallest enumerator and e max is the largest, the values of the enumeration are the values in the range b min to b max , defined as follows: Let K be 1 for a two’s complement representation and 0 for a one’s complement or sign-magnitude representation. b max is the smallest value greater than or equal to max(|e min | − K, |e max |) and equal to 2 M − 1, where M is a non-negative integer. b min is zero if e min is non-negative and −(b max + K) otherwise. The size of the smallest bit-field large enough to hold all the values of the enumeration type is max(M, 1) if b min is zero and M + 1 otherwise. It is possible to define an enumeration that has values not defined by any of its enumerators. If the enumerator-list is empty, the values of the enumeration are as if the enumeration had a single enumerator with value 0. 96 96) This set of values is used to define promotion and conversion semantics for the enumeration type. It does not preclude an expression of enumeration type from having a value that falls outside this range. 7.5 Linkage specifications 3 Every implementation shall provide for linkage to functions written in the C programming language, "C", and linkage to C ++ functions, "C++". N1570 (C11) 6.2.5 Types 17 The type char, the signed and unsigned integer types, and the enumerated types are collectively called integer types. The integer and real floating types are collectively called real types. 6.3 Conversions 2 Conversion of an operand value to a compatible type causes no change to the value or the representation. 6.4.4.3 Enumeration constants 2 An identifier declared as an enumeration constant has type int. 6.7.2.2 Enumeration specifiers 4 Each enumerated type shall be compatible with char, a signed integer type, or an unsigned integer type. The choice of type is implementation-defined, 128) but shall be capable of representing the values of all the members of the enumeration.
That still leaves undefined behavior where C++ code is converting to GtkStateFlags. Some options that would avoid performing the conversion in C++: 1) Use a C function to perform the conversion from integer to enumerated type. 2) Use C functions receiving integer types to call each GTK function receiving enumerated types. 3) Provide a different declaration for the GTK functions so that integral types are passed instead of enumeration types, and depend on the calling convention to sort it out.
OS: Unspecified → Linux
Priority: -- → P3
Hardware: Unspecified → All
The warnings from -fsanitize=enum are actually generated on load (not store) of out-of-range values. It's not clear to me that loading out-of-ranges values is actually undefined behavior, when the values may derive from C code. However, the best fix I think would be to modify widget/gtk/compat-gtk3/gtk/gtkenums.h so as to temporarily #define GtkStateFlags during #include_next <gtk/gtkenums.h> and to declare GtkStateFlags with all the most recent enumerators. That would provide that all values are within range and also ensure that the underlying type is of sufficient precision.
Severity: normal → S3
Blocks: ubsan
You need to log in before you can comment on or make changes to this bug.