Skip to content

Commit d82fed8

Browse files
ahatanakahatanak
committed
Error out if struct or complex type arguments are passed
1 parent b00413b commit d82fed8

File tree

5 files changed

+61
-60
lines changed

5 files changed

+61
-60
lines changed

‎clang/include/clang/Basic/DiagnosticSemaKinds.td‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10448,6 +10448,9 @@ def warn_format_conversion_argument_type_mismatch : Warning<
1044810448
"format specifies type %0 but the argument has "
1044910449
"%select{type|underlying type}2 %1">,
1045010450
InGroup<Format>;
10451+
def err_format_conversion_argument_type_mismatch : Error<
10452+
"format specifies type %0 but the argument has "
10453+
"%select{type|underlying type}2 %1">;
1045110454
def warn_format_conversion_argument_type_mismatch_pedantic : Extension<
1045210455
warn_format_conversion_argument_type_mismatch.Summary>,
1045310456
InGroup<FormatPedantic>;
@@ -10497,6 +10500,8 @@ def warn_printf_asterisk_missing_arg : Warning<
1049710500
def warn_printf_asterisk_wrong_type : Warning<
1049810501
"field %select{width|precision}0 should have type %1, but argument has type %2">,
1049910502
InGroup<Format>;
10503+
def err_printf_asterisk_wrong_type : Error<
10504+
"field %select{width|precision}0 should have type %1, but argument has type %2">;
1050010505
def warn_printf_nonsensical_optional_amount: Warning<
1050110506
"%select{field width|precision}0 used with '%1' conversion specifier, resulting in undefined behavior">,
1050210507
InGroup<Format>;

‎clang/lib/AST/OSLog.cpp‎

Lines changed: 16 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -70,31 +70,22 @@ class OSLogFormatStringHandler
7070
bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS,
7171
const char *StartSpecifier, unsigned SpecifierLen,
7272
const TargetInfo &) override {
73-
// Set the argument expression. Arguments of struct/class/complex types are
74-
// ignored.
75-
auto CheckAndSetArgExpr = [&](unsigned Idx, auto &ArgE) {
76-
const Expr *E = Args[Idx];
77-
if (E && (E->getType()->isRecordType() || E->getType()->isComplexType()))
78-
return false;
79-
ArgE = E;
80-
return true;
81-
};
82-
8373
if (!FS.consumesDataArgument() &&
8474
FS.getConversionSpecifier().getKind() !=
8575
clang::analyze_format_string::ConversionSpecifier::PrintErrno)
8676
return true;
8777

88-
ArgData ArgD;
78+
ArgsData.emplace_back();
8979
unsigned ArgIndex = FS.getArgIndex();
9080
if (ArgIndex < Args.size())
91-
if (!CheckAndSetArgExpr(ArgIndex, ArgD.E))
92-
return true;
81+
ArgsData.back().E = Args[ArgIndex];
9382

9483
// First get the Kind
95-
ArgD.Kind = getKind(FS.getConversionSpecifier().getKind());
96-
if (ArgD.Kind != OSLogBufferItem::ErrnoKind && !ArgD.E) {
84+
ArgsData.back().Kind = getKind(FS.getConversionSpecifier().getKind());
85+
if (ArgsData.back().Kind != OSLogBufferItem::ErrnoKind &&
86+
!ArgsData.back().E) {
9787
// missing argument
88+
ArgsData.pop_back();
9889
return false;
9990
}
10091

@@ -106,11 +97,10 @@ class OSLogFormatStringHandler
10697
case clang::analyze_format_string::OptionalAmount::NotSpecified: // "%s"
10798
break;
10899
case clang::analyze_format_string::OptionalAmount::Constant: // "%.16s"
109-
ArgD.Size = precision.getConstantAmount();
100+
ArgsData.back().Size = precision.getConstantAmount();
110101
break;
111102
case clang::analyze_format_string::OptionalAmount::Arg: // "%.*s"
112-
if (!CheckAndSetArgExpr(precision.getArgIndex(), ArgD.Count))
113-
return true;
103+
ArgsData.back().Count = Args[precision.getArgIndex()];
114104
break;
115105
case clang::analyze_format_string::OptionalAmount::Invalid:
116106
return false;
@@ -123,11 +113,10 @@ class OSLogFormatStringHandler
123113
case clang::analyze_format_string::OptionalAmount::NotSpecified: // "%P"
124114
return false; // length must be supplied with pointer format specifier
125115
case clang::analyze_format_string::OptionalAmount::Constant: // "%.16P"
126-
ArgD.Size = precision.getConstantAmount();
116+
ArgsData.back().Size = precision.getConstantAmount();
127117
break;
128118
case clang::analyze_format_string::OptionalAmount::Arg: // "%.*P"
129-
if (!CheckAndSetArgExpr(precision.getArgIndex(), ArgD.Count))
130-
return true;
119+
ArgsData.back().Count = Args[precision.getArgIndex()];
131120
break;
132121
case clang::analyze_format_string::OptionalAmount::Invalid:
133122
return false;
@@ -136,27 +125,22 @@ class OSLogFormatStringHandler
136125
}
137126
default:
138127
if (FS.getPrecision().hasDataArgument()) {
139-
if (!CheckAndSetArgExpr(FS.getPrecision().getArgIndex(),
140-
ArgD.Precision))
141-
return true;
128+
ArgsData.back().Precision = Args[FS.getPrecision().getArgIndex()];
142129
}
143130
break;
144131
}
145132
if (FS.getFieldWidth().hasDataArgument()) {
146-
if (!CheckAndSetArgExpr(FS.getFieldWidth().getArgIndex(),
147-
ArgD.FieldWidth))
148-
return true;
133+
ArgsData.back().FieldWidth = Args[FS.getFieldWidth().getArgIndex()];
149134
}
150135

