grame-cncm / faust

Functional programming language for signal processing and sound synthesis
http://faust.grame.fr
Other
2.59k stars 323 forks source link

Add lint for nondeterministic pointer comparison #1054

Open nuchi opened 1 month ago

nuchi commented 1 month ago

@sletz I figured out a CMake integration for the pointer nondeterminism lint I mentioned to you. I did it by wrapping clang-query into a clang plugin, so that a query can be automatically converted into a compiler warning. The plugin is at https://github.com/nuchi/node-matcher-plugin

(Context for others: code generation is nondeterministic because sometimes the compiler will compare pointers in situations where it needs to arbitrarily choose one. But pointers are generated by malloc/new and therefore change from run to run. One way to eliminate this non-determinism is to not compare pointers. But that can be hard to detect. Hence this new lint step.)

Adds a compiler warning for doing a pointer comparison between CTreeBase* or Symbol*, in order to better find where nondeterminism is happening.

It's enabled automatically for make debug, and can be enabled for other targets by doing make compiler NONDETERMINISM_LINT=yes

Needs llvm-config to be in one's PATH; it'll set the C++ compiler to be the version of clang that's shipped with LLVM. (It builds a clang plugin against the installed LLVM, so it needs to use the same C++ compiler.)

Sample cmake output:

[  1%] Building CXX object CMakeFiles/faust.dir/Users/nuchi/faust/compiler/box_signal_api.cpp.o
In file included from /Users/nuchi/faust/compiler/box_signal_api.cpp:30:
In file included from /Users/nuchi/faust/compiler/global.hh:39:
In file included from /Users/nuchi/faust/compiler/evaluate/loopDetector.hh:33:
In file included from /Users/nuchi/faust/compiler/boxes/boxes.hh:42:
In file included from /Users/nuchi/faust/compiler/tlib/tlib.hh:162:
In file included from /Users/nuchi/faust/compiler/tlib/list.hh:108:
/Users/nuchi/faust/compiler/tlib/tree.hh:181:46: warning: this function call leads to pointer comparison of type CTreeBase *, don't do that
  181 |     void setProperty(Tree key, Tree value) { fProperties[key] = value; }
      |                                              ^~~~~~~~~~~~~~~~
/Users/nuchi/faust/compiler/tlib/tree.hh:182:36: warning: this function call leads to pointer comparison of type CTreeBase *, don't do that
  182 |     void clearProperty(Tree key) { fProperties.erase(key); }
      |                                    ^~~~~~~~~~~~~~~~~~~~~~
/Users/nuchi/faust/compiler/tlib/tree.hh:189:29: warning: this function call leads to pointer comparison of type CTreeBase *, don't do that
  189 |         plist::iterator i = fProperties.find(key);
      |                             ^~~~~~~~~~~~~~~~~~~~~
/Users/nuchi/faust/compiler/tlib/tree.hh:221:22: warning: don't compare pointers of type CTreeBase *
  221 |             } while (new_block < gAllocatedBlock);
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
4 warnings generated.

Note that it'll be very noisy, because it'll give the same warning for e.g. tree.hh in every translation unit that includes it.

sletz commented 1 month ago

Thanks, I'll have a look.