[clang] [clang-tools-extra] [CLANGD] Do not crash on designator initialization of union (PR #83369)

via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 15 15:44:57 PDT 2024


https://github.com/alirezamoshtaghi updated https://github.com/llvm/llvm-project/pull/83369

>From 3d6afe011221ac239bb668b375ed3f6c356fc47d Mon Sep 17 00:00:00 2001
From: alirezamoshtaghi <alireza.moshtaghi at gmail.com>
Date: Wed, 28 Feb 2024 13:55:11 -0800
Subject: [PATCH 1/5] [CLANGD] Do not crash on designator initialization of
 union

---
 .../clangd/test/designator_init.test          | 31 +++++++++++++++++++
 clang/lib/AST/Expr.cpp                        | 14 +++++++--
 2 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100644 clang-tools-extra/clangd/test/designator_init.test

diff --git a/clang-tools-extra/clangd/test/designator_init.test b/clang-tools-extra/clangd/test/designator_init.test
new file mode 100644
index 00000000000000..739f2bfab54bcf
--- /dev/null
+++ b/clang-tools-extra/clangd/test/designator_init.test
@@ -0,0 +1,31 @@
+//# RUN: rm -rf %t.dir/* && mkdir -p %t.dir
+//# RUN: echo '[{"directory": "%/t.dir", "command": "clang -x c -c %s", "file": "%s"}]' > %t.dir/compile_commands.json
+//# RUN: clangd --compile-commands-dir=%t.dir --check=%s 2>&1 | FileCheck %s
+
+typedef struct S {
+  unsigned char id;
+  union {
+    unsigned int mask;
+    struct {
+      unsigned int unused:10;
+      unsigned int reserved:3;
+      unsigned int rest:19;
+    };
+  };
+} __attribute__((packed)) S_t;
+
+typedef struct H {
+  unsigned char hid;
+  unsigned int val;
+} handler_t;
+
+struct S
+get_foo (handler_t *h)
+{
+  S_t retval =
+    {.id=h->hid,
+     .mask=h->val};
+  return retval;
+}
+
+// CHECK: All checks completed, 0 errors
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index b4de2155adcebd..33eeeda89fe7a5 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4601,11 +4601,21 @@ SourceRange DesignatedInitExpr::getDesignatorsSourceRange() const {
 SourceLocation DesignatedInitExpr::getBeginLoc() const {
   auto *DIE = const_cast<DesignatedInitExpr *>(this);
   Designator &First = *DIE->getDesignator(0);
-  if (First.isFieldDesignator())
-    return GNUSyntax ? First.getFieldLoc() : First.getDotLoc();
+  if (First.isFieldDesignator()) {
+    /* search all designators in case the first one is not
+       initialized */
+    for (unsigned int i=0; i<DIE->size(); i++) {
+      Designator &Des = *DIE->getDesignator(i);
+      SourceLocation retval = GNUSyntax ? Des.getFieldLoc() : Des.getDotLoc();
+      if (!retval.isValid ())
+	continue;
+      return retval;
+    }
+  }
   return First.getLBracketLoc();
 }
 
+
 SourceLocation DesignatedInitExpr::getEndLoc() const {
   return getInit()->getEndLoc();
 }

>From 397f6cd893a6a07426d39f1dabd7ad27282435af Mon Sep 17 00:00:00 2001
From: alirezamoshtaghi <alireza.moshtaghi at gmail.com>
Date: Mon, 4 Mar 2024 11:54:56 -0800
Subject: [PATCH 2/5] fix formatting errors

---
 clang/lib/AST/Expr.cpp | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 33eeeda89fe7a5..2e7170ecac0618 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4602,20 +4602,17 @@ SourceLocation DesignatedInitExpr::getBeginLoc() const {
   auto *DIE = const_cast<DesignatedInitExpr *>(this);
   Designator &First = *DIE->getDesignator(0);
   if (First.isFieldDesignator()) {
-    /* search all designators in case the first one is not
-       initialized */
-    for (unsigned int i=0; i<DIE->size(); i++) {
+    for (unsigned int i = 0; i < DIE->size(); i++) {
       Designator &Des = *DIE->getDesignator(i);
       SourceLocation retval = GNUSyntax ? Des.getFieldLoc() : Des.getDotLoc();
-      if (!retval.isValid ())
-	continue;
+      if (!retval.isValid())
+        continue;
       return retval;
     }
   }
   return First.getLBracketLoc();
 }
 
-
 SourceLocation DesignatedInitExpr::getEndLoc() const {
   return getInit()->getEndLoc();
 }

>From d1d1458809e0b27dc05c70ef449ebbe6896437d4 Mon Sep 17 00:00:00 2001
From: alirezamoshtaghi <alireza.moshtaghi at gmail.com>
Date: Tue, 5 Mar 2024 00:04:52 -0800
Subject: [PATCH 3/5] dont test designator on Windows

---
 clang-tools-extra/clangd/test/designator_init.test | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang-tools-extra/clangd/test/designator_init.test b/clang-tools-extra/clangd/test/designator_init.test
index 739f2bfab54bcf..4009945ed81fd4 100644
--- a/clang-tools-extra/clangd/test/designator_init.test
+++ b/clang-tools-extra/clangd/test/designator_init.test
@@ -1,7 +1,7 @@
 //# RUN: rm -rf %t.dir/* && mkdir -p %t.dir
 //# RUN: echo '[{"directory": "%/t.dir", "command": "clang -x c -c %s", "file": "%s"}]' > %t.dir/compile_commands.json
 //# RUN: clangd --compile-commands-dir=%t.dir --check=%s 2>&1 | FileCheck %s
-
+//# UNSUPPORTED: system-windows
 typedef struct S {
   unsigned char id;
   union {

>From a792acd4eec0f549bdbf9779dce14aca5357c2db Mon Sep 17 00:00:00 2001
From: alirezamoshtaghi <alireza.moshtaghi at gmail.com>
Date: Wed, 13 Mar 2024 18:51:38 -0700
Subject: [PATCH 4/5] Move the test to clangd unit tests

---
 .../clangd/test/designator_init.test          | 31 -------------------
 .../unittests/tweaks/ExtractVariableTests.cpp | 16 ++++++++++
 clang/lib/AST/Expr.cpp                        |  2 ++
 3 files changed, 18 insertions(+), 31 deletions(-)
 delete mode 100644 clang-tools-extra/clangd/test/designator_init.test

diff --git a/clang-tools-extra/clangd/test/designator_init.test b/clang-tools-extra/clangd/test/designator_init.test
deleted file mode 100644
index 4009945ed81fd4..00000000000000
--- a/clang-tools-extra/clangd/test/designator_init.test
+++ /dev/null
@@ -1,31 +0,0 @@
-//# RUN: rm -rf %t.dir/* && mkdir -p %t.dir
-//# RUN: echo '[{"directory": "%/t.dir", "command": "clang -x c -c %s", "file": "%s"}]' > %t.dir/compile_commands.json
-//# RUN: clangd --compile-commands-dir=%t.dir --check=%s 2>&1 | FileCheck %s
-//# UNSUPPORTED: system-windows
-typedef struct S {
-  unsigned char id;
-  union {
-    unsigned int mask;
-    struct {
-      unsigned int unused:10;
-      unsigned int reserved:3;
-      unsigned int rest:19;
-    };
-  };
-} __attribute__((packed)) S_t;
-
-typedef struct H {
-  unsigned char hid;
-  unsigned int val;
-} handler_t;
-
-struct S
-get_foo (handler_t *h)
-{
-  S_t retval =
-    {.id=h->hid,
-     .mask=h->val};
-  return retval;
-}
-
-// CHECK: All checks completed, 0 errors
diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
index 42dd612eeeec46..947b1ed3b421f8 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
@@ -72,6 +72,22 @@ TEST_F(ExtractVariableTest, Test) {
     )cpp";
   EXPECT_UNAVAILABLE(NoCrashCasesC);
 
+  ExtraArgs = {"-xc"};
+  const char *NoCrashDesignator = R"cpp(
+    struct A {
+      struct {
+        int x;
+      };
+    };
+    struct B {
+      int y;
+    };
+    void foo(struct B *b) {
+      struct A a = {.[[x]]=b->y};
+    }
+  )cpp";
+  EXPECT_UNAVAILABLE(NoCrashDesignator);
+
   ExtraArgs = {"-xobjective-c"};
   const char *AvailableObjC = R"cpp(
     __attribute__((objc_root_class))
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 2e7170ecac0618..2ae1df8e27da2f 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4602,6 +4602,8 @@ SourceLocation DesignatedInitExpr::getBeginLoc() const {
   auto *DIE = const_cast<DesignatedInitExpr *>(this);
   Designator &First = *DIE->getDesignator(0);
   if (First.isFieldDesignator()) {
+    // Skip past implicit designators for anonymous structs/unions, since
+    // these do not have valid source locations.
     for (unsigned int i = 0; i < DIE->size(); i++) {
       Designator &Des = *DIE->getDesignator(i);
       SourceLocation retval = GNUSyntax ? Des.getFieldLoc() : Des.getDotLoc();

>From 57ba8885d255c39f7fe715eccc73fa28f30992f3 Mon Sep 17 00:00:00 2001
From: alirezamoshtaghi <alireza.moshtaghi at gmail.com>
Date: Fri, 15 Mar 2024 15:44:39 -0700
Subject: [PATCH 5/5] use EXPECT_AVAILABLE in the testcase

---
 .../clangd/unittests/tweaks/ExtractVariableTests.cpp          | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
index 947b1ed3b421f8..656b62c9a1f4e1 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
@@ -83,10 +83,10 @@ TEST_F(ExtractVariableTest, Test) {
       int y;
     };
     void foo(struct B *b) {
-      struct A a = {.[[x]]=b->y};
+      struct A a = {.x=b[[->]]y};
     }
   )cpp";
-  EXPECT_UNAVAILABLE(NoCrashDesignator);
+  EXPECT_AVAILABLE(NoCrashDesignator);
 
   ExtraArgs = {"-xobjective-c"};
   const char *AvailableObjC = R"cpp(



More information about the cfe-commits mailing list