On Tue, Sep 26, 2017 at 11:10 AM, David Majnemer <david.majne...@gmail.com> wrote:
> Shouldn't you only loosen the check for things targeting the Windows SDK? > GNU platforms shouldn't need this. > That code path only actually happens for MS mode. `__va_start` is marked as `ALL_MS_LANGUAGES`. I suppoes that it would be better to rename the function to `SemaBuiltinVAStartARMMicrosoft`. Actually, I like this much better, I'll rename it. > On Tue, Sep 26, 2017 at 10:44 AM, Saleem Abdulrasool via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: compnerd >> Date: Tue Sep 26 10:44:10 2017 >> New Revision: 314226 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=314226&view=rev >> Log: >> Sema: Windows/ARM __va_start is not const correct >> >> The `__va_start` intrinsic for Windows ARM does not account for const >> correctness when performing a check. All local qualifiers are ignored >> when validating the invocation. This was exposed by building the swift >> stdlib against the Windows 10586 SDK for ARM. Simply expand out the >> check for the two parameters and ignore the qualifiers for the check. >> >> Modified: >> cfe/trunk/lib/Sema/SemaChecking.cpp >> cfe/trunk/test/SemaCXX/microsoft-varargs.cpp >> >> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaC >> hecking.cpp?rev=314226&r1=314225&r2=314226&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Sep 26 10:44:10 2017 >> @@ -3943,23 +3943,33 @@ bool Sema::SemaBuiltinVAStartARM(CallExp >> if (checkVAStartIsInVariadicFunction(*this, Func)) >> return true; >> >> - const struct { >> - unsigned ArgNo; >> - QualType Type; >> - } ArgumentTypes[] = { >> - { 1, Context.getPointerType(Context.CharTy.withConst()) }, >> - { 2, Context.getSizeType() }, >> - }; >> - >> - for (const auto &AT : ArgumentTypes) { >> - const Expr *Arg = Call->getArg(AT.ArgNo)->IgnoreParens(); >> - if (Arg->getType().getCanonicalType() == AT.Type.getCanonicalType()) >> - continue; >> - Diag(Arg->getLocStart(), diag::err_typecheck_convert_incompatible) >> - << Arg->getType() << AT.Type << 1 /* different class */ >> - << 0 /* qualifier difference */ << 3 /* parameter mismatch */ >> - << AT.ArgNo + 1 << Arg->getType() << AT.Type; >> - } >> + // __va_start on Windows does not validate the parameter qualifiers >> + >> + const Expr *Arg1 = Call->getArg(1)->IgnoreParens(); >> + const Type *Arg1Ty = Arg1->getType().getCanonicalType().getTypePtr(); >> + >> + const Expr *Arg2 = Call->getArg(2)->IgnoreParens(); >> + const Type *Arg2Ty = Arg2->getType().getCanonicalType().getTypePtr(); >> + >> + const QualType &ConstCharPtrTy = >> + Context.getPointerType(Context.CharTy.withConst()); >> + if (!Arg1Ty->isPointerType() || >> + Arg1Ty->getPointeeType().withoutLocalFastQualifiers() != >> Context.CharTy) >> + Diag(Arg1->getLocStart(), diag::err_typecheck_convert_incompatible) >> + << Arg1->getType() << ConstCharPtrTy >> + << 1 /* different class */ >> + << 0 /* qualifier difference */ >> + << 3 /* parameter mismatch */ >> + << 2 << Arg1->getType() << ConstCharPtrTy; >> + >> + const QualType SizeTy = Context.getSizeType(); >> + if (Arg2Ty->getCanonicalTypeInternal().withoutLocalFastQualifiers() >> != SizeTy) >> + Diag(Arg2->getLocStart(), diag::err_typecheck_convert_incompatible) >> + << Arg2->getType() << SizeTy >> + << 1 /* different class */ >> + << 0 /* qualifier difference */ >> + << 3 /* parameter mismatch */ >> + << 3 << Arg2->getType() << SizeTy; >> >> return false; >> } >> >> Modified: cfe/trunk/test/SemaCXX/microsoft-varargs.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ >> microsoft-varargs.cpp?rev=314226&r1=314225&r2=314226&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/test/SemaCXX/microsoft-varargs.cpp (original) >> +++ cfe/trunk/test/SemaCXX/microsoft-varargs.cpp Tue Sep 26 10:44:10 2017 >> @@ -20,3 +20,8 @@ int builtin(int i, ...) { >> return __builtin_va_arg(ap, int); >> } >> >> +void test___va_start_ignore_const(const char *format, ...) { >> + va_list args; >> + ((void)(__va_start(&args, (&const_cast<char &>(reinterpret_cast<const >> volatile char &>(format))), ((sizeof(format) + 4 - 1) & ~(4 - 1)), >> (&const_cast<char &>(reinterpret_cast<const volatile char &>(format)))))); >> +} >> + >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > > -- Saleem Abdulrasool compnerd (at) compnerd (dot) org
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
