r314226 - Sema: Windows/ARM __va_start is not const correct

[Saleem Abdulrasool via cfe-commits]

On Tue, Sep 26, 2017 at 11:10 AM, David Majnemer <david.majnemer at 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 at 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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>


-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170926/f3387711/attachment.html>