Background: Bug 18412 suggests that the compiler should issue a
security warning when a scanf %s format specifier does not include a
field width. This is the second of 3 patches working toward this
(first was r202114).
This patch updates the fixit system to suggest a field width for %s
specifiers when the length of the target array is a know fixed size.
Example:
char a[10];
scanf("%s", a);
^-
%9s
In order to determine the array length, the fixType function needs to
know the complete type of the argument, otherwise it is just the raw
pointer type that we can't reason about.
Index: lib/Analysis/ScanfFormatString.cpp
===================================================================
--- lib/Analysis/ScanfFormatString.cpp (revision 202902)
+++ lib/Analysis/ScanfFormatString.cpp (working copy)
@@ -379,15 +379,19 @@
return ArgType();
}
-bool ScanfSpecifier::fixType(QualType QT, const LangOptions &LangOpt,
+bool ScanfSpecifier::fixType(QualType ArgQT, const LangOptions &LangOpt,
ASTContext &Ctx) {
- if (!QT->isPointerType())
- return false;
// %n is different from other conversion specifiers; don't try to fix it.
if (CS.getKind() == ConversionSpecifier::nArg)
return false;
+ if (!ArgQT->isPointerType()) {
+ if (!ArgQT->canDecayToPointerType())
+ return false;
+ }
+
+ QualType QT = Ctx.getCanonicalParamType(ArgQT);
QualType PT = QT->getPointeeType();
// If it's an enum, get its underlying type.
@@ -403,8 +407,17 @@
CS.setKind(ConversionSpecifier::sArg);
if (PT->isWideCharType())
LM.setKind(LengthModifier::AsWideChar);
- else
+ else {
+ // if we can determine the array length,
+ // we should include it as a field width
+ const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(ArgQT);
+ if (CAT && CAT->getSizeModifier() == ArrayType::Normal)
+ FieldWidth = OptionalAmount(OptionalAmount::Constant,
+ CAT->getSize().getZExtValue() - 1,
+ "", 0, false);
+
LM.setKind(LengthModifier::None);
+ }
return true;
}
Index: test/Sema/format-strings-fixit.c
===================================================================
--- test/Sema/format-strings-fixit.c (revision 202902)
+++ test/Sema/format-strings-fixit.c (working copy)
@@ -206,7 +207,7 @@
// CHECK: printf("%La", (long double) 42);
// CHECK: printf("%LA", (long double) 42);
-// CHECK: scanf("%s", str);
+// CHECK: scanf("%99s", str);
// CHECK: scanf("%hd", &shortVar);
// CHECK: scanf("%hu", &uShortVar);
// CHECK: scanf("%d", &intVar);
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp (revision 202902)
+++ lib/Sema/SemaChecking.cpp (working copy)
@@ -3496,7 +3503,7 @@
const analyze_format_string::ArgType &AT = FS.getArgType(S.Context);
if (AT.isValid() && !AT.matchesType(S.Context, Ex->getType())) {
ScanfSpecifier fixedFS = FS;
- bool success = fixedFS.fixType(Ex->getType(), S.getLangOpts(),
+ bool success = fixedFS.fixType(Ex->IgnoreImpCasts()->getType(), S.getLangOpts(),
S.Context);
if (success) {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits