[libc-commits] [PATCH] D80178: [libc] Fix accidental inclusion of system libc headers.

[Paula Toth via Phabricator via libc-commits]

PaulkaToast created this revision.
PaulkaToast added reviewers: abrachet, sivachandra.
PaulkaToast added a project: libc-project.
Herald added subscribers: libc-commits, ecnelises, tschuett, aheejin, mgorny.

I found that because `--system-headers` flag was not included when running clang-tidy, errors produced from compiler provided headers were being suppressed. After passing this flag I realized that by including headers like stdint.h we were indirectly including headers from the system libc. See below example of `memcpy` compiled with `-H`. To prevent this we set the preprocessor flag `__STDC_HOSTED__` to false. See comment in `common.h.def` for more details

  /home/paulatoth/workspace/llvm-project/libc/src/string/memcpy.cpp
  . /home/paulatoth/workspace/llvm-project/libc/src/string/memcpy.h
  .. projects/libc/include/string.h
  ... projects/libc/include/__llvm-libc-common.h
  ... /usr/lib/llvm-9/lib/clang/9.0.1/include/stddef.h
  ... /usr/lib/llvm-9/lib/clang/9.0.1/include/stddef.h
  .. /usr/lib/llvm-9/lib/clang/9.0.1/include/stddef.h
  ... /usr/lib/llvm-9/lib/clang/9.0.1/include/__stddef_max_align_t.h
  . projects/libc/src/__support/common.h
  . projects/libc/src/string/memcpy_arch_specific.h
  .. /home/paulatoth/workspace/llvm-project/libc/src/string/memory_utils/memcpy_utils.h
  ... /home/paulatoth/workspace/llvm-project/libc/src/string/memory_utils/utils.h
  .... projects/libc/src/string/memory_utils/cacheline_size.h
  .... /usr/lib/llvm-9/lib/clang/9.0.1/include/stddef.h
  .... /usr/lib/llvm-9/lib/clang/9.0.1/include/stdint.h
  ..... /usr/include/stdint.h
  ...... /usr/include/x86_64-linux-gnu/bits/libc-header-start.h
  ....... /usr/include/features.h
  ........ /usr/include/stdc-predef.h
  ........ /usr/include/x86_64-linux-gnu/sys/cdefs.h
  ......... /usr/include/x86_64-linux-gnu/bits/wordsize.h
  ......... /usr/include/x86_64-linux-gnu/bits/long-double.h
  ........ /usr/include/x86_64-linux-gnu/gnu/stubs.h
  ......... /usr/include/x86_64-linux-gnu/gnu/stubs-64.h
  ...... /usr/include/x86_64-linux-gnu/bits/types.h
  ....... /usr/include/x86_64-linux-gnu/bits/wordsize.h
  ....... /usr/include/x86_64-linux-gnu/bits/timesize.h
  ....... /usr/include/x86_64-linux-gnu/bits/typesizes.h
  ....... /usr/include/x86_64-linux-gnu/bits/time64.h
  ...... /usr/include/x86_64-linux-gnu/bits/wchar.h
  ...... /usr/include/x86_64-linux-gnu/bits/wordsize.h
  ...... /usr/include/x86_64-linux-gnu/bits/stdint-intn.h
  ...... /usr/include/x86_64-linux-gnu/bits/stdint-uintn.h
  ... /usr/lib/llvm-9/lib/clang/9.0.1/include/stddef.h


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80178

Files:
  libc/cmake/modules/LLVMLibCObjectRules.cmake
  libc/src/.clang-tidy
  libc/src/__support/common.h.def


Index: libc/src/__support/common.h.def
===================================================================
--- libc/src/__support/common.h.def
+++ libc/src/__support/common.h.def
@@ -15,6 +15,14 @@
 #define unlikely(x) __builtin_expect (x, 0)
 #define UNUSED __attribute__((unused))
 
+// This flag is used to indicate to the compiler whether it has access
+// to a libc. It is used in compiler provided headers like stdint.h
+// which contain an #include_next directive which extends the system header
+// instead of replacing it. Therefore we want this to be set to false to prevent
+// accidental inclusion of system headers and ensure use of stand alone headers. 
+#undef __STDC_HOSTED__
+#define __STDC_HOSTED__ 0
+
 <!> Include the platform specific definitions at build time. For example, that
 <!> of entrypoint macro.
 %%include_file(${platform_defs})
Index: libc/src/.clang-tidy
===================================================================
--- libc/src/.clang-tidy
+++ libc/src/.clang-tidy
@@ -3,4 +3,4 @@
 WarningsAsErrors: 'llvmlibc-*'
 CheckOptions:
   - key:             llvmlibc-restrict-system-libc-headers.Includes
-    value:           '-*, linux/*, asm/unistd.h'
+    value:           '-*, linux/*, asm/*.h, asm-generic/*.h'
Index: libc/cmake/modules/LLVMLibCObjectRules.cmake
===================================================================
--- libc/cmake/modules/LLVMLibCObjectRules.cmake
+++ libc/cmake/modules/LLVMLibCObjectRules.cmake
@@ -217,7 +217,8 @@
       #     X warnings generated.
       # Until this is fixed upstream, we use -fno-caret-diagnostics to surpress
       # these.
-      COMMAND $<TARGET_FILE:clang-tidy> "--extra-arg=-fno-caret-diagnostics" --quiet
+      COMMAND $<TARGET_FILE:clang-tidy> --system-headers
+              "--extra-arg=-fno-caret-diagnostics" --quiet
               # Path to directory containing compile_commands.json
               -p ${PROJECT_BINARY_DIR}
               ${ADD_ENTRYPOINT_OBJ_SRCS}


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80178.264783.patch
Type: text/x-patch
Size: 1975 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libc-commits/attachments/20200519/95a2e96b/attachment.bin>