[clang] [TBAA] Skip all bitfields when generating !tbaa.struct metadata. (PR #82922)

Florian Hahn via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 26 02:20:33 PST 2024


https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/82922

>From 556fcefed9645aa0a0a965ff8f22d8accdf9eefc Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Sun, 25 Feb 2024 13:23:17 +0000
Subject: [PATCH 1/2] [TBAA] Skip all bitfields when generating !tbaa.struct
 metadata.

At the moment, clang generates what I believe are incorrect !tbaa.struct
fields for named bitfields. At the moment, the base type size is used
for named bifields (e.g. sizeof(int)) instead of the bifield width per
field. This results in overalpping fields in !tbaa.struct metadata.

This causes incorrect results when extracting individual copied fields
from !tbaa.struct as in added in dc85719d5.

This patch fixes that by skipping all bitfields, not only unnamed ones
(note that CollectFields has a TODO to support bitfields). As bitfields
specify their widths in bits, while !tbaa metadata uses bytes for sizes
and offsets, I don't think we would be able to generate correct metadata
for them in general.

If this understanding is correct, I can also extend the verifier to
check that !tbaa.struct fields aren't overlapping.

Fixes https://github.com/llvm/llvm-project/issues/82586
---
 clang/lib/CodeGen/CodeGenTBAA.cpp  | 2 +-
 clang/test/CodeGen/tbaa-struct.cpp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp b/clang/lib/CodeGen/CodeGenTBAA.cpp
index dc288bc3f6157a..43a1aee3d73823 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -298,7 +298,7 @@ CodeGenTBAA::CollectFields(uint64_t BaseOffset,
     unsigned idx = 0;
     for (RecordDecl::field_iterator i = RD->field_begin(),
          e = RD->field_end(); i != e; ++i, ++idx) {
-      if ((*i)->isZeroSize(Context) || (*i)->isUnnamedBitfield())
+      if ((*i)->isZeroSize(Context) || (*i)->isBitField())
         continue;
       uint64_t Offset = BaseOffset +
                         Layout.getFieldOffset(idx) / Context.getCharWidth();
diff --git a/clang/test/CodeGen/tbaa-struct.cpp b/clang/test/CodeGen/tbaa-struct.cpp
index ff5521fcf3f604..17c9d6bf6a7260 100644
--- a/clang/test/CodeGen/tbaa-struct.cpp
+++ b/clang/test/CodeGen/tbaa-struct.cpp
@@ -130,7 +130,7 @@ void copy8(NamedBitfields *a1, NamedBitfields *a2) {
 // CHECK-OLD: [[TS3]] = !{i64 0, i64 8, !{{.*}}, i64 0, i64 2, !{{.*}}, i64 4, i64 8, !{{.*}}}
 // CHECK-OLD: [[TS4]] = !{i64 0, i64 1, [[TAG_CHAR]], i64 1, i64 1, [[TAG_CHAR]], i64 2, i64 1, [[TAG_CHAR]]}
 // CHECK-OLD: [[TS5]] = !{i64 0, i64 1, [[TAG_CHAR]], i64 4, i64 1, [[TAG_CHAR]], i64 5, i64 1, [[TAG_CHAR]]}
-// CHECK-OLD: [[TS6]] = !{i64 0, i64 4, [[TAG_INT]], i64 1, i64 4, [[TAG_INT]], i64 2, i64 1, [[TAG_CHAR]], i64 8, i64 8, [[TAG_DOUBLE:!.+]]}
+// CHECK-OLD: [[TS6]] = !{i64 2, i64 1, [[TAG_CHAR]], i64 8, i64 8, [[TAG_DOUBLE:!.+]]}
 // CHECK-OLD: [[TAG_DOUBLE]] = !{[[DOUBLE:!.+]], [[DOUBLE]], i64 0}
 // CHECK-OLD  [[DOUBLE]] = !{!"double", [[CHAR]], i64 0}
 

>From 32be3b7d944fc5da50add4b6fea551ba6c9d428c Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Mon, 26 Feb 2024 10:19:57 +0000
Subject: [PATCH 2/2] WIP use CGBitFieldInfo.

---
 clang/lib/CodeGen/CodeGenModule.cpp |  2 +-
 clang/lib/CodeGen/CodeGenTBAA.cpp   | 33 +++++++++++++++++++++++------
 clang/lib/CodeGen/CodeGenTBAA.h     |  4 +++-
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 95e457bef28ed3..ed8db055723024 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -397,7 +397,7 @@ CodeGenModule::CodeGenModule(ASTContext &C,
   // Enable TBAA unless it's suppressed. ThreadSanitizer needs TBAA even at O0.
   if (LangOpts.Sanitize.has(SanitizerKind::Thread) ||
       (!CodeGenOpts.RelaxedAliasing && CodeGenOpts.OptimizationLevel > 0))
-    TBAA.reset(new CodeGenTBAA(Context, TheModule, CodeGenOpts, getLangOpts(),
+    TBAA.reset(new CodeGenTBAA(Context, getTypes(), TheModule, CodeGenOpts, getLangOpts(),
                                getCXXABI().getMangleContext()));
 
   // If debug info or coverage generation is enabled, create the CGDebugInfo
diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp b/clang/lib/CodeGen/CodeGenTBAA.cpp
index 43a1aee3d73823..6a453bb1144582 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -14,6 +14,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "CGRecordLayout.h"
+#include "CodeGenTypes.h"
 #include "CodeGenTBAA.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
@@ -29,10 +31,10 @@
 using namespace clang;
 using namespace CodeGen;
 
-CodeGenTBAA::CodeGenTBAA(ASTContext &Ctx, llvm::Module &M,
+CodeGenTBAA::CodeGenTBAA(ASTContext &Ctx, CodeGenTypes &CGTypes, llvm::Module &M,
                          const CodeGenOptions &CGO,
                          const LangOptions &Features, MangleContext &MContext)
-  : Context(Ctx), Module(M), CodeGenOpts(CGO),
+  : Context(Ctx), CGTypes(CGTypes), Module(M), CodeGenOpts(CGO),
     Features(Features), MContext(MContext), MDHelper(M.getContext()),
     Root(nullptr), Char(nullptr)
 {}
@@ -294,18 +296,37 @@ CodeGenTBAA::CollectFields(uint64_t BaseOffset,
         return false;
 
     const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
+    const CGRecordLayout &CGRL = CGTypes.getCGRecordLayout(RD);
 
     unsigned idx = 0;
     for (RecordDecl::field_iterator i = RD->field_begin(),
          e = RD->field_end(); i != e; ++i, ++idx) {
-      if ((*i)->isZeroSize(Context) || (*i)->isBitField())
+      if ((*i)->isZeroSize(Context))
         continue;
+
       uint64_t Offset = BaseOffset +
                         Layout.getFieldOffset(idx) / Context.getCharWidth();
       QualType FieldQTy = i->getType();
-      if (!CollectFields(Offset, FieldQTy, Fields,
-                         MayAlias || TypeHasMayAlias(FieldQTy)))
-        return false;
+      if ((*i)->isBitField()) {
+        RecordDecl::field_iterator j = i;
+        unsigned CurrentBitFieldSize = 0;
+        while (j != e && (*j)->isBitField()) {
+          const CGBitFieldInfo &Info = CGRL.getBitFieldInfo(*j);
+          CurrentBitFieldSize += Info.Size;
+          j++;
+        }
+        uint64_t Offset = BaseOffset;
+        uint64_t Size = llvm::divideCeil(CurrentBitFieldSize , 8);
+        llvm::MDNode *TBAAType = getChar();
+        llvm::MDNode *TBAATag = getAccessTagInfo(TBAAAccessInfo(TBAAType, Size));
+        Fields.push_back(llvm::MDBuilder::TBAAStructField(Offset, Size, TBAATag));
+        i = std::prev(j);
+      } else {
+        if (!CollectFields(Offset, FieldQTy, Fields,
+                           MayAlias || TypeHasMayAlias(FieldQTy)))
+          return false;
+
+        }
     }
     return true;
   }
diff --git a/clang/lib/CodeGen/CodeGenTBAA.h b/clang/lib/CodeGen/CodeGenTBAA.h
index a65963596fe9de..7e315c285be389 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.h
+++ b/clang/lib/CodeGen/CodeGenTBAA.h
@@ -29,6 +29,7 @@ namespace clang {
   class Type;
 
 namespace CodeGen {
+  class CodeGenTypes;
 
 // TBAAAccessKind - A kind of TBAA memory access descriptor.
 enum class TBAAAccessKind : unsigned {
@@ -115,6 +116,7 @@ struct TBAAAccessInfo {
 /// while lowering AST types to LLVM types.
 class CodeGenTBAA {
   ASTContext &Context;
+  CodeGenTypes &CGTypes;
   llvm::Module &Module;
   const CodeGenOptions &CodeGenOpts;
   const LangOptions &Features;
@@ -167,7 +169,7 @@ class CodeGenTBAA {
   llvm::MDNode *getBaseTypeInfoHelper(const Type *Ty);
 
 public:
-  CodeGenTBAA(ASTContext &Ctx, llvm::Module &M, const CodeGenOptions &CGO,
+  CodeGenTBAA(ASTContext &Ctx, CodeGenTypes &CGTypes, llvm::Module &M, const CodeGenOptions &CGO,
               const LangOptions &Features, MangleContext &MContext);
   ~CodeGenTBAA();
 



More information about the cfe-commits mailing list