[PATCH] D25266: Add a static_assert to enforce that parameters to llvm::format() are not totally unsafe

[Justin Bogner via llvm-commits]

Since the attribute approach is tricky this patch seems like a simple
and obvious enough thing to do for now. LGTM and if we come up with
something better later we can always reevaluate.

Mehdi Amini <mehdi.amini at apple.com> writes:
> That’s a good idea, unfortunately: "error: format attribute requires
> variadic function”.
>
> Also since we end up calling snprintf, I wonder why we don’t already
> catch these there, it is correctly decorated with the attribute on
> Darwin. Issue with variadic template (or template alone)? I should try
> a minimal test case.
>
>  
>> On Oct 5, 2016, at 9:35 AM, Zachary Turner <zturner at google.com> wrote:
>> 
>> Can't you add __attribute__((format(printf))); to the constructor?  That would catch these issues.
>> 
>> On Wed, Oct 5, 2016 at 9:05 AM Mehdi AMINI via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>> mehdi_amini added a comment.
>> 
>> This is also catching existing latent bugs in the codebase:
>> 
>>   In file included from /llvm-project/llvm/lib/Target/PowerPC/PPCVSXSwapRemoval.cpp:55:
>>   /llvm-project/llvm/include/llvm/Support/Format.h:82:3: error: static_assert failed "format can't be used with non fundamental / non pointer type"
>>     static_assert(std::is_scalar<Arg>::value,
>>     ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~
>>   /llvm-project/llvm/include/llvm/Support/Format.h:105:5: note: in instantiation of template class 'llvm::validate_format_parameters<llvm::StringRef>' requested here
>>       validate_format_parameters<Ts...>();
>>       ^
>>   /llvm-project/llvm/include/llvm/Support/Format.h:124:10: note: in instantiation of member function 'llvm::format_object<llvm::StringRef>::format_object' requested here
>>     return format_object<Ts...>(Fmt, Vals...);
>>            ^
>>   /llvm-project/llvm/lib/Target/PowerPC/PPCVSXSwapRemoval.cpp:965:21: note: in instantiation of function template specialization 'llvm::format<llvm::StringRef>' requested here
>>       DEBUG(dbgs() << format("  %14s  ", TII->getName(MI->getOpcode())));
>> 
>> 
>> https://reviews.llvm.org/D25266 <https://reviews.llvm.org/D25266>
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>