Skip to content

Commit

Permalink
[TBAA] Handle bitfields when generating !tbaa.struct metadata. (#82922)
Browse files Browse the repository at this point in the history
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 dc85719.

This patch fixes that by skipping by combining adjacent bitfields
in fields with correct sizes.

Fixes #82586
  • Loading branch information
fhahn committed Feb 27, 2024
1 parent 0d1f957 commit d2a9df2
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 18 deletions.
4 changes: 2 additions & 2 deletions clang/lib/CodeGen/CodeGenModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,8 @@ 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(),
getCXXABI().getMangleContext()));
TBAA.reset(new CodeGenTBAA(Context, getTypes(), TheModule, CodeGenOpts,
getLangOpts(), getCXXABI().getMangleContext()));

// If debug info or coverage generation is enabled, create the CGDebugInfo
// object.
Expand Down
44 changes: 33 additions & 11 deletions clang/lib/CodeGen/CodeGenTBAA.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
//===----------------------------------------------------------------------===//

#include "CodeGenTBAA.h"
#include "CGRecordLayout.h"
#include "CodeGenTypes.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Attr.h"
#include "clang/AST/Mangle.h"
Expand All @@ -26,16 +28,16 @@
#include "llvm/IR/Metadata.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/Type.h"
#include "llvm/Support/Debug.h"
using namespace clang;
using namespace CodeGen;

CodeGenTBAA::CodeGenTBAA(ASTContext &Ctx, llvm::Module &M,
const CodeGenOptions &CGO,
CodeGenTBAA::CodeGenTBAA(ASTContext &Ctx, CodeGenTypes &CGTypes,
llvm::Module &M, const CodeGenOptions &CGO,
const LangOptions &Features, MangleContext &MContext)
: Context(Ctx), Module(M), CodeGenOpts(CGO),
Features(Features), MContext(MContext), MDHelper(M.getContext()),
Root(nullptr), Char(nullptr)
{}
: Context(Ctx), CGTypes(CGTypes), Module(M), CodeGenOpts(CGO),
Features(Features), MContext(MContext), MDHelper(M.getContext()),
Root(nullptr), Char(nullptr) {}

CodeGenTBAA::~CodeGenTBAA() {
}
Expand Down Expand Up @@ -294,14 +296,34 @@ 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)->isUnnamedBitfield())
for (RecordDecl::field_iterator i = RD->field_begin(), e = RD->field_end();
i != e; ++i, ++idx) {
if ((*i)->isZeroSize(Context))
continue;
uint64_t Offset = BaseOffset +
Layout.getFieldOffset(idx) / Context.getCharWidth();

uint64_t Offset =
BaseOffset + Layout.getFieldOffset(idx) / Context.getCharWidth();

// Create a single field for consecutive named bitfields using char as
// base type.
if ((*i)->isBitField()) {
const CGBitFieldInfo &Info = CGRL.getBitFieldInfo(*i);
if (Info.Offset != 0)
continue;
unsigned CurrentBitFieldSize = Info.StorageSize;
uint64_t Size =
llvm::divideCeil(CurrentBitFieldSize, Context.getCharWidth());
llvm::MDNode *TBAAType = getChar();
llvm::MDNode *TBAATag =
getAccessTagInfo(TBAAAccessInfo(TBAAType, Size));
Fields.push_back(
llvm::MDBuilder::TBAAStructField(Offset, Size, TBAATag));
continue;
}

QualType FieldQTy = i->getType();
if (!CollectFields(Offset, FieldQTy, Fields,
MayAlias || TypeHasMayAlias(FieldQTy)))
Expand Down
7 changes: 5 additions & 2 deletions clang/lib/CodeGen/CodeGenTBAA.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ namespace clang {
class Type;

namespace CodeGen {
class CodeGenTypes;

// TBAAAccessKind - A kind of TBAA memory access descriptor.
enum class TBAAAccessKind : unsigned {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -167,8 +169,9 @@ class CodeGenTBAA {
llvm::MDNode *getBaseTypeInfoHelper(const Type *Ty);

public:
CodeGenTBAA(ASTContext &Ctx, llvm::Module &M, const CodeGenOptions &CGO,
const LangOptions &Features, MangleContext &MContext);
CodeGenTBAA(ASTContext &Ctx, CodeGenTypes &CGTypes, llvm::Module &M,
const CodeGenOptions &CGO, const LangOptions &Features,
MangleContext &MContext);
~CodeGenTBAA();

/// getTypeInfo - Get metadata used to describe accesses to objects of the
Expand Down
6 changes: 3 additions & 3 deletions clang/test/CodeGen/tbaa-struct.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,11 @@ void copy10(NamedBitfields3 *a1, NamedBitfields3 *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 0, i64 2, [[TAG_CHAR]], 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}
// CHECK-OLD: [[TS7]] = !{i64 0, i64 1, [[TAG_CHAR]], i64 1, i64 1, [[TAG_CHAR]], i64 2, i64 1, [[TAG_CHAR]], i64 3, i64 4, [[TAG_INT]], i64 3, i64 4, [[TAG_INT]], i64 4, i64 1, [[TAG_CHAR]], i64 8, i64 8, [[TAG_DOUBLE]], i64 16, i64 4, [[TAG_INT]]}
// CHECK-OLD: [[TS8]] = !{i64 1, i64 4, [[TAG_INT]], i64 2, i64 4, [[TAG_INT]], i64 8, i64 8, [[TAG_DOUBLE]]}
// CHECK-OLD: [[TS7]] = !{i64 0, i64 1, [[TAG_CHAR]], i64 1, i64 1, [[TAG_CHAR]], i64 2, i64 1, [[TAG_CHAR]], i64 3, i64 1, [[TAG_CHAR]], i64 4, i64 1, [[TAG_CHAR]], i64 8, i64 8, [[TAG_DOUBLE]], i64 16, i64 1, [[TAG_CHAR]]}
// CHECK-OLD: [[TS8]] = !{i64 0, i64 4, [[TAG_CHAR]], i64 8, i64 8, [[TAG_DOUBLE]]}

// CHECK-NEW-DAG: [[TYPE_char:!.*]] = !{{{.*}}, i64 1, !"omnipotent char"}
// CHECK-NEW-DAG: [[TAG_char]] = !{[[TYPE_char]], [[TYPE_char]], i64 0, i64 0}
Expand Down

0 comments on commit d2a9df2

Please sign in to comment.