This is an archive of the discontinued LLVM Phabricator instance.

Add a static_assert to enforce that parameters to llvm::format() are not totally unsafe
ClosedPublic

Authored by mehdi_amini on Oct 4 2016, 11:05 PM.

Details

Summary

I had for the second time today a bug where llvm::format("%s", Str)
was called with Str being a StringRef. The Linux and MacOS bots were
fine, but windows having different calling convention, it printed
garbage.

Instead we can catch this at compile-time: it is never expected to
call a C vararg printf-like function with non scalar type I believe.

Diff Detail

Repository
rL LLVM

Event Timeline

mehdi_amini retitled this revision from to Add a static_assert to enforce that parameters to llvm::format() are not totally unsafe.
mehdi_amini updated this object.
mehdi_amini added a subscriber: llvm-commits.

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())));
mehdi_amini updated this revision to Diff 73722.Oct 5 2016, 8:45 PM

Fix the build failure found by this assert

This revision was automatically updated to reflect the committed changes.