nickdesaulniers added a comment.
Patch LGTM; just want moar tests.
---
Mind adding a test for
#if !__has_attribute(counted_by)
#error "has attribute broken"
#endif
================
Comment at: clang/test/CodeGen/attr-counted-by.c:2
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
UTC_ARGS: --version 3
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O2 -Wall
-fsanitize=array-bounds,object-size,local-bounds -fstrict-flex-arrays=3
-emit-llvm -o - %s | FileCheck %s
+
----------------
Can you add another run line without the `-fsanitize` flags set, and use 2
different `--check-prefixes` for the two RUN lines? I'd be curious to see the
differences in codegen between those set or not. I assume this attribute
should affect codegen even with all of those disabled (maybe trapping instead
of libcalling into ubsan runtime). I think update_cc_test_checks should be
able to handle that.
================
Comment at: clang/test/CodeGen/attr-counted-by.c:39-51
+// CHECK-LABEL: define dso_local void @test2(
+// CHECK-SAME: ptr noundef [[P:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT: entry:
+// CHECK-NEXT: [[COUNT:%.*]] = getelementptr inbounds
[[STRUCT_ANNOTATED:%.*]], ptr [[P]], i64 0, i32 1
+// CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[COUNT]], align 8, !tbaa
[[TBAA2]]
+// CHECK-NEXT: [[TMP1:%.*]] = shl i32 [[TMP0]], 2
+// CHECK-NEXT: [[TMP2:%.*]] = zext i32 [[TMP1]] to i64
----------------
Is the call to `func` is unnecessary for this test or can we just `return
__builtin_dynamic_object_size(p->array, 1)`?
================
Comment at: clang/test/CodeGen/attr-counted-by.c:67-73
+// CHECK-NEXT: br i1 [[TMP4]], label [[TRAP:%.*]], label [[TMP5:%.*]]
+// CHECK: 5:
+// CHECK-NEXT: tail call void @func(i64 noundef [[ADD1]]) #[[ATTR5]]
+// CHECK-NEXT: ret void
+// CHECK: trap:
+// CHECK-NEXT: tail call void @llvm.trap() #[[ATTR4]]
+// CHECK-NEXT: unreachable
----------------
What is the intent of this test? Mind adding a comment?
In particular, it's not clear to my //why// this case should conditionally
trap, seeing as the dynamically allocated object's `array` field is yet to be
accessed in this example. What am I missing?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148381/new/
https://reviews.llvm.org/D148381
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
