You're asking for the canonical type, which is looking through typedefs. I don't think that's what we want to do. What sort of segfaults did you get when using the decayed type?
Jordan On Mar 5, 2014, at 13:19 , Zach Davis <[email protected]> wrote: > Thanks for the comments. > > I have made the style changes and re-arranged the checks so wide > char types are also included. > > However, when I changed > > + QualType QT = Ctx.getCanonicalParamType(ArgQT) > > to getDecayedType, I got a number of seg faults. So I have left > it for now as GetCanonicalParamType. Along this same line, the > patch does cause an existing fixit test to fail, namely: > > size_t size_t_var; > scanf("%f", size_t_var); > ^- > %ul > > The test expects "%zu", not "%ul". This seems to be related to > the type of decay we do, but I don't know where to go from there. > > Zach > > On Wed, Mar 5, 2014 at 11:50 AM, Jordan Rose <[email protected]> wrote: >> Good idea! I have a few comments on the patch itself, but the general >> implementation seems sound. >> >> + QualType QT = Ctx.getCanonicalParamType(ArgQT); >> >> This probably doesn't need to be canonical; just "getDecayedType" should be >> good enough. >> >> >> + 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) >> >> Three notes: >> >> - This would more idiomatically be spelled "else if (const ConstantArrayType >> *CAT = ...)". >> - Please use complete sentences and punctuation for comments in the LLVM >> code. >> - Is there any reason the same logic won't work with wide string buffers? >> >> >> + bool success = fixedFS.fixType(Ex->IgnoreImpCasts()->getType(), >> S.getLangOpts(), >> >> Please reflow the arguments to stay within 80 columns. >> >> Thanks for working on this! >> Jordan >> >> >> On Mar 4, 2014, at 21:07 , Zach Davis <[email protected]> wrote: >> >>> 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. >>> <scanf_fixit.patch>_______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > <scanf_fixit2.patch> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
