Skip to content

Conversation

DavidSpickett
Copy link
Collaborator

The behaviour of strncmp is undefined if either string pointer is null (https://en.cppreference.com/w/cpp/string/byte/strncmp.html).

I've copied the logic over from Compare to another CharBlock, which had code to avoid UB in memcmp.

The test Preprocessing/kind-suffix.F90 was failing with UBSAN enabled, and now passes.

The behaviour of strncmp is undefined if either string pointer
is null (https://en.cppreference.com/w/cpp/string/byte/strncmp.html).

I've copied the logic over from Compare to another CharBlock,
which had code to avoid UB in memcmp.

The test Preprocessing/kind-suffix.F90 was failing
with UBSAN enabled, and now passes.
@DavidSpickett DavidSpickett changed the title [flang] Avoid UB in CharBlock::Compare to C string [flang] Avoid UB in CharBlock Compare to C string Jul 7, 2025
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:parser labels Jul 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 7, 2025

@llvm/pr-subscribers-flang-parser

Author: David Spickett (DavidSpickett)

Changes

The behaviour of strncmp is undefined if either string pointer is null (https://en.cppreference.com/w/cpp/string/byte/strncmp.html).

I've copied the logic over from Compare to another CharBlock, which had code to avoid UB in memcmp.

The test Preprocessing/kind-suffix.F90 was failing with UBSAN enabled, and now passes.


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

1 Files Affected:

  • (modified) flang/include/flang/Parser/char-block.h (+6-1)
diff --git a/flang/include/flang/Parser/char-block.h b/flang/include/flang/Parser/char-block.h
index 38f4f7b82e1ea..b3b1f04034d3c 100644
--- a/flang/include/flang/Parser/char-block.h
+++ b/flang/include/flang/Parser/char-block.h
@@ -150,7 +150,12 @@ class CharBlock {
 
   int Compare(const char *that) const {
     std::size_t bytes{size()};
-    if (int cmp{std::strncmp(begin(), that, bytes)}) {
+    // strncmp is undefined if either pointer is null.
+    if (!bytes) {
+      return that == nullptr ? 0 : -1;
+    } else if (!that) {
+      return 1;
+    } else if (int cmp{std::strncmp(begin(), that, bytes)}) {
       return cmp;
     }
     return that[bytes] == '\0' ? 0 : -1;

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

Thanks for the fix

@DavidSpickett DavidSpickett merged commit d889a74 into llvm:main Jul 8, 2025
12 checks passed
@DavidSpickett DavidSpickett deleted the flang-strncmp branch July 8, 2025 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:parser flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants