vsk added a comment.
Thanks for the feedback!
================
Comment at: test/CodeGenCXX/ubsan-bitfields.cpp:21
+ // CHECK: call void @__ubsan_handle_load_invalid_value
+ return s->e1;
+}
----------------
arphaman wrote:
> Can we avoid the check if the bitfield is 2 bits wide?
I don't think so, because if the user memset()'s the memory backing the struct,
we could still load a value outside of {1, 2, 3} from the bitfield.
================
Comment at: test/CodeGenObjC/ubsan-bool.m:25
+ // OBJC: [[ASHR:%.*]] = ashr i8 [[SHL]], 7
+ // OBJC: [[ICMP:%.*]] = icmp ule i8 [[ASHR]], 1, !nosanitize
+ // OBJC: call void @__ubsan_handle_load_invalid_value
----------------
arphaman wrote:
> One unrelated thing that I noticed in the IR, the `zext`s to `i64` are
> emitted before the branch, even though they are used only in the
> `invalid_value` blocks. I know that the optimizer can move those anyway, but
> would there any be any benefit in moving them into the blocks at the frontend
> IRGen level?
I'm not sure. Maybe it would make the live range of the zext smaller at -O0?
I'll look into this and see if there's a real perf impact.
================
Comment at: test/CodeGenObjC/ubsan-bool.m:26
+ // OBJC: [[ICMP:%.*]] = icmp ule i8 [[ASHR]], 1, !nosanitize
+ // OBJC: call void @__ubsan_handle_load_invalid_value
+
----------------
arphaman wrote:
> Is it possible to avoid the check here since the bitfield is just one bit
> wide?
No, a user could do:
```
struct S1 s;
s.b1 = 1;
if (s.b1 == 1) // evaluates to false, s.b1 is negative
```
With this patch we get:
```
runtime error: load of value -1, which is not a valid value for type 'BOOL'
(aka 'signed char')
```
https://reviews.llvm.org/D30423
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
