Missing -Wformat-extra-args warning when calling basename() in printf() argument

[Quuxplusone]

Bugzilla Link	PR36972
Status	NEW
Importance	P enhancement
Reported by	Bernie Innocenti (bernie@codewiz.org)
Reported on	2018-04-01 19:12:06 -0700
Last modified on	2018-04-01 22:21:45 -0700
Version	trunk
Hardware	PC Linux
CC	dgregor@apple.com, llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments	0001-Sema-Re-enable-Wformat-extra-args-even-when-system-m.patch (1483 bytes, text/plain)
Blocks
Blocked by
See also

All versions of clang I tried did not warn about an extra printf argument in
the testcase below.

Fails in: clang 7.0.1, 5.0.1, 3.8.275480
Works in: gcc 7.3.0

Reduced testcase:
--------------------------------------------
#include <stdio.h>
#include <libgen.h>  // basename()

int main(int argc, char** argv) {
    // Fails to emit -Wformat-extra-args diagnostic
    printf("too %s", "many", basename(argv[0]));

    // Works as expected
    printf("too %s", "many", __xpg_basename(argv[0]));
}
--------------------------------------------

[Quuxplusone]

The reason this happens is that basename is a macro defined in a system header,
and the extra printf argument diagnostic is suppressed if the expression
contains a system macro:

f883b21a961 lib/Sema/SemaChecking.cpp (Andy Gibbs             2016-02-26
15:35:16 +0000  5514)   if (S.getSourceManager().isInSystemMacro(Loc))
f883b21a961 lib/Sema/SemaChecking.cpp (Andy Gibbs             2016-02-26
15:35:16 +0000  5515)     return;

[Quuxplusone]

The check for system macros was added in this commit: https://github.com/llvm-mirror/clang/commit/c03f2df2a2b

It was intended as a work-around to suppress the diagnostic for NSAssert macros defined in the OS X 10.7 system headers. I think it's time to revisit, since the current OS X headers seem to be disabling the warning:

https://github.com/apportable/Foundation/blob/master/System/Foundation/include/Foundation/NSException.h#L33

GCC currently emits the warning, so I don't expect a lot of code to break if clang went back to the same behavior.

[Quuxplusone]

Attached 0001-Sema-Re-enable-Wformat-extra-args-even-when-system-m.patch (1483 bytes, text/plain): patch

[Quuxplusone]

Attached the obvious patch, in case maintainers agree with my analysis
(but feel free to propose a different fix, of course).

[Quuxplusone]

Please be aware that comments posted to bugzilla after a bug is filed do not trigger notifications to anyone not on the CC list (and even the llvm-bugs@ list doesn't get mail for added comments). Patches should be sent to cfe-commits@lists.llvm.org or to the phabricator instance at reviews.llvm.org so that someone sees them :)

(Your patch will also need a test.)