Skip to content

Commit

Permalink
Revert "[TBAA] Emit distinct TBAA tags for pointers with different de…
Browse files Browse the repository at this point in the history
…pths,types. (#76612)"

This reverts commit 038c48c.

This is causing test failures in some configurations, reverted while I
investigate.

Failures include

 http://lab.llvm.org/buildbot/#/builders/11/builds/1623
 http://lab.llvm.org/buildbot/#/builders/108/builds/1172
  • Loading branch information
fhahn committed Jul 12, 2024
1 parent 99685a5 commit bee2403
Show file tree
Hide file tree
Showing 9 changed files with 472 additions and 565 deletions.
7 changes: 0 additions & 7 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -425,10 +425,6 @@ Non-comprehensive list of changes in this release
- Added support for ``TypeLoc::dump()`` for easier debugging, and improved
textual and JSON dumping for various ``TypeLoc``-related nodes.

- Clang can now emit distinct type-based alias analysis tags for incompatible
pointers, enabling more powerful alias analysis when accessing pointer types.
The new behavior can be enabled using ``-fpointer-tbaa``.

New Compiler Flags
------------------
- ``-fsanitize=implicit-bitfield-conversion`` checks implicit truncation and
Expand Down Expand Up @@ -471,9 +467,6 @@ New Compiler Flags
- For the ARM target, added ``-Warm-interrupt-vfp-clobber`` that will emit a
diagnostic when an interrupt handler is declared and VFP is enabled.

- ``-fpointer-tbaa`` enables emission of distinct type-based alias
analysis tags for incompatible pointers.

Deprecated Compiler Flags
-------------------------

Expand Down
1 change: 0 additions & 1 deletion clang/include/clang/Basic/CodeGenOptions.def
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,6 @@ ENUM_CODEGENOPT(StructReturnConvention, StructReturnConventionKind, 2, SRCK_Defa

CODEGENOPT(RelaxAll , 1, 0) ///< Relax all machine code instructions.
CODEGENOPT(RelaxedAliasing , 1, 0) ///< Set when -fno-strict-aliasing is enabled.
CODEGENOPT(PointerTBAA, 1, 0) ///< Whether or not to use distinct TBAA tags for pointers.
CODEGENOPT(StructPathTBAA , 1, 0) ///< Whether or not to use struct-path TBAA.
CODEGENOPT(NewStructPathTBAA , 1, 0) ///< Whether or not to use enhanced struct-path TBAA.
CODEGENOPT(SaveTempLabels , 1, 0) ///< Save temporary labels.
Expand Down
5 changes: 0 additions & 5 deletions clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -3379,7 +3379,6 @@ def fstruct_path_tbaa : Flag<["-"], "fstruct-path-tbaa">, Group<f_Group>;
def fno_struct_path_tbaa : Flag<["-"], "fno-struct-path-tbaa">, Group<f_Group>;
def fno_strict_enums : Flag<["-"], "fno-strict-enums">, Group<f_Group>;
def fno_strict_overflow : Flag<["-"], "fno-strict-overflow">, Group<f_Group>;
def fno_pointer_tbaa : Flag<["-"], "fno-pointer-tbaa">, Group<f_Group>;
def fno_temp_file : Flag<["-"], "fno-temp-file">, Group<f_Group>,
Visibility<[ClangOption, CC1Option, CLOption, DXCOption]>, HelpText<
"Directly create compilation output files. This may lead to incorrect incremental builds if the compiler crashes">,
Expand Down Expand Up @@ -3884,7 +3883,6 @@ defm strict_vtable_pointers : BoolFOption<"strict-vtable-pointers",
" overwriting polymorphic C++ objects">,
NegFlag<SetFalse>>;
def fstrict_overflow : Flag<["-"], "fstrict-overflow">, Group<f_Group>;
def fpointer_tbaa : Flag<["-"], "fpointer-tbaa">, Group<f_Group>;
def fdriver_only : Flag<["-"], "fdriver-only">, Flags<[NoXarchOption]>,
Visibility<[ClangOption, CLOption, DXCOption]>,
Group<Action_Group>, HelpText<"Only run the driver.">;
Expand Down Expand Up @@ -7148,9 +7146,6 @@ def fuse_register_sized_bitfield_access: Flag<["-"], "fuse-register-sized-bitfie
def relaxed_aliasing : Flag<["-"], "relaxed-aliasing">,
HelpText<"Turn off Type Based Alias Analysis">,
MarshallingInfoFlag<CodeGenOpts<"RelaxedAliasing">>;
def pointer_tbaa: Flag<["-"], "pointer-tbaa">,
HelpText<"Turn on Type Based Alias Analysis for pointer accesses">,
MarshallingInfoFlag<CodeGenOpts<"PointerTBAA">>;
def no_struct_path_tbaa : Flag<["-"], "no-struct-path-tbaa">,
HelpText<"Turn off struct-path aware Type Based Alias Analysis">,
MarshallingInfoNegativeFlag<CodeGenOpts<"StructPathTBAA">>;
Expand Down
54 changes: 4 additions & 50 deletions clang/lib/CodeGen/CodeGenTBAA.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,56 +185,10 @@ llvm::MDNode *CodeGenTBAA::getTypeInfoHelper(const Type *Ty) {
return getChar();

// Handle pointers and references.
//
// C has a very strict rule for pointer aliasing. C23 6.7.6.1p2:
// For two pointer types to be compatible, both shall be identically
// qualified and both shall be pointers to compatible types.
//
// This rule is impractically strict; we want to at least ignore CVR
// qualifiers. Distinguishing by CVR qualifiers would make it UB to
// e.g. cast a `char **` to `const char * const *` and dereference it,
// which is too common and useful to invalidate. C++'s similar types
// rule permits qualifier differences in these nested positions; in fact,
// C++ even allows that cast as an implicit conversion.
//
// Other qualifiers could theoretically be distinguished, especially if
// they involve a significant representation difference. We don't
// currently do so, however.
//
// Computing the pointee type string recursively is implicitly more
// forgiving than the standards require. Effectively, we are turning
// the question "are these types compatible/similar" into "are
// accesses to these types allowed to alias". In both C and C++,
// the latter question has special carve-outs for signedness
// mismatches that only apply at the top level. As a result, we are
// allowing e.g. `int *` l-values to access `unsigned *` objects.
if (Ty->isPointerType() || Ty->isReferenceType()) {
llvm::MDNode *AnyPtr = createScalarTypeNode("any pointer", getChar(), Size);
if (!CodeGenOpts.PointerTBAA)
return AnyPtr;
// Compute the depth of the pointer and generate a tag of the form "p<depth>
// <base type tag>".
unsigned PtrDepth = 0;
do {
PtrDepth++;
Ty = Ty->getPointeeType().getTypePtr();
} while (Ty->isPointerType());
// TODO: Implement C++'s type "similarity" and consider dis-"similar"
// pointers distinct for non-builtin types.
if (isa<BuiltinType>(Ty)) {
llvm::MDNode *ScalarMD = getTypeInfoHelper(Ty);
StringRef Name =
cast<llvm::MDString>(
ScalarMD->getOperand(CodeGenOpts.NewStructPathTBAA ? 2 : 0))
->getString();
SmallString<256> OutName("p");
OutName += std::to_string(PtrDepth);
OutName += " ";
OutName += Name;
return createScalarTypeNode(OutName, AnyPtr, Size);
}
return AnyPtr;
}
// TODO: Implement C++'s type "similarity" and consider dis-"similar"
// pointers distinct.
if (Ty->isPointerType() || Ty->isReferenceType())
return createScalarTypeNode("any pointer", getChar(), Size);

// Accesses to arrays are accesses to objects of their element types.
if (CodeGenOpts.NewStructPathTBAA && Ty->isArrayType())
Expand Down
3 changes: 0 additions & 3 deletions clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5721,9 +5721,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
if (!Args.hasFlag(options::OPT_fstrict_aliasing, StrictAliasingAliasOption,
options::OPT_fno_strict_aliasing, !IsWindowsMSVC))
CmdArgs.push_back("-relaxed-aliasing");
if (Args.hasFlag(options::OPT_fpointer_tbaa, options::OPT_fno_pointer_tbaa,
false))
CmdArgs.push_back("-pointer-tbaa");
if (!Args.hasFlag(options::OPT_fstruct_path_tbaa,
options::OPT_fno_struct_path_tbaa, true))
CmdArgs.push_back("-no-struct-path-tbaa");
Expand Down
46 changes: 30 additions & 16 deletions clang/test/CodeGen/sanitize-metadata-nosanitize.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
//.
// CHECK: Function Attrs: mustprogress nofree noinline norecurse nosync nounwind willreturn memory(write, argmem: none, inaccessiblemem: none)
// CHECK-LABEL: define dso_local void @escape
// CHECK-SAME: (ptr noundef [[P:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] !pcsections [[META2:![0-9]+]] {
// CHECK-SAME: (ptr noundef [[P:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] !pcsections !2 {
// CHECK-NEXT: entry:
// CHECK-NEXT: ret void
//
Expand All @@ -23,13 +23,13 @@ __attribute__((noinline, not_tail_called)) void escape(const volatile void *p) {

// CHECK: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none)
// CHECK-LABEL: define dso_local i32 @normal_function
// CHECK-SAME: (ptr noundef [[X:%.*]], ptr nocapture noundef readonly [[Y:%.*]]) local_unnamed_addr #[[ATTR1:[0-9]+]] !pcsections [[META4:![0-9]+]] {
// CHECK-SAME: (ptr noundef [[X:%.*]], ptr nocapture noundef readonly [[Y:%.*]]) local_unnamed_addr #[[ATTR1:[0-9]+]] !pcsections !4 {
// CHECK-NEXT: entry:
// CHECK-NEXT: [[X_ADDR:%.*]] = alloca ptr, align 8
// CHECK-NEXT: store ptr [[X]], ptr [[X_ADDR]], align 8, !tbaa [[TBAA6:![0-9]+]]
// CHECK-NEXT: store atomic i32 1, ptr [[X]] monotonic, align 4, !pcsections [[META11:![0-9]+]]
// CHECK-NEXT: store atomic i32 1, ptr [[X]] monotonic, align 4, !pcsections !10
// CHECK-NEXT: notail call void @escape(ptr noundef nonnull [[X_ADDR]])
// CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[Y]], align 4, !tbaa [[TBAA12:![0-9]+]]
// CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[Y]], align 4, !tbaa [[TBAA11:![0-9]+]]
// CHECK-NEXT: ret i32 [[TMP0]]
//
int normal_function(int *x, int *y) {
Expand All @@ -46,7 +46,7 @@ int normal_function(int *x, int *y) {
// CHECK-NEXT: store ptr [[X]], ptr [[X_ADDR]], align 8, !tbaa [[TBAA6]]
// CHECK-NEXT: store atomic i32 1, ptr [[X]] monotonic, align 4
// CHECK-NEXT: notail call void @escape(ptr noundef nonnull [[X_ADDR]])
// CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[Y]], align 4, !tbaa [[TBAA12]]
// CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[Y]], align 4, !tbaa [[TBAA11]]
// CHECK-NEXT: ret i32 [[TMP0]]
//
__attribute__((disable_sanitizer_instrumentation)) int test_disable_sanitize_instrumentation(int *x, int *y) {
Expand All @@ -57,13 +57,13 @@ __attribute__((disable_sanitizer_instrumentation)) int test_disable_sanitize_ins

// CHECK: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none)
// CHECK-LABEL: define dso_local i32 @test_no_sanitize_thread
// CHECK-SAME: (ptr noundef [[X:%.*]], ptr nocapture noundef readonly [[Y:%.*]]) local_unnamed_addr #[[ATTR3:[0-9]+]] !pcsections [[META14:![0-9]+]] {
// CHECK-SAME: (ptr noundef [[X:%.*]], ptr nocapture noundef readonly [[Y:%.*]]) local_unnamed_addr #[[ATTR3:[0-9]+]] !pcsections !13 {
// CHECK-NEXT: entry:
// CHECK-NEXT: [[X_ADDR:%.*]] = alloca ptr, align 8
// CHECK-NEXT: store ptr [[X]], ptr [[X_ADDR]], align 8, !tbaa [[TBAA6]]
// CHECK-NEXT: store atomic i32 1, ptr [[X]] monotonic, align 4, !pcsections [[META11]]
// CHECK-NEXT: store atomic i32 1, ptr [[X]] monotonic, align 4, !pcsections !10
// CHECK-NEXT: notail call void @escape(ptr noundef nonnull [[X_ADDR]])
// CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[Y]], align 4, !tbaa [[TBAA12]]
// CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[Y]], align 4, !tbaa [[TBAA11]]
// CHECK-NEXT: ret i32 [[TMP0]]
//
__attribute__((no_sanitize("thread"))) int test_no_sanitize_thread(int *x, int *y) {
Expand All @@ -74,13 +74,13 @@ __attribute__((no_sanitize("thread"))) int test_no_sanitize_thread(int *x, int *

// CHECK: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none)
// CHECK-LABEL: define dso_local i32 @test_no_sanitize_all
// CHECK-SAME: (ptr noundef [[X:%.*]], ptr nocapture noundef readonly [[Y:%.*]]) local_unnamed_addr #[[ATTR3]] !pcsections [[META14]] {
// CHECK-SAME: (ptr noundef [[X:%.*]], ptr nocapture noundef readonly [[Y:%.*]]) local_unnamed_addr #[[ATTR3]] !pcsections !13 {
// CHECK-NEXT: entry:
// CHECK-NEXT: [[X_ADDR:%.*]] = alloca ptr, align 8
// CHECK-NEXT: store ptr [[X]], ptr [[X_ADDR]], align 8, !tbaa [[TBAA6]]
// CHECK-NEXT: store atomic i32 1, ptr [[X]] monotonic, align 4, !pcsections [[META11]]
// CHECK-NEXT: store atomic i32 1, ptr [[X]] monotonic, align 4, !pcsections !10
// CHECK-NEXT: notail call void @escape(ptr noundef nonnull [[X_ADDR]])
// CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[Y]], align 4, !tbaa [[TBAA12]]
// CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[Y]], align 4, !tbaa [[TBAA11]]
// CHECK-NEXT: ret i32 [[TMP0]]
//
__attribute__((no_sanitize("all"))) int test_no_sanitize_all(int *x, int *y) {
Expand All @@ -89,9 +89,23 @@ __attribute__((no_sanitize("all"))) int test_no_sanitize_all(int *x, int *y) {
return *y;
}
//.
// CHECK: attributes #[[ATTR0]] = { mustprogress nofree noinline norecurse nosync nounwind willreturn memory(write, argmem: none, inaccessiblemem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
// CHECK: attributes #[[ATTR1]] = { mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
// CHECK: attributes #[[ATTR2]] = { disable_sanitizer_instrumentation mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
// CHECK: attributes #[[ATTR3]] = { mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "no_sanitize_thread" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
// CHECK: attributes #[[ATTR4:[0-9]+]] = { nounwind "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
// CHECK: attributes #0 = { mustprogress nofree noinline norecurse nosync nounwind willreturn memory(write, argmem: none, inaccessiblemem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
// CHECK: attributes #1 = { mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
// CHECK: attributes #2 = { disable_sanitizer_instrumentation mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
// CHECK: attributes #3 = { mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "no_sanitize_thread" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
// CHECK: attributes #4 = { nounwind "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
//.
// CHECK: !2 = !{!"sanmd_covered2!C", !3}
// CHECK: !3 = !{i64 0}
// CHECK: !4 = !{!"sanmd_covered2!C", !5}
// CHECK: !5 = !{i64 3}
// CHECK: !6 = !{!7, !7, i64 0}
// CHECK: !7 = !{!"any pointer", !8, i64 0}
// CHECK: !8 = !{!"omnipotent char", !9, i64 0}
// CHECK: !9 = !{!"Simple C/C++ TBAA"}
// CHECK: !10 = !{!"sanmd_atomics2!C"}
// CHECK: !11 = !{!12, !12, i64 0}
// CHECK: !12 = !{!"int", !8, i64 0}
// CHECK: !13 = !{!"sanmd_covered2!C", !14}
// CHECK: !14 = !{i64 2}
//.
Loading

0 comments on commit bee2403

Please sign in to comment.