Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lint for nondeterministic pointer comparison #1054

Merged
merged 2 commits into from
Dec 19, 2024
Merged

Add lint for nondeterministic pointer comparison #1054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c5f33b
Add lint test for nondeterministic pointer comparison
nuchi Oct 2, 2024
87c523c
Fix submodule, fix plugin extension
nuchi Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitmodules
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,6 @@
[submodule "tools/physicalModeling/mesh2faust/spectra"]
path = tools/physicalModeling/mesh2faust/spectra
url = https://github.com/yixuan/spectra.git
[submodule "node-matcher-plugin"]
path = node-matcher-plugin
url = https://github.com/nuchi/node-matcher-plugin.git
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ remote : developer
$(MAKE) -C embedded/faustremote all

debug :
$(MAKE) -C $(BUILDLOCATION) FAUSTDIR=faustdebug CMAKEOPT=-DCMAKE_BUILD_TYPE=Debug
$(MAKE) -C $(BUILDLOCATION) FAUSTDIR=faustdebug RELEASE_TYPE=Debug NONDETERMINISM_LINT=yes
# $(MAKE) -C compiler debug -f $(MAKEFILE) prefix=$(prefix)

ioslib :
Expand Down
27 changes: 25 additions & 2 deletions build/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ option ( INCLUDE_WASM_GLUE "Include wasm glue targets" on )
option ( MSVC_STATIC "Use static runtimes with MSVC" off)
option ( SELF_CONTAINED_LIBRARY "Don't search system architecture files." off)
option ( LINK_LLVM_STATIC "Link LLVM statically" on )
option ( NONDETERMINISM_LINT "Add compiler warning for nondeterminism" off)

#######################################
# Check output options
Expand Down Expand Up @@ -172,20 +173,36 @@ endif()

####################################
# compiler dependent settings

if (NONDETERMINISM_LINT)
if(APPLE)
set (PLUGIN_EXT "dylib")
else()
set (PLUGIN_EXT "so")
endif()

set (EXTRA_CXX_FLAGS "-fplugin=node-matcher-plugin/NodeMatcherPlugin.${PLUGIN_EXT} -fplugin-arg-NodeMatcherPlugin-${CMAKE_CURRENT_SOURCE_DIR}/lint.query")
# Add NodeMatcherPlugin as a submodule
add_subdirectory(../node-matcher-plugin node-matcher-plugin)
endif()

if (MSVC)
set (FAUST_DEFINITIONS ${FAUST_DEFINITIONS} -DMSVisualStudio)
else()
set (FAUST_DEFINITIONS ${FAUST_DEFINITIONS} -DLIBDIR="${LIBSDIR}")
#set(CMAKE_CXX_FLAGS_DEBUG "-g -fsanitize=address -fno-omit-frame-pointer -fsanitize-address-use-after-scope -fvisibility=hidden -Wall -Wextra -Wno-unused-parameter -Wno-unused-function -Wno-overloaded-virtual -Wshadow")
set(CMAKE_CXX_FLAGS_DEBUG "-g -fvisibility=hidden -Wall -Wextra -Wno-unused-parameter -Wno-unused-function -Wno-overloaded-virtual -Wshadow")
set(CMAKE_CXX_FLAGS_RELEASE "-O3 -fvisibility=hidden -Wall -Wextra -Wno-unused-parameter -Wno-unused-function -Wno-overloaded-virtual")
set(CMAKE_CXX_FLAGS_DEBUG "${EXTRA_CXX_FLAGS} -g -fvisibility=hidden -Wall -Wextra -Wno-unused-parameter -Wno-unused-function -Wno-overloaded-virtual -Wshadow")
set(CMAKE_CXX_FLAGS_RELEASE "${EXTRA_CXX_FLAGS} -O3 -fvisibility=hidden -Wall -Wextra -Wno-unused-parameter -Wno-unused-function -Wno-overloaded-virtual")
endif()

####################################
# Add the different targets
####################################
if (INCLUDE_EXECUTABLE)
add_executable(faust ${SRC} ${HH})
if (NONDETERMINISM_LINT)
add_dependencies(faust NodeMatcherPlugin)
endif()
target_compile_definitions (faust PRIVATE ${FAUST_DEFINITIONS})
if(INCLUDE_LLVM)
target_link_directories(faust PRIVATE "${LLVM_LIB_DIR}")
Expand All @@ -201,6 +218,9 @@ endif()
# static faust library
if (INCLUDE_STATIC)
add_library(staticlib STATIC ${SRC} ${HH})
if (NONDETERMINISM_LINT)
add_dependencies(staticlib NodeMatcherPlugin)
endif()
target_compile_definitions (staticlib PRIVATE ${FAUST_DEFINITIONS})
if(INCLUDE_LLVM)
target_link_directories(staticlib PRIVATE "${LLVM_LIB_DIR}")
Expand Down Expand Up @@ -239,6 +259,9 @@ endif()
# dynamic faust library
if (INCLUDE_DYNAMIC)
add_library(dynamiclib SHARED ${SRC} ${HH})
if (NONDETERMINISM_LINT)
add_dependencies(dynamiclib NodeMatcherPlugin)
endif()
target_compile_definitions (dynamiclib PRIVATE ${FAUST_DEFINITIONS})
if(INCLUDE_LLVM)
target_link_directories(dynamiclib PRIVATE "${LLVM_LIB_DIR}")
Expand Down
21 changes: 16 additions & 5 deletions build/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,18 @@ ifeq ($(WORKLET), on)
WORKLETOPT = "-DWORKLET=on"
endif

CMAKEOPT ?= -DCMAKE_BUILD_TYPE=Release $(WORKLETOPT)
BUILDOPT ?= --config Release
NONDETERMINISM_LINT ?= no
NONDETERMINISM_LINT_OPT =
CXX_COMPILER_OPT =
ifeq ($(NONDETERMINISM_LINT), yes)
NONDETERMINISM_LINT_OPT = "-DNONDETERMINISM_LINT=ON"
CXX_COMPILER_OPT = "-DCMAKE_CXX_COMPILER=$(shell llvm-config --bindir)/clang++"
endif

RELEASE_TYPE ?= Release
CMAKEOPT ?=
BUILDOPT ?= --config $(RELEASE_TYPE)
CMAKEOPTS = -DCMAKE_BUILD_TYPE=$(RELEASE_TYPE) $(WORKLETOPT) $(CMAKEOPT) $(NONDETERMINISM_LINT_OPT) $(CXX_COMPILER_OPT)

#CMAKEOPT ?= -DCMAKE_BUILD_TYPE=Debug $(WORKLETOPT)
#BUILDOPT ?= --config Debug
Expand Down Expand Up @@ -136,7 +146,8 @@ help:
@echo " FAUSTDIR=<dir> : the compilation directory. Default to '$(FAUSTDIR)'"
@echo " LIBSDIR : the libraries destination directory, default: $(LIBSDIR)"
@echo " GENERATOR=<a cmake generator>: see cmake -h. Default to '$(GENERATOR)'"
@echo " CMAKEOPT=<cmake options> : pass options to cmake for project generation."
@echo " CMAKEOPT=<cmake options> : pass extra options to cmake for project generation."
@echo " RELEASE_TYPE : cmake release type (defaults to Release)."
@echo " BUILDOPT=<cmake options> : pass options to cmake at build time (default to $(BUILDOPT))."
@echo " BACKENDS=<backends file> : see 'Backends' below"
@echo " TARGETS=<targets file> : see 'Targets' below"
Expand Down Expand Up @@ -320,7 +331,7 @@ silent: $(FAUSTDIR)
cd $(FAUSTDIR) && $(CMAKE) -DCMAKE_VERBOSE_MAKEFILE=OFF ..

cmake: $(FAUSTDIR)
cd $(FAUSTDIR) && $(CMAKE) -C ../backends/$(BACKENDS) -C ../targets/$(TARGETS) $(CMAKEOPT) -DINCLUDE_LLVM=$(INCLUDE_LLVM) -DUSE_LLVM_CONFIG=$(USE_LLVM_CONFIG) -DLLVM_PACKAGE_VERSION=$(LLVM_PACKAGE_VERSION) -DLLVM_LIBS="$(LLVM_LIBS)" -DLLVM_LIB_DIR="$(LLVM_LIB_DIR)" -DLLVM_INCLUDE_DIRS="$(LLVM_INCLUDE_DIRS)" -DLLVM_DEFINITIONS="$(LLVM_DEFINITIONS)" -DLLVM_LD_FLAGS="$(LLVM_LD_FLAGS)" $(DEPLOYMENT) -DLIBSDIR=$(LIBSDIR) -DBUILD_HTTP_STATIC=$(BUILD_HTTP_STATIC) -G '$(GENERATOR)' ..
cd $(FAUSTDIR) && $(CMAKE) -C ../backends/$(BACKENDS) -C ../targets/$(TARGETS) $(CMAKEOPTS) -DCMAKE_BUILD_TYPE=$(RELEASE_TYPE) $(WORKLETOPT) -DINCLUDE_LLVM=$(INCLUDE_LLVM) -DUSE_LLVM_CONFIG=$(USE_LLVM_CONFIG) -DLLVM_PACKAGE_VERSION=$(LLVM_PACKAGE_VERSION) -DLLVM_LIBS="$(LLVM_LIBS)" -DLLVM_LIB_DIR="$(LLVM_LIB_DIR)" -DLLVM_INCLUDE_DIRS="$(LLVM_INCLUDE_DIRS)" -DLLVM_DEFINITIONS="$(LLVM_DEFINITIONS)" -DLLVM_LD_FLAGS="$(LLVM_LD_FLAGS)" $(DEPLOYMENT) -DLIBSDIR=$(LIBSDIR) -DBUILD_HTTP_STATIC=$(BUILD_HTTP_STATIC) -G '$(GENERATOR)' ..
@echo BACKENDS=$(BACKENDS) > $(BCACHE)
@echo TARGETS=$(TARGETS) > $(TCACHE)
@echo LIBSDIR=$(LIBSDIR) > $(LCACHE)
Expand Down Expand Up @@ -353,7 +364,7 @@ checkemcc:
installLog := $(FAUSTDIR)/install_manifest.txt
install:
if test -d ../.git; then git submodule update --init; fi
cd $(FAUSTDIR) && $(CMAKE) .. -DCMAKE_INSTALL_PREFIX=$(PREFIX) $(CMAKEOPT)
cd $(FAUSTDIR) && $(CMAKE) .. -DCMAKE_INSTALL_PREFIX=$(PREFIX) $(CMAKEOPTS)
$(CMAKE) --build $(FAUSTDIR) --target install

uninstall: $(installLog)
Expand Down
72 changes: 72 additions & 0 deletions build/lint.query
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# Check for a pointer comparison between CTreeBase* / Symbol*

let hasTargetComparisonNoBind binaryOperator(
hasAnyOperatorName("<", ">", "<=", ">="),
hasEitherOperand(hasType(hasUnqualifiedDesugaredType(pointerType(pointee(hasDeclaration(namedDecl(hasAnyName("CTreeBase", "Symbol"))))).bind("pointer-type"))))
)

# Or a function call to a function in the STL which has one of those pointer comparisons

let hasTargetComparison hasTargetComparisonNoBind.bind("comparison")

let hasTransitiveTargetCall0 callExpr(callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTargetComparison))))

# Or nested function calls. We can't do arbitrary recursion, so just go 8 deep

let hasTransitiveTargetCall1 callExpr(anyOf(
callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTargetComparison)))
, callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTransitiveTargetCall0)))
))

let hasTransitiveTargetCall2 callExpr(anyOf(
callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTargetComparison)))
, callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTransitiveTargetCall1)))
))

let hasTransitiveTargetCall3 callExpr(anyOf(
callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTargetComparison)))
, callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTransitiveTargetCall2)))
))

let hasTransitiveTargetCall4 callExpr(anyOf(
callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTargetComparison)))
, callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTransitiveTargetCall3)))
))

let hasTransitiveTargetCall5 callExpr(anyOf(
callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTargetComparison)))
, callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTransitiveTargetCall4)))
))

let hasTransitiveTargetCall6 callExpr(anyOf(
callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTargetComparison)))
, callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTransitiveTargetCall5)))
))

let hasTransitiveTargetCall7 callExpr(anyOf(
callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTargetComparison)))
, callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTransitiveTargetCall6)))
))

let hasTransitiveTargetCall8 callExpr(anyOf(
callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTargetComparison)))
, callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTransitiveTargetCall7)))
))

let hasTransitiveTargetCall hasTransitiveTargetCall8

let Diagnostic "this function call leads to pointer comparison of type %1, don't do that"
let DiagnosticArgs "root pointer-type"

# Match a function call in a source file (either the .cpp or an included .hh)
# whose definition is in a system header
match callExpr(
unless(isExpansionInSystemHeader()),
callee(decl(isExpansionInSystemHeader(), hasDescendant(hasTransitiveTargetCall)))
)

let Diagnostic "don't compare pointers of type %1"
let DiagnosticArgs "root pointer-type"

# Match a pointer comparison in a source file (either .cpp or included .hh)
match stmt(unless(isExpansionInSystemHeader()), hasTargetComparisonNoBind)
1 change: 1 addition & 0 deletions node-matcher-plugin
Open in desktop
Submodule node-matcher-plugin added at 08fcc1
Loading