Page MenuHomePhabricator

[Clang][Sema] Fix attribute((format)) bug on non-variadic functions
ClosedPublic

Authored by fcloutier on Nov 7 2022, 7:49 PM.

Details

Summary

The initial implementation of __attribute__((format)) on non-variadic functions accidentally only accepted one data argument. This worked:

__attribute__((format(printf, 1, 2)))
void f(const char *, int);

but this didn't:

__attribute__((format(printf, 1, 2)))
void f(const char *, int, int);

This is due to an oversight in changing the way diagnostics are emitted for attribute((format)), and to a coincidence in the handling of the variadic case. Test cases only covered the case that worked by coincidence.

Before the previous change, using __attribute__((format)) on a non-variadic function at all was an error and clang bailed out. After that change, it only generates a GCC compatibility warning. However, as execution falls through, it hits a second diagnostic when the first data argument is neither 0 nor the last parameter of the function.

This change updates that check to allow any parameter after the format string to be the first data argument when the function is non-variadic. When the function is variadic, it still needs to be the index of the ... "parameter". Attribute documentation is updated to reflect the change and new tests are added to verify that it works with two data parameters.

rdar://102069446

Diff Detail

Event Timeline

fcloutier created this revision.Nov 7 2022, 7:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 7 2022, 7:49 PM
fcloutier requested review of this revision.Nov 7 2022, 7:49 PM
aaron.ballman accepted this revision.Nov 30 2022, 8:38 AM

LGTM! This should probably have a release note as well.

clang/lib/Sema/SemaDeclAttr.cpp
3897–3909

Given that we know what value we want to see there, should we emit a FixIt in these cases to adjust the format arg to the right value? (Can be done in a follow-up if you feel like doing it, I don't insist.)

This revision is now accepted and ready to land.Nov 30 2022, 8:38 AM
fcloutier updated this revision to Diff 479427.Dec 1 2022, 3:01 PM

Addressed Aaron's comment about fixits, ran clang-format.

aaron.ballman accepted this revision.Dec 2 2022, 7:04 AM

LGTM, thank you!

clang/docs/ReleaseNotes.rst
322
fcloutier updated this revision to Diff 479658.Dec 2 2022, 10:57 AM

Fix new fixit test for Windows, where directory separators are backslashes instead of forward slashes.