void added a comment.
I changed more than just the testcase (not a lot, but non-trivial nonetheless).
PTAL.
================
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
+
----------------
nickdesaulniers wrote:
> 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.
Done.
================
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
----------------
nickdesaulniers wrote:
> Is the call to `func` is unnecessary for this test or can we just `return
> __builtin_dynamic_object_size(p->array, 1)`?
Nope. Not sure why I had it. :-/ Removed.
================
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
----------------
nickdesaulniers wrote:
> 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?
This is a bit weird. I don't understand why it would trap either. It might have
something to do with the 'alloc_size' attribute? (The code that this patch
generates doesn't put that trap in there.) That's probably a bug.
However, when I modify the test to include an actual store:
```
void test3(int index) {
/* ... */
p->array[index] = 42;
}
```
I get two checks: one for the 'counted_by' attribute, and this one, probably
for the bounds check. Again, it's probably because of the 'alloc_size', because
when I remove the call to 'malloc' it generates only the 'counted_by' check...
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
