filcab added inline comments.
================ Comment at: test/CodeGenObjC/ubsan-bool.m:26 + // OBJC: [[ICMP:%.*]] = icmp ule i8 [[ASHR]], 1, !nosanitize + // OBJC: call void @__ubsan_handle_load_invalid_value + ---------------- jroelofs wrote: > vsk wrote: > > jroelofs wrote: > > > vsk wrote: > > > > 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') > > > > ``` > > > What if BOOL is an `unsigned char`, or a `char` that's not signed? > > Good point, we don't need to emit the range check if the bitfield is an > > unsigned, 1-bit wide BOOL. Would you be OK with me tackling that in a > > follow-up patch? > No problem, sounds good to me. I don't like generating an error here. -1 is a perfectly valid BOOL (signed char) value and it's a perfectly valid value to have in a (signed) 1-wide bitfield. ================ Comment at: test/CodeGenObjC/ubsan-bool.m:50 +// OBJC: [[SHL:%.*]] = shl i8 [[LOAD]], 7 +// OBJC: [[ASHR:%.*]] = ashr i8 [[SHL]], 7 +// OBJC: icmp ule i8 [[ASHR]], 1, !nosanitize ---------------- jroelofs wrote: > hmmm. just occurred to me that this value is always going to be 0xff or 0x0, > so it might be useful if there were also a frontend warning that complains > about signed bitfield bools (maybe something useful for another patch). I'm more ok with this, assuming it makes sense in ObjC. https://reviews.llvm.org/D30423 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
