When defining functions with two or more format attributes, if the
format strings don't have the same format family, there is a false
positive warning that the incorrect kind of format string is being
passed at forwarded format string call sites.
This happens because we check that the format string family of each
format attribute is compatible before we check that we're using the
associated format parameter. The fix is to move the check down one
scope, after we've established that we are checking the right parameter.
Tests are updated to include a true negative and a true positive of this
situation.
This implements ``__attribute__((format_matches))``, as described in the
RFC:
https://discourse.llvm.org/t/rfc-format-attribute-attribute-format-like/83076
The ``format`` attribute only allows the compiler to check that a format
string matches its arguments. If the format string is passed
independently of its arguments, there is no way to have the compiler
check it. ``format_matches(flavor, fmtidx, example)`` allows the
compiler to check format strings against the ``example`` format string
instead of against format arguments. See the changes to AttrDocs.td in
this diff for more information.
Implementation-wise, this change subclasses CheckPrintfHandler and
CheckScanfHandler to allow them to collect specifiers into arrays, and
implements comparing that two specifiers are equivalent.
`checkFormatStringExpr` gets a new `ReferenceFormatString` argument that
is piped down when calling a function with the `format_matches`
attribute (and is `nullptr` otherwise); this is the string that the
actual format string is compared against.
Although this change does not enable -Wformat-nonliteral by default,
IMO, all the pieces are now in place such that it could be.
We have a new policy in place making links to private resources
something we try to avoid in source and test files. Normally, we'd
organically switch to the new policy rather than make a sweeping change
across a project. However, Clang is in a somewhat special circumstance
currently: recently, I've had several new contributors run into rdar
links around test code which their patch was changing the behavior of.
This turns out to be a surprisingly bad experience, especially for
newer folks, for a handful of reasons: not understanding what the link
is and feeling intimidated by it, wondering whether their changes are
actually breaking something important to a downstream in some way,
having to hunt down strangers not involved with the patch to impose on
them for help, accidental pressure from asking for potentially private
IP to be made public, etc. Because folks run into these links entirely
by chance (through fixing bugs or working on new features), there's not
really a set of problematic links to focus on -- all of the links have
basically the same potential for causing these problems. As a result,
this is an omnibus patch to remove all such links.
This was not a mechanical change; it was done by manually searching for
rdar, radar, radr, and other variants to find all the various
problematic links. From there, I tried to retain or reword the
surrounding comments so that we would lose as little context as
possible. However, because most links were just a plain link with no
supporting context, the majority of the changes are simple removals.
Differential Review: https://reviews.llvm.org/D158071
This patch warns on snprintf calls whose n argument is known to be smaller than the size of the formatted string like
```
char buf[5];
snprintf(buf, 5, "Hello");
```
This is a counterpart of gcc's Wformat-truncation
Fixes https://github.com/llvm/llvm-project/issues/64871
Reviewed By: aaron.ballman, nickdesaulniers
Differential Revision: https://reviews.llvm.org/D158562
Fix https://github.com/llvm/llvm-project/issues/62247
D131057 added `bArg` and `BArg` in the `AsLongLong` label in
`FormatSpecifier::hasValidLengthModifier`, but missed the `AsLong` label,
therefore `%llb` is allowed while `%lb` (e.g. `printf("%lb", (long)10)`) has a
spurious warning. Add the missing case labels.
Reviewed By: aaron.ballman, enh
Differential Revision: https://reviews.llvm.org/D148779
The main focus of this patch is to make ArgType::matchesType check for
possible default parameter promotions when the argType is not a pointer.
If so, no warning will be given for `int`, `unsigned int` types as
corresponding arguments to %hhd and %hd. However, the usage of %hhd
corresponding to short is relatively rare, and it is more likely to be a
misuse. This patch keeps the original behavior of clang like this as
much as possible, while making it more convenient to consider the
default arguments promotion.
Fixes https://github.com/llvm/llvm-project/issues/57102
Reviewed By: aaron.ballman, nickdesaulniers, #clang-language-wg
Differential Revision: https://reviews.llvm.org/D132568
Close#56885: WG14 N2630 added %b to fprintf/fscanf and recommended %B for
fprintf. This patch teaches -Wformat %b for the printf/scanf family of functions
and %B for the printf family of functions.
glibc 2.35 and latest Android bionic added %b/%B printf support. From
https://www.openwall.com/lists/libc-coord/2022/07/ no scanf support is available
yet.
Like GCC, we don't test library support.
GCC 12 -Wformat -pedantic emits a warning:
> warning: ISO C17 does not support the ‘%b’ gnu_printf format [-Wformat=]
The behavior is not ported.
Note: `freebsd_kernel_printf` uses %b differently.
Reviewed By: aaron.ballman, dim, enh
Differential Revision: https://reviews.llvm.org/D131057
Clang has traditionally allowed C programs to implicitly convert
integers to pointers and pointers to integers, despite it not being
valid to do so except under special circumstances (like converting the
integer 0, which is the null pointer constant, to a pointer). In C89,
this would result in undefined behavior per 3.3.4, and in C99 this rule
was strengthened to be a constraint violation instead. Constraint
violations are most often handled as an error.
This patch changes the warning to default to an error in all C modes
(it is already an error in C++). This gives us better security posture
by calling out potential programmer mistakes in code but still allows
users who need this behavior to use -Wno-error=int-conversion to retain
the warning behavior, or -Wno-int-conversion to silence the diagnostic
entirely.
Differential Revision: https://reviews.llvm.org/D129881
Clang only allows you to use __attribute__((format)) on variadic functions. There are legit use cases for __attribute__((format)) on non-variadic functions, such as:
(1) variadic templates
```c++
template<typename… Args>
void print(const char *fmt, Args… &&args) __attribute__((format(1, 2))); // error: format attribute requires variadic function
```
(2) functions which take fixed arguments and a custom format:
```c++
void print_number_string(const char *fmt, unsigned number, const char *string) __attribute__((format(1, 2)));
// ^error: format attribute requires variadic function
void foo(void) {
print_number_string(“%08x %s\n”, 0xdeadbeef, “hello”);
print_number_string(“%d %s”, 0xcafebabe, “bar”);
}
```
This change allows Clang users to attach __attribute__((format)) to non-variadic functions, including functions with C++ variadic templates. It replaces the error with a GCC compatibility warning and improves the type checker to ensure that received arrays are treated like pointers (this is a possibility in C++ since references to template types can bind to arrays).
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D112579
rdar://84629099
A significant number of our tests in C accidentally use functions
without prototypes. This patch converts the function signatures to have
a prototype for the situations where the test is not specific to K&R C
declarations. e.g.,
void func();
becomes
void func(void);
This is the second batch of tests being updated (there are a significant
number of other tests left to be updated).
The `printf` specifier `%n` is not supported on Android's libc and will soon be removed from Fuchsia's
Reviewed By: enh
Differential Revision: https://reviews.llvm.org/D117611
- `[[format(archetype, fmt-idx, ellipsis)]]` specifies that a function accepts a
format string and arguments according to `archetype`. This is how Clang
type-checks `printf` arguments based on the format string.
- Clang has a `-Wformat-nonliteral` warning that is triggered when a function
with the `format` attribute is called with a format string that is not
inspectable because it isn't constant. This warning is suppressed if the
caller has the `format` attribute itself and the format argument to the callee
is the caller's own format parameter.
- When using the `format` attribute on a block, Clang wouldn't recognize its
format parameter when calling another function with the format attribute. This
would cause unsuppressed -Wformat-nonliteral warnings for no supported reason.
Reviewed By: ahatanak
Differential Revision: https://reviews.llvm.org/D112569
Radar-Id: rdar://84603673
error: 'error' diagnostics expected but not seen:
File /vol/llvm/src/clang/local/test/Sema/wchar.c Line 22: initializing wide char array with non-wide string literal
error: 'error' diagnostics seen but not expected:
File /vol/llvm/src/clang/local/test/Sema/wchar.c Line 20: array initializer must be an initializer list
File /vol/llvm/src/clang/local/test/Sema/wchar.c Line 22: array initializer must be an initializer list
It turns out the definition is wrong, as can be seen in GCC's gcc/config/sol2.h:
/* wchar_t is called differently in <wchar.h> for 32 and 64-bit
compilations. This is called for by SCD 2.4.1, p. 6-83, Figure 6-65
(32-bit) and p. 6P-10, Figure 6.38 (64-bit). */
#undef WCHAR_TYPE
#define WCHAR_TYPE (TARGET_64BIT ? "int" : "long int")
The following patch implements this, and at the same time corrects the wint_t
definition which is the same:
/* Same for wint_t. See SCD 2.4.1, p. 6-83, Figure 6-66 (32-bit). There's
no corresponding 64-bit definition, but this is what Solaris 8
<iso/wchar_iso.h> uses. */
#undef WINT_TYPE
#define WINT_TYPE (TARGET_64BIT ? "int" : "long int")
Clang :: Preprocessor/wchar_t.c and Clang :: Sema/format-strings.c need to
be adjusted to account for that.
Tested on i386-pc-solaris2.11, x86_64-pc-solaris2.11, and x86_64-pc-linux-gnu.
Differential Revision: https://reviews.llvm.org/D62944
llvm-svn: 363612
Re-enable format string warnings on printf.
The warnings are still incomplete. Apparently it is undefined to use a
vector specifier without a length modifier, which is not currently
warned on. Additionally, type warnings appear to not be working with
the hh modifier, and aren't warning on all of the special restrictions
from c99 printf.
llvm-svn: 352540
This avoids spurious warnings, but could use
a lot of work. For example the number of vector
elements is not verified, and the passed
value type is not checked.
Fixes bug 39486
llvm-svn: 346806
I had locally changed the test to add an explicit triple to figure out the issue
with the SCEI buildbots, and that hid the error. This now works with and
without the explicit triple.
llvm-svn: 342581
In the case that `win_t` is an `unsigned short` (e.g. on Windows), we would
previously incorrectly diagnose the conversion because we would immediately
promote the argument type from `wint_t` (aka `unsigned short`) to `int` before
checking if the type matched. This should repair the Windows hosted bots.
llvm-svn: 342565
Summary:
The warning for a format string not being a string literal and therefore
being potentially insecure is overly strict for indices into string
literals. This fix checks if the index into the string literal is
precomputable. If that's the case it will check if the suffix of that
string literal is a valid format string string literal. It will still
issue the aforementioned warning for out of range indices into the
string literal.
Patch by Meike Baumgärtner (meikeb)
Reviewers: rsmith
Subscribers: srhines, cfe-commits
Differential Revision: https://reviews.llvm.org/D24584
llvm-svn: 281686
Summary: This reverts r281527 because I messed up the attribution.
Reviewers: srhines
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D24579
llvm-svn: 281530
Summary:
The warning for a format string not being a sting literal and therefore
being potentially insecure is overly strict for indecies into sting
literals. This fix checks if the index into the string literal is
precomputable. If thats the case it will check if the suffix of that
sting literal is a valid format string string literal. It will still
issue the aforementioned warning for out of range indecies into the
string literal.
Reviewers: rsmith
Subscribers: srhines, cfe-commits
Differential Revision: https://reviews.llvm.org/D23820
llvm-svn: 281527
r263299 added a fixit for the -Wformat-security warning, but that runs
into complications with our guideline that error recovery should be done
as-if the fixit had been applied. Putting the fixit on a note avoids that.
llvm-svn: 263584
Summary:
The printf/scanf format checker is a little over-zealous in handling the conditional operator. This patch reduces work by not checking code-paths that are never used and reduces false positives regarding uncovered arguments, for example in the code fragment:
printf(minimal ? "%i\n" : "%i: %s\n", code, msg);
Reviewers: rtrieu
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D15636
llvm-svn: 262025
glibc expects that stddef.h only defines a single thing if either of these
defines is set. For example, before this change, a C file containing
#include <stdlib.h>
int ptrdiff_t = 0;
would compile with gcc but not with clang. Now it compiles with clang too.
This also fixes PR12997, where older versions of the Linux headers would define
NULL incorrectly, and glibc would define __need_NULL and expect stddef.h to
redefine NULL with the correct definition.
llvm-svn: 207606
While '%n' can be used for evil in an attacker-controlled format string, there
isn't any acute danger in using it in a literal format string with an argument
of the appropriate type.
llvm-svn: 160984
about argument type mismatch.
This gives a nicer diagnostic in cases like
printf(fmt,
i);
where previously the snippet just pointed at 'fmt' (with a note at the
definition of fmt).
It's a wash for cases like
printf("%f",
i);
where previously we snippeted the offending portion of the format string,
but didn't indicate which argument was at fault.
llvm-svn: 156968
For "%hhx", printf expects an unsigned char. This makes Clang
accept a 'char' argument for that also when using -funsigned-char.
This fixes PR12761.
llvm-svn: 156388
Teach ASTContext about WIntType, and have it taken from TargetInfo like WCharType. Should fix test/Sema/format-strings.c for ARM, with the exception of one subtest which will fail if wint_t and wchar_t are the same size and wint_t is signed, wchar_t is unsigned.
There'll be a followup commit to fix that.
Reviewed by Chandler and Hans at http://llvm.org/reviews/r/8
llvm-svn: 156165
