Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang] Fix preprocessor output from #embed (#126742) #127222

Merged

Conversation

Fznamznon
Copy link
Contributor

When bytes with negative signed char values appear in the data, make sure to use raw bytes from the data string when preprocessing, not char values.

Fixes #102798

@Fznamznon Fznamznon added this to the LLVM 20.X Release milestone Feb 14, 2025
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Feb 14, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 14, 2025

@llvm/pr-subscribers-clang

Author: Mariya Podchishchaeva (Fznamznon)

Changes

When bytes with negative signed char values appear in the data, make sure to use raw bytes from the data string when preprocessing, not char values.

Fixes #102798


Full diff: https://github.com/llvm/llvm-project/pull/127222.diff

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/lib/Frontend/PrintPreprocessedOutput.cpp (+2-3)
  • (modified) clang/test/Preprocessor/embed_preprocess_to_file.c (+8)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ad1a5e7ae282e..08f8491e2928d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -897,6 +897,8 @@ Bug Fixes in This Version
 - No longer return ``false`` for ``noexcept`` expressions involving a
   ``delete`` which resolves to a destroying delete but the type of the object
   being deleted has a potentially throwing destructor (#GH118660).
+- Clang now outputs correct values when #embed data contains bytes with negative
+  signed char values (#GH102798).
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Frontend/PrintPreprocessedOutput.cpp b/clang/lib/Frontend/PrintPreprocessedOutput.cpp
index 1005825441b3e..2ae355fb33885 100644
--- a/clang/lib/Frontend/PrintPreprocessedOutput.cpp
+++ b/clang/lib/Frontend/PrintPreprocessedOutput.cpp
@@ -974,11 +974,10 @@ static void PrintPreprocessedTokens(Preprocessor &PP, Token &Tok,
       // Loop over the contents and print them as a comma-delimited list of
       // values.
       bool PrintComma = false;
-      for (auto Iter = Data->BinaryData.begin(), End = Data->BinaryData.end();
-           Iter != End; ++Iter) {
+      for (unsigned char Byte : Data->BinaryData.bytes()) {
         if (PrintComma)
           *Callbacks->OS << ", ";
-        *Callbacks->OS << static_cast<unsigned>(*Iter);
+        *Callbacks->OS << static_cast<int>(Byte);
         PrintComma = true;
       }
     } else if (Tok.isAnnotation()) {
diff --git a/clang/test/Preprocessor/embed_preprocess_to_file.c b/clang/test/Preprocessor/embed_preprocess_to_file.c
index 9895d958cf96d..b3c99d36f784a 100644
--- a/clang/test/Preprocessor/embed_preprocess_to_file.c
+++ b/clang/test/Preprocessor/embed_preprocess_to_file.c
@@ -37,3 +37,11 @@ const char even_more[] = {
 // DIRECTIVE-NEXT: #embed <jk.txt> prefix(4, 5,) suffix(, 6, 7) /* clang -E -dE */
 // DIRECTIVE-NEXT:  , 8, 9, 10
 // DIRECTIVE-NEXT: };
+
+constexpr char big_one[] = {
+#embed <big_char.txt>
+};
+
+// EXPANDED: constexpr char big_one[] = {255
+// DIRECTIVE: constexpr char big_one[] = {
+// DIRECTIVE-NEXT: #embed <big_char.txt>

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

When bytes with negative signed char values appear in the data, make
sure to use raw bytes from the data string when preprocessing, not char
values.

Fixes llvm#102798
@tstellar tstellar force-pushed the cherry-pick-gh102798-to-clang20 branch from d7e55d9 to 8e06e0e Compare February 21, 2025 18:39
@tstellar tstellar merged commit 8e06e0e into llvm:release/20.x Feb 21, 2025
9 of 12 checks passed
Copy link

@Fznamznon (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

4 participants