[libcxx-commits] [libcxx] a4ba780 - [libc++] Enable -Wformat-nonliteral when building libc++

[Louis Dionne via libcxx-commits]

Author: Louis Dionne
Date: 2021-11-09T13:17:45-05:00
New Revision: a4ba780510518150cc11b330bbd2beb447e1f50e

URL: https://github.com/llvm/llvm-project/commit/a4ba780510518150cc11b330bbd2beb447e1f50e
DIFF: https://github.com/llvm/llvm-project/commit/a4ba780510518150cc11b330bbd2beb447e1f50e.diff

LOG: [libc++] Enable -Wformat-nonliteral when building libc++

Using user-provided data as a format string is a well known source of
security vulnerabilities. For this reason, it is a good idea to compile
our code with -Wformat-nonliteral, which basically warns if a non-constant
string is used as a format specifier. This is the compiler’s best signal
that a format string call may be insecure.

I audited the code after adding the warning and made sure that the few
places where we used a non-literal string as a format string were not
potential security issues. I either disabled the warning locally for
those instances or fixed the warning by using a literal. The idea is
that after we add the warning to the build, any new use of a non-literal
string in a format string will trigger a diagnostic, and we can either
get rid of it or disable the warning locally, which is a way of
acknowledging that it has been audited.

I also looked into enabling it in the test suite, which would perhaps
allow finding additional instances of it in our headers, however that
is not possible at the moment because Clang doesn't support putting
__attribute__((__format__(...))) on variadic templates, which would
be needed.

rdar://84571685

Differential Revision: https://reviews.llvm.org/D112927

Added: 
    

Modified: 
    libcxx/CMakeLists.txt
    libcxx/include/locale

Removed: 
    


################################################################################
diff  --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index e7edf2429e11..657714fb0094 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -614,7 +614,8 @@ function(cxx_add_warning_flags target)
   endif()
   target_add_compile_flags_if_supported(${target} PRIVATE -Wextra -W -Wwrite-strings
                                                           -Wno-unused-parameter -Wno-long-long
-                                                          -Werror=return-type -Wextra-semi -Wundef)
+                                                          -Werror=return-type -Wextra-semi -Wundef
+                                                          -Wformat-nonliteral)
   if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
     target_add_compile_flags_if_supported(${target} PRIVATE
       -Wno-user-defined-literals

diff  --git a/libcxx/include/locale b/libcxx/include/locale
index 2d37521de9a8..478ca1ea06c0 100644
--- a/libcxx/include/locale
+++ b/libcxx/include/locale
@@ -1486,7 +1486,10 @@ num_put<_CharT, _OutputIterator>::__do_put_integral(iter_type __s, ios_base& __i
         + ((numeric_limits<_Unsigned>::digits % 3) != 0) // round up
         + 2; // base prefix + terminating null character
     char __nar[__nbuf];
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wformat-nonliteral"
     int __nc = __libcpp_snprintf_l(__nar, sizeof(__nar), _LIBCPP_GET_C_LOCALE, __fmt, __v);
+#pragma clang diagnostic pop
     char* __ne = __nar + __nc;
     char* __np = this->__identify_padding(__nar, __ne, __iob);
     // Stage 2 - Widen __nar while adding thousands separators
@@ -1546,6 +1549,8 @@ num_put<_CharT, _OutputIterator>::__do_put_floating_point(iter_type __s, ios_bas
     char __nar[__nbuf];
     char* __nb = __nar;
     int __nc;
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wformat-nonliteral"
     if (__specify_precision)
         __nc = __libcpp_snprintf_l(__nb, __nbuf, _LIBCPP_GET_C_LOCALE, __fmt,
                                    (int)__iob.precision(), __v);
@@ -1562,6 +1567,7 @@ num_put<_CharT, _OutputIterator>::__do_put_floating_point(iter_type __s, ios_bas
             __throw_bad_alloc();
         __nbh.reset(__nb);
     }
+#pragma clang diagnostic pop
     char* __ne = __nb + __nc;
     char* __np = this->__identify_padding(__nb, __ne, __iob);
     // Stage 2 - Widen __nar while adding thousands separators
@@ -1606,10 +1612,9 @@ num_put<_CharT, _OutputIterator>::do_put(iter_type __s, ios_base& __iob,
                                          char_type __fl, const void* __v) const
 {
     // Stage 1 - Get pointer in narrow char
-    char __fmt[6] = "%p";
     const unsigned __nbuf = 20;
     char __nar[__nbuf];
-    int __nc = __libcpp_snprintf_l(__nar, sizeof(__nar), _LIBCPP_GET_C_LOCALE, __fmt, __v);
+    int __nc = __libcpp_snprintf_l(__nar, sizeof(__nar), _LIBCPP_GET_C_LOCALE, "%p", __v);
     char* __ne = __nar + __nc;
     char* __np = this->__identify_padding(__nar, __ne, __iob);
     // Stage 2 - Widen __nar