Re: Your clang TSA patches
On Tue, 9 May 2023 at 03:01, Marc-André Lureau wrote: > > Hi Stefan > > On Mon, May 8, 2023 at 6:54 PM Stefan Hajnoczi wrote: >> >> Hi Marc-André, >> clang's Thread Safety Analysis is being used more in the QEMU block >> layer and a few limitations have been hit: >> 1. Mutexes that are struct fields are not supported (only global mutexes >> work). >> 2. Analysis does not extend across function pointers. See >> GRAPH_RDLOCK_PTR for a workaround that is currently used. >> >> (There was a third limitation that I don't remember. Maybe Kevin remembers.) >> >> I heard you had clang or LLVM patches in that past that improve TSA. >> Did your patches address these things? Any idea how hard it is to fix >> these limitations? > > > > I don't remember much about that work, I didn't spend that much time on it, > and it was 5y ago already! > > My WIP branch is still available: > https://github.com/elmarco/clang/commits/qemu-ta > > it seems I was trying to modify clang to support TSA annotations on typedef > and function pointers, and warn on unmatching. I have no idea if this is > still relevant. I remember dropping the effort at that time because of lack > of interest or help in both projects. Thanks! The changes seem small enough that it may be possible to get them merged without a major time commitment. It seems the function pointer analysis limitation still exists after these 5 years :). Stefan
Re: Your clang TSA patches
Am 09.05.2023 um 09:00 hat Marc-André Lureau geschrieben: > Hi Stefan > > On Mon, May 8, 2023 at 6:54 PM Stefan Hajnoczi wrote: > > > Hi Marc-André, > > clang's Thread Safety Analysis is being used more in the QEMU block > > layer and a few limitations have been hit: > > 1. Mutexes that are struct fields are not supported (only global mutexes > > work). > > 2. Analysis does not extend across function pointers. See > > GRAPH_RDLOCK_PTR for a workaround that is currently used. > > > > (There was a third limitation that I don't remember. Maybe Kevin > > remembers.) > > > > I heard you had clang or LLVM patches in that past that improve TSA. > > Did your patches address these things? Any idea how hard it is to fix > > these limitations? > > I don't remember much about that work, I didn't spend that much time on it, > and it was 5y ago already! > > My WIP branch is still available: > https://github.com/elmarco/clang/commits/qemu-ta > > it seems I was trying to modify clang to support TSA annotations on > typedef and function pointers, and warn on unmatching. I have no idea > if this is still relevant. I remember dropping the effort at that time > because of lack of interest or help in both projects. TSA support for functions pointers would be very helpful for the work we're currently doing in the context of multiqueue support. Function pointers are essentially where the compiler checks break down for us and we have to cover these parts manually during review. And we have function pointers in almost every call path (mostly those in BlockDriver these days, but also some callbacks passed to functions, like BHs). Does "unmatching" mean that it checks that the TSA annotations in prototypes and the actual function declaration agree? If so, that would be nice, too. For now, I've settled on annotating only the header file for public functions, which works, but it's harder to read. The third limitation Stefan couldn't remember is lack of support for __attribute__((cleanup)), which is used by the lock guard macros. This one is pretty annoying and means that I'm using the guards less than I would like. Stefan's first point, not supporting mutexes in struct fields, is a major limitation of TSA with C, too, but it doesn't actually affect the current work in QEMU where we're dealing with a single global lock. Of course, if it were supported, we would certainly have uses for that, too. Kevin
Re: Your clang TSA patches
Hi Stefan On Mon, May 8, 2023 at 6:54 PM Stefan Hajnoczi wrote: > Hi Marc-André, > clang's Thread Safety Analysis is being used more in the QEMU block > layer and a few limitations have been hit: > 1. Mutexes that are struct fields are not supported (only global mutexes > work). > 2. Analysis does not extend across function pointers. See > GRAPH_RDLOCK_PTR for a workaround that is currently used. > > (There was a third limitation that I don't remember. Maybe Kevin > remembers.) > > I heard you had clang or LLVM patches in that past that improve TSA. > Did your patches address these things? Any idea how hard it is to fix > these limitations? > I don't remember much about that work, I didn't spend that much time on it, and it was 5y ago already! My WIP branch is still available: https://github.com/elmarco/clang/commits/qemu-ta it seems I was trying to modify clang to support TSA annotations on typedef and function pointers, and warn on unmatching. I have no idea if this is still relevant. I remember dropping the effort at that time because of lack of interest or help in both projects. -- Marc-André Lureau
Your clang TSA patches
Hi Marc-André, clang's Thread Safety Analysis is being used more in the QEMU block layer and a few limitations have been hit: 1. Mutexes that are struct fields are not supported (only global mutexes work). 2. Analysis does not extend across function pointers. See GRAPH_RDLOCK_PTR for a workaround that is currently used. (There was a third limitation that I don't remember. Maybe Kevin remembers.) I heard you had clang or LLVM patches in that past that improve TSA. Did your patches address these things? Any idea how hard it is to fix these limitations? Thanks, Stefan
