Bug 98195

Summary: Fix -Wsuggest-attribute warnings from gcc and clang
Product: dbus Reporter: Simon McVittie <smcv>
Component: coreAssignee: Simon McVittie <smcv>
Status: RESOLVED FIXED QA Contact: D-Bus Maintainers <dbus>
Severity: normal    
Priority: medium CC: dbus, tzimmermann
Version: git masterKeywords: patch
Hardware: Other   
OS: All   
Whiteboard: review?
i915 platform: i915 features:
Bug Depends on: 88922, 97357    
Bug Blocks:    
Attachments: _dbus_listen_tcp_socket: correct format string
dbus_signature_validate: be sure to use a literal format string
dbus-nonce: print sockets correctly
Print errors parsing match rules correctly
Print XML parse errors correctly
dbus-file-win: print a HANDLE correctly
dbus-launch-x11: print a window ID portably
test-privserver: avoid -Wformat-security
test-segfault: mark exception_handler as NORETURN
Enable format, noreturn, unused attributes for clang
Add missing format attributes suggested by -Wsuggest-attribute=format
Add missing function attributes suggested by clang (but not by gcc)
Configure the compiler to suggest useful function attributes

Description Simon McVittie 2016-10-10 14:54:42 UTC
+++ This bug was initially created as a clone of Bug #97357 +++

Bug #97357 specifically disables -Wsuggest-attribute warnings.

When I added the suggested attributes, I found a possible security vulnerability (Bug #98157) and some other bugs.

We should enable these compiler warnings, and let the compiler help us.
Comment 1 Simon McVittie 2016-10-10 14:59:34 UTC
Testing <https://github.com/smcv/dbus/tree/suggest-attribute> on Travis-CI.
Comment 2 Simon McVittie 2016-10-10 16:51:04 UTC
Some of these patches assume that the patches from Bug #97357 are applied first.
Comment 3 Simon McVittie 2016-10-10 16:51:26 UTC
Created attachment 127185 [details] [review]
_dbus_listen_tcp_socket: correct format string

res is an integer, not a string.

Bug found by adding more _DBUS_GNUC_PRINTF attributes.
Comment 4 Simon McVittie 2016-10-10 16:51:47 UTC
Created attachment 127186 [details] [review]
dbus_signature_validate: be sure to use a literal  format string

This was not a security vulnerability because
_dbus_validity_to_error_message() doesn't return anything containing
"%", but the compiler can't know that.

Found by adding more _DBUS_GNUC_PRINTF attributes.
Comment 5 Simon McVittie 2016-10-10 16:52:07 UTC
Created attachment 127187 [details] [review]
dbus-nonce: print sockets correctly

Since early 2015, a DBusSocket has been a struct containing either
an int or a pointer-sized Windows SOCKET. Print them with
"%" DBUS_SOCKET_FORMAT and _dbus_socket_printable().
Comment 6 Simon McVittie 2016-10-10 16:52:28 UTC
Created attachment 127188 [details] [review]
Print errors parsing match rules correctly

Not an exploitable vulnerability, just incorrect output.
Comment 7 Simon McVittie 2016-10-10 16:52:55 UTC
Created attachment 127189 [details] [review]
Print XML parse errors correctly
Comment 8 Simon McVittie 2016-10-10 16:53:14 UTC
Created attachment 127190 [details] [review]
dbus-file-win: print a HANDLE correctly

HANDLEs are pointers, not integers.
Comment 9 Simon McVittie 2016-10-10 16:53:31 UTC
Created attachment 127191 [details] [review]
dbus-launch-x11: print a window ID portably

On LP64 platforms, a Window is unsigned long.
Comment 10 Simon McVittie 2016-10-10 16:53:50 UTC
Created attachment 127192 [details] [review]
test-privserver: avoid -Wformat-security

This is not a security vulnerability because it's test code that
should never be compiled in production.
Comment 11 Simon McVittie 2016-10-10 16:54:10 UTC
Created attachment 127193 [details] [review]
test-segfault: mark exception_handler as NORETURN

It calls ExitProcess(), which is correctly detected as not returning.
Comment 12 Simon McVittie 2016-10-10 16:54:30 UTC
Created attachment 127194 [details] [review]
Enable format, noreturn, unused attributes for clang

I'm assuming here that any version of clang will be new enough to
understand gcc 2.4 features, which seems rather safe.
Comment 13 Simon McVittie 2016-10-10 16:55:08 UTC
Created attachment 127195 [details] [review]
Add missing format attributes suggested by  -Wsuggest-attribute=format
Comment 14 Simon McVittie 2016-10-10 16:55:25 UTC
Created attachment 127196 [details] [review]
Add missing function attributes suggested by clang (but  not by gcc)

clang is a little more enthusiastic about suggesting these.
Comment 15 Simon McVittie 2016-10-10 16:57:09 UTC
Created attachment 127197 [details] [review]
Configure the compiler to suggest useful function  attributes

---
Requires Attachment #127179 [details] from Bug #97357.
Comment 16 Simon McVittie 2016-11-07 18:34:25 UTC
Timed out, pushed unreviewed for 1.11.8. Please revert anything that is problematic.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.