Bug 98195 - Fix -Wsuggest-attribute warnings from gcc and clang
Summary: Fix -Wsuggest-attribute warnings from gcc and clang
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: D-Bus Maintainers
URL:
Whiteboard: review?
Keywords: patch
Depends on: 88922 97357
Blocks:
  Show dependency treegraph
 
Reported: 2016-10-10 14:54 UTC by Simon McVittie
Modified: 2016-11-07 18:34 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
_dbus_listen_tcp_socket: correct format string (1.15 KB, patch)
2016-10-10 16:51 UTC, Simon McVittie
Details | Splinter Review
dbus_signature_validate: be sure to use a literal format string (1.10 KB, patch)
2016-10-10 16:51 UTC, Simon McVittie
Details | Splinter Review
dbus-nonce: print sockets correctly (2.66 KB, patch)
2016-10-10 16:52 UTC, Simon McVittie
Details | Splinter Review
Print errors parsing match rules correctly (1.42 KB, patch)
2016-10-10 16:52 UTC, Simon McVittie
Details | Splinter Review
Print XML parse errors correctly (1.48 KB, patch)
2016-10-10 16:52 UTC, Simon McVittie
Details | Splinter Review
dbus-file-win: print a HANDLE correctly (952 bytes, patch)
2016-10-10 16:53 UTC, Simon McVittie
Details | Splinter Review
dbus-launch-x11: print a window ID portably (1.11 KB, patch)
2016-10-10 16:53 UTC, Simon McVittie
Details | Splinter Review
test-privserver: avoid -Wformat-security (1022 bytes, patch)
2016-10-10 16:53 UTC, Simon McVittie
Details | Splinter Review
test-segfault: mark exception_handler as NORETURN (900 bytes, patch)
2016-10-10 16:54 UTC, Simon McVittie
Details | Splinter Review
Enable format, noreturn, unused attributes for clang (1.02 KB, patch)
2016-10-10 16:54 UTC, Simon McVittie
Details | Splinter Review
Add missing format attributes suggested by -Wsuggest-attribute=format (3.31 KB, patch)
2016-10-10 16:55 UTC, Simon McVittie
Details | Splinter Review
Add missing function attributes suggested by clang (but not by gcc) (12.02 KB, patch)
2016-10-10 16:55 UTC, Simon McVittie
Details | Splinter Review
Configure the compiler to suggest useful function attributes (917 bytes, patch)
2016-10-10 16:57 UTC, Simon McVittie
Details | Splinter Review

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.