vabridgers wrote:
> > Hi @AaronBallman , I ran a compile using this change on clang as you asked
> > and have results. The compile ran with no crashes or errors, and produced
> > 1478 bitfield-conversion warnings. I'll show a few examples below. To put
> > that number into context, I enabled -Wconversion to compare the new warning
> > - bitfield-conversion - with the other conversion checks. You can see from
> > the results that while 1478 findings is large, it's less than 10% of the
> > total conversion findings that exist - so looking at those numbers in
> > context is helpful.
> > Are there any other suggested improvements for this patch, or do you think
> > it's ok to merge?
> > 
>
> Thank you for running the experiment! From spot-checking the results, did
> they help you to find any actual bugs? And did you notice any patterns in the
> results that could be used to safely silence the diagnostic in some cases?
We did find issues in our code base, and an "interesting" study in coding style
related to the bitfield-enum-conversion check I'll describe.
For the bitfield-conversion check, we found no real interesting patterns other
than maybe the developer could use a mask on the RHS. But we observed that can
mask problems (pun intended!!!) because a value could actually be greater in
rank than the bitfield. An improvement to be made is a static analysis check
(flow and context sensitive) to check the range of a value to be assigned to a
bitfield. This would be an additional check to bitfield-conversion, and could
be used to "vote" on the presence of a real defect or not. This may seem a bit
too pedantic, but it is what it is :)
Back to the interesting topic of bitfield-enum-conversion, you may know some
programmers use a style to define C enums where the last element of an enum is
the number of enums. An example looks like this...
`
1
2 enum myenum {
3 red,
4 blue,
5 numenums,
6 };
7
8 struct bits1 {
9 int bf:1;
10 };
11
12 struct bits1 xx;
13
14 int test(enum myenum value) {
15 xx.bf = value;
16 }
`
We see false positives related to this style, but this actually caught a real
bug where the bitfield was not large enough to contain the enum. The trouble is
suppressing the false positives - and this is certainly possible. It's also
possible to change the coding style to accommodate the compiler diagnostic
behavior.
But tying these two together,
https://github.com/llvm/llvm-project/pull/68276
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