151136
if (FS.isSensitive())
152-
ArgD.Flags |= OSLogBufferItem::IsSensitive;
137+
ArgsData.back().Flags |= OSLogBufferItem::IsSensitive;
153138
else if (FS.isPrivate())
154-
ArgD.Flags |= OSLogBufferItem::IsPrivate;
139+
ArgsData.back().Flags |= OSLogBufferItem::IsPrivate;
155140
else if (FS.isPublic())
156-
ArgD.Flags |= OSLogBufferItem::IsPublic;
141+
ArgsData.back().Flags |= OSLogBufferItem::IsPublic;
157142

158-
ArgD.MaskType = FS.getMaskType();
159-
ArgsData.push_back(ArgD);
143+
ArgsData.back().MaskType = FS.getMaskType();
160144
return true;
161145
}
162146

‎clang/lib/Sema/SemaChecking.cpp‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7653,6 +7653,14 @@ void CheckPrintfHandler::handleInvalidMaskType(StringRef MaskType) {
76537653
S.Diag(getLocationOfByte(MaskType.data()), diag::err_invalid_mask_type_size);
76547654
}
76557655

7656+
// Error out if struct or complex type argments are passed to os_log.
7657+
static bool isInvalidOSLogArgTypeForCodeGen(FormatStringType FSType,
7658+
QualType T) {
7659+
if (FSType != FormatStringType::OSLog)
7660+
return false;
7661+
return T->isRecordType() || T->isComplexType();
7662+
}
7663+
76567664
bool CheckPrintfHandler::HandleAmount(
76577665
const analyze_format_string::OptionalAmount &Amt, unsigned k,
76587666
const char *startSpecifier, unsigned specifierLen) {
@@ -7685,11 +7693,14 @@ bool CheckPrintfHandler::HandleAmount(
76857693
assert(AT.isValid());
76867694

76877695
if (!AT.matchesType(S.Context, T)) {
7688-
EmitFormatDiagnostic(S.PDiag(diag::warn_printf_asterisk_wrong_type)
7689-
<< k << AT.getRepresentativeTypeName(S.Context)
7690-
<< T << Arg->getSourceRange(),
7696+
unsigned DiagID = isInvalidOSLogArgTypeForCodeGen(FSType, T)
7697+
? diag::err_printf_asterisk_wrong_type
7698+
: diag::warn_printf_asterisk_wrong_type;
7699+
EmitFormatDiagnostic(S.PDiag(DiagID)
7700+
<< k << AT.getRepresentativeTypeName(S.Context)
7701+
<< T << Arg->getSourceRange(),
76917702
getLocationOfByte(Amt.getStart()),
7692-
/*IsStringLocation*/true,
7703+
/*IsStringLocation*/ true,
76937704
getSpecifierRange(startSpecifier, specifierLen));
76947705
// Don't do any more checking. We will just emit
76957706
// spurious errors.
@@ -8744,7 +8755,9 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
87448755
Diag = diag::warn_format_conversion_argument_type_mismatch_confusion;
87458756
break;
87468757
case ArgType::NoMatch:
8747-
Diag = diag::warn_format_conversion_argument_type_mismatch;
8758+
Diag = isInvalidOSLogArgTypeForCodeGen(FSType, ExprTy)
8759+
? diag::err_format_conversion_argument_type_mismatch
8760+
: diag::warn_format_conversion_argument_type_mismatch;
87488761
break;
87498762
}
87508763

‎clang/test/CodeGenObjC/os_log.m‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,6 @@ + (id)m1;
1313

1414
C *c;
1515

16-
struct S {
17-
int a[4];
18-
};
19-
20-
struct S s;
21-
_Complex float cf;
22-
2316
@class NSString;
2417
extern __attribute__((visibility("default"))) NSString *GenString(void);
2518
void os_log_pack_send(void *);
@@ -130,19 +123,3 @@ void test_builtin_os_log5(void *buf) {
130123
__builtin_os_log_format(buf, "capabilities: %@", (0, GenString()));
131124
os_log_pack_send(buf);
132125
}
133-
134-
// CHECK-LABEL: define void @test_builtin_os_log6(
135-
// CHECK: call void @__os_log_helper_1_0_0(
136-
137-
void test_builtin_os_log6(void *buf) {
138-
__builtin_os_log_format(buf, "%.*s %.*P %*.*f", s, s, s, s, s, s, s);
139-
}
140-
141-
// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_1_0_0(
142-
143-
// CHECK-LABEL: define void @test_builtin_os_log7(
144-
// CHECK: call void @__os_log_helper_1_0_0(
145-
146-
void test_builtin_os_log7(void *buf) {
147-
__builtin_os_log_format(buf, "%.*s %.*P %*.*f", cf, cf, cf, cf, cf, cf, cf);
148-
}

‎clang/test/SemaObjC/os_log.m‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// RUN: %clang_cc1 -verify %s
2+
3+
struct S {
4+
int a[4];
5+
};
6+
7+
struct S s;
8+
_Complex float cf;
9+
10+
void test_builtin_os_log_invalid_arg(void *buf) {
11+
__builtin_os_log_format(buf, "%*.*f", s, 5, 1.3); // expected-error {{field width should have type 'int', but argument has type 'struct S'}}
12+
__builtin_os_log_format(buf, "%*.*f", 1, s, 1.3); // expected-error {{field precision should have type 'int', but argument has type 'struct S'}}
13+
__builtin_os_log_format(buf, "%*.*f", 1, 5, s); // expected-error {{format specifies type 'double' but the argument has type 'struct S'}}
14+
15+
__builtin_os_log_format(buf, "%*.*f", cf, 5, 1.3); // expected-error {{field width should have type 'int', but argument has type '_Complex float'}}
16+
__builtin_os_log_format(buf, "%*.*f", 1, cf, 1.3); // expected-error {{field precision should have type 'int', but argument has type '_Complex float'}}
17+
__builtin_os_log_format(buf, "%*.*f", 1, 5, cf); // expected-error {{format specifies type 'double' but the argument has type '_Complex float'}}
18+
19+
__builtin_os_log_format(buf, "%*.*f", (void *)0, 5, 1.3); // expected-warning {{field width should have type 'int', but argument has type 'void *'}}
20+
__builtin_os_log_format(buf, "%*.*f", 1, (void *)0, 1.3); // expected-warning {{field precision should have type 'int', but argument has type 'void *'}}
21+
__builtin_os_log_format(buf, "%*.*f", 1, 5, (void *)0); // expected-warning {{format specifies type 'double' but the argument has type 'void *'}}
22+
}

0 commit comments

Comments
 (0)