[ClangScanDeps] Avoid use-of-uninitialized-memory for end-of-directive edge case#188590
[ClangScanDeps] Avoid use-of-uninitialized-memory for end-of-directive edge case#188590
Conversation
…e edge case llvm#186966 was reverted because the test case triggered a use-of-uninitialized-memory (https://lab.llvm.org/buildbot/#/builders/94/builds/16379), due to the include directive did not have a trailing newline. This patch adds a minor fix to avoid the use-of-uninitialized-memory, and deliberately re-adds the test case sans trailing newline for regression testing. MSan report prior to this fix: ``` @@@BUILD_STEP sanitizer logs: stage2/msan_track_origins check@@@ ==clang-scan-deps==616960==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x5555599c3300 in isAnnotation /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/include/clang/Lex/Token.h:131:38 #1 0x5555599c3300 in setLength /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/include/clang/Lex/Token.h:152:13 llvm#2 0x5555599c3300 in clang::Lexer::FormTokenWithChars(clang::Token&, char const*, clang::tok::TokenKind) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/include/clang/Lex/Lexer.h:644:12 llvm#3 0x5555599cf895 in clang::Lexer::LexEndOfFile(clang::Token&, char const*) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/Lexer.cpp:3166:5 llvm#4 0x555559bb229b in clang::Preprocessor::Lex(clang::Token&) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/Preprocessor.cpp:916:11 llvm#5 0x555559aa5365 in __invoke<void (clang::Preprocessor::*&)(clang::Token &), clang::Preprocessor *, clang::Token &> /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/libcxx_install_msan_track_origins/include/c++/v1/__type_traits/invoke.h:90:27 llvm#6 0x555559aa5365 in invoke<void (clang::Preprocessor::*&)(clang::Token &), clang::Preprocessor *, clang::Token &> /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/libcxx_install_msan_track_origins/include/c++/v1/__functional/invoke.h:29:10 llvm#7 0x555559aa5365 in operator()<void (clang::Preprocessor::*)(clang::Token &)> /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/PPDirectives.cpp:470:5 llvm#8 0x555559aa5365 in clang::Preprocessor::CheckEndOfDirective(llvm::StringRef, bool, llvm::SmallVectorImpl<clang::Token>*) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/PPDirectives.cpp:478:5 llvm#9 0x555559ab96b5 in clang::Preprocessor::HandleIncludeDirective(clang::SourceLocation, clang::Token&, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/PPDirectives.cpp:2205:7 ```
|
@llvm/pr-subscribers-clang Author: Thurston Dang (thurstond) Changes#186966 was reverted because the test case triggered a use-of-uninitialized-memory (https://lab.llvm.org/buildbot/#/builders/94/builds/16379), due to the include directive omitting a trailing newline. This patch adds a minor fix to avoid the use-of-uninitialized-memory, and deliberately re-adds the test case sans trailing newline for regression testing. MSan report prior to this fix: Full diff: https://github.com/llvm/llvm-project/pull/188590.diff 2 Files Affected:
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index c89402fa137c0..1a07ac3a059b2 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -466,6 +466,9 @@ SourceLocation
Preprocessor::CheckEndOfDirective(StringRef DirType, bool EnableMacros,
SmallVectorImpl<Token> *ExtraToks) {
Token Tmp;
+ // Avoid use-of-uninitialized-memory for edge case(s) where there is no extra
+ // token to be parsed.
+ Tmp.setKind(tok::eod);
auto ReadNextTok = [this, ExtraToks, &Tmp](auto &&LexFn) {
std::invoke(LexFn, this, Tmp);
if (ExtraToks && Tmp.isNot(tok::eod))
diff --git a/clang/test/ClangScanDeps/p1689-suppress-warnings-no-eol.cppm b/clang/test/ClangScanDeps/p1689-suppress-warnings-no-eol.cppm
new file mode 100644
index 0000000000000..79964100b20ad
--- /dev/null
+++ b/clang/test/ClangScanDeps/p1689-suppress-warnings-no-eol.cppm
@@ -0,0 +1,25 @@
+// Test that there is no use-of-uninitialized memory when parsing '#include' in
+// the last line, without a newline.
+//
+// Forked from p1689-suppress-warnings.cppm.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// Test P1689 format - should NOT emit warnings
+// RUN: clang-scan-deps -format=p1689 -- %clang++ -std=c++20 -I%t -c %t/mylib.cppm -o %t/mylib.o 2>&1 | FileCheck %s
+
+// CHECK-NOT: warning:
+// CHECK: {
+// CHECK: "revision"
+
+//--- header.h
+// Empty header for testing
+
+//--- mylib.cppm
+module;
+
+export module mylib;
+
+#include <header.h>
\ No newline at end of file
|
|
@yronglin not sure if this relates to your patch, but I know you know preprocessor a lot than me. |
@yronglin Thanks for the review :-) |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/141/builds/16698 Here is the relevant piece of the build log for the reference |
…e edge case (llvm#188590) llvm#186966 was reverted because the test case triggered a use-of-uninitialized-memory (https://lab.llvm.org/buildbot/#/builders/94/builds/16379), due to the include directive omitting a trailing newline. This patch adds a minor fix to avoid the use-of-uninitialized-memory, and deliberately re-adds the test case sans trailing newline for regression testing. MSan report prior to this fix: ``` @@@BUILD_STEP sanitizer logs: stage2/msan_track_origins check@@@ ==clang-scan-deps==616960==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x5555599c3300 in isAnnotation /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/include/clang/Lex/Token.h:131:38 llvm#1 0x5555599c3300 in setLength /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/include/clang/Lex/Token.h:152:13 llvm#2 0x5555599c3300 in clang::Lexer::FormTokenWithChars(clang::Token&, char const*, clang::tok::TokenKind) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/include/clang/Lex/Lexer.h:644:12 llvm#3 0x5555599cf895 in clang::Lexer::LexEndOfFile(clang::Token&, char const*) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/Lexer.cpp:3166:5 llvm#4 0x555559bb229b in clang::Preprocessor::Lex(clang::Token&) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/Preprocessor.cpp:916:11 llvm#5 0x555559aa5365 in __invoke<void (clang::Preprocessor::*&)(clang::Token &), clang::Preprocessor *, clang::Token &> /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/libcxx_install_msan_track_origins/include/c++/v1/__type_traits/invoke.h:90:27 llvm#6 0x555559aa5365 in invoke<void (clang::Preprocessor::*&)(clang::Token &), clang::Preprocessor *, clang::Token &> /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/libcxx_install_msan_track_origins/include/c++/v1/__functional/invoke.h:29:10 llvm#7 0x555559aa5365 in operator()<void (clang::Preprocessor::*)(clang::Token &)> /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/PPDirectives.cpp:470:5 llvm#8 0x555559aa5365 in clang::Preprocessor::CheckEndOfDirective(llvm::StringRef, bool, llvm::SmallVectorImpl<clang::Token>*) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/PPDirectives.cpp:478:5 llvm#9 0x555559ab96b5 in clang::Preprocessor::HandleIncludeDirective(clang::SourceLocation, clang::Token&, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/PPDirectives.cpp:2205:7 ... ```
…e edge case (llvm#188590) llvm#186966 was reverted because the test case triggered a use-of-uninitialized-memory (https://lab.llvm.org/buildbot/#/builders/94/builds/16379), due to the include directive omitting a trailing newline. This patch adds a minor fix to avoid the use-of-uninitialized-memory, and deliberately re-adds the test case sans trailing newline for regression testing. MSan report prior to this fix: ``` @@@BUILD_STEP sanitizer logs: stage2/msan_track_origins check@@@ ==clang-scan-deps==616960==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x5555599c3300 in isAnnotation /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/include/clang/Lex/Token.h:131:38 llvm#1 0x5555599c3300 in setLength /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/include/clang/Lex/Token.h:152:13 llvm#2 0x5555599c3300 in clang::Lexer::FormTokenWithChars(clang::Token&, char const*, clang::tok::TokenKind) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/include/clang/Lex/Lexer.h:644:12 llvm#3 0x5555599cf895 in clang::Lexer::LexEndOfFile(clang::Token&, char const*) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/Lexer.cpp:3166:5 llvm#4 0x555559bb229b in clang::Preprocessor::Lex(clang::Token&) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/Preprocessor.cpp:916:11 llvm#5 0x555559aa5365 in __invoke<void (clang::Preprocessor::*&)(clang::Token &), clang::Preprocessor *, clang::Token &> /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/libcxx_install_msan_track_origins/include/c++/v1/__type_traits/invoke.h:90:27 llvm#6 0x555559aa5365 in invoke<void (clang::Preprocessor::*&)(clang::Token &), clang::Preprocessor *, clang::Token &> /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/libcxx_install_msan_track_origins/include/c++/v1/__functional/invoke.h:29:10 llvm#7 0x555559aa5365 in operator()<void (clang::Preprocessor::*)(clang::Token &)> /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/PPDirectives.cpp:470:5 llvm#8 0x555559aa5365 in clang::Preprocessor::CheckEndOfDirective(llvm::StringRef, bool, llvm::SmallVectorImpl<clang::Token>*) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/PPDirectives.cpp:478:5 llvm#9 0x555559ab96b5 in clang::Preprocessor::HandleIncludeDirective(clang::SourceLocation, clang::Token&, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/PPDirectives.cpp:2205:7 ... ```
…e edge case (llvm#188590) llvm#186966 was reverted because the test case triggered a use-of-uninitialized-memory (https://lab.llvm.org/buildbot/#/builders/94/builds/16379), due to the include directive omitting a trailing newline. This patch adds a minor fix to avoid the use-of-uninitialized-memory, and deliberately re-adds the test case sans trailing newline for regression testing. MSan report prior to this fix: ``` @@@BUILD_STEP sanitizer logs: stage2/msan_track_origins check@@@ ==clang-scan-deps==616960==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x5555599c3300 in isAnnotation /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/include/clang/Lex/Token.h:131:38 llvm#1 0x5555599c3300 in setLength /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/include/clang/Lex/Token.h:152:13 llvm#2 0x5555599c3300 in clang::Lexer::FormTokenWithChars(clang::Token&, char const*, clang::tok::TokenKind) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/include/clang/Lex/Lexer.h:644:12 llvm#3 0x5555599cf895 in clang::Lexer::LexEndOfFile(clang::Token&, char const*) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/Lexer.cpp:3166:5 llvm#4 0x555559bb229b in clang::Preprocessor::Lex(clang::Token&) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/Preprocessor.cpp:916:11 llvm#5 0x555559aa5365 in __invoke<void (clang::Preprocessor::*&)(clang::Token &), clang::Preprocessor *, clang::Token &> /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/libcxx_install_msan_track_origins/include/c++/v1/__type_traits/invoke.h:90:27 llvm#6 0x555559aa5365 in invoke<void (clang::Preprocessor::*&)(clang::Token &), clang::Preprocessor *, clang::Token &> /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/libcxx_install_msan_track_origins/include/c++/v1/__functional/invoke.h:29:10 llvm#7 0x555559aa5365 in operator()<void (clang::Preprocessor::*)(clang::Token &)> /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/PPDirectives.cpp:470:5 llvm#8 0x555559aa5365 in clang::Preprocessor::CheckEndOfDirective(llvm::StringRef, bool, llvm::SmallVectorImpl<clang::Token>*) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/PPDirectives.cpp:478:5 llvm#9 0x555559ab96b5 in clang::Preprocessor::HandleIncludeDirective(clang::SourceLocation, clang::Token&, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*) /home/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Lex/PPDirectives.cpp:2205:7 ... ```
#186966 was reverted because the test case triggered a use-of-uninitialized-memory (https://lab.llvm.org/buildbot/#/builders/94/builds/16379), due to the include directive omitting a trailing newline. This patch adds a minor fix to avoid the use-of-uninitialized-memory, and deliberately re-adds the test case sans trailing newline for regression testing.
MSan report prior to this fix: