On Wed, Nov 08, 2023 at 09:03:34PM -0800, Christoph Hellwig wrote: > Hi dear spearse developers, > > in a lot of kernel code we have integer types that store offsets and > length in certain units (typically 512 byte disk "sectors", file systems > block sizes, and some weird variations of the same), and we had a fair > amount of bugs beause people get confused about which ones to use. > > I wonder if it is possible to add an attribute (say nocast) that works > similar to __attribute__((bitwise)) in that it disallows mixing this > type with other integer types, but unlike __attribute__((bitwise)) > allows all the normal arithmetics on it? That way we could annotate > all the normal conversion helpers with __force overrides and check > where people are otherwise mixing these types. I started writing something like this in Smatch for tying variables to a specific unit. https://github.com/error27/smatch/blob/master/smatch_units.c But unfortunately, it doesn't actually work. The problem is that once I said x is a byte, then if you have y = x then I would store that in the database. If the first "x is a byte" assessment was wrong then the misinformation gets amplified times 100 and can't be purged without a mass delete. The second problem is that Smatch automatically determines that a struct foo->bar is a byte unit or whatever. Which generally works, but sometimes fails catastrophically. For example, it's not true to all the registers are used to store byte units. But some code does store bytes there and now Smatch thinks the everything stored in registers is in bytes. My plan was to go through the false positives and manually edit out stuff like this. The problem is that it's a lot of work and I haven't done it. I did a similar thing for tracking user data and that works pretty decently these days. So it's doable. I tend to avoid manual annotations, but here it could be good. Manually annotating things would avoid the false positives (at the expense of missing bugs). I'd prefer an annotation that had the type of the unit built in. Creating an annotation like that is difficult because you have to coordinate with GCC and Clang etc. In the mean time, I could just create a table in smatch which has stuff like: { "(struct foo)->member", &byte_units }, regards, dan carpenter
