From c2fb46afa268141f6fa9adbd4c3de262fe6fb6c5 Mon Sep 17 00:00:00 2001 From: zerbina <100542850+zerbina@users.noreply.github.com> Date: Sat, 13 Apr 2024 17:23:15 +0000 Subject: [PATCH 1/3] add the `usesValgrind` conditional symbol Valgrind-specific code is only included in the build if it's present. `usesValgrind` is defined for the files in the test director. --- actors/valgrind.nim | 17 +++++++++++++---- tests/nim.cfg | 1 + 2 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 tests/nim.cfg diff --git a/actors/valgrind.nim b/actors/valgrind.nim index 565df1d..31c3ee2 100644 --- a/actors/valgrind.nim +++ b/actors/valgrind.nim @@ -1,20 +1,29 @@ +const + usesValgrind {.booldefine.} = false + {.emit:"#include ".} template valgrind_annotate_happens_before*(x) = block: let y {.exportc,inject.} = x - {.emit:"ANNOTATE_HAPPENS_BEFORE(y);".} + when usesValgrind: + {.emit:"ANNOTATE_HAPPENS_BEFORE(y);".} template valgrind_annotate_happens_after*(x) = block: let y {.exportc,inject.} = x - {.emit:"ANNOTATE_HAPPENS_AFTER(y);".} + when usesValgrind: + {.emit:"ANNOTATE_HAPPENS_AFTER(y);".} template valgrind_annotate_happens_before_forget_all*(x) = block: let y {.exportc,inject.} = x - {.emit:"ANNOTATE_HAPPENS_BEFORE_FORGET_ALL(y);".} + when usesValgrind: + {.emit:"ANNOTATE_HAPPENS_BEFORE_FORGET_ALL(y);".} proc running_on_valgrind*(): bool = - {.emit: "result = RUNNING_ON_VALGRIND;".} + when usesValgrind: + {.emit: "result = RUNNING_ON_VALGRIND;".} + else: + false diff --git a/tests/nim.cfg b/tests/nim.cfg new file mode 100644 index 0000000..fdc9b27 --- /dev/null +++ b/tests/nim.cfg @@ -0,0 +1 @@ +--define:usesValgrind \ No newline at end of file From 20285d54f938ace01e37e9a091a3f1adf9fe5f19 Mon Sep 17 00:00:00 2001 From: zerbina <100542850+zerbina@users.noreply.github.com> Date: Sat, 13 Apr 2024 17:23:15 +0000 Subject: [PATCH 2/3] don't use `.emit` for interfacing with valgrind Use `.importc` procedures and constants instead. --- actors/pool.nim | 2 -- actors/valgrind.nim | 39 ++++++++++++++++++--------------------- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/actors/pool.nim b/actors/pool.nim index 4c1ca10..918bf38 100644 --- a/actors/pool.nim +++ b/actors/pool.nim @@ -6,8 +6,6 @@ import isisolated import mallinfo import valgrind -{.emit:"#include ".} - type Pool* = object diff --git a/actors/valgrind.nim b/actors/valgrind.nim index 31c3ee2..6a9b3c6 100644 --- a/actors/valgrind.nim +++ b/actors/valgrind.nim @@ -2,28 +2,25 @@ const usesValgrind {.booldefine.} = false -{.emit:"#include ".} +when usesValgrind: + const header = "" -template valgrind_annotate_happens_before*(x) = - block: - let y {.exportc,inject.} = x - when usesValgrind: - {.emit:"ANNOTATE_HAPPENS_BEFORE(y);".} + proc valgrind_annotate_happens_before*(x: pointer) {. + header: header, importc: "ANNOTATE_HAPPENS_BEFORE".} + proc valgrind_annotate_happens_after*(x: pointer) {. + header: header, importc: "ANNOTATE_HAPPENS_AFTER".} + proc valgrind_annotate_happens_before_forget_all*(x: pointer) {. + header: header, importc: "ANNOTATE_HAPPENS_BEFORE_FORGET_ALL".} -template valgrind_annotate_happens_after*(x) = - block: - let y {.exportc,inject.} = x - when usesValgrind: - {.emit:"ANNOTATE_HAPPENS_AFTER(y);".} + let enabled {.header: header, importc: "RUNNING_ON_VALGRIND".}: bool -template valgrind_annotate_happens_before_forget_all*(x) = - block: - let y {.exportc,inject.} = x - when usesValgrind: - {.emit:"ANNOTATE_HAPPENS_BEFORE_FORGET_ALL(y);".} + proc running_on_valgrind*(): bool = + {.cast(noSideEffect), cast(gcSafe).}: + result = enabled -proc running_on_valgrind*(): bool = - when usesValgrind: - {.emit: "result = RUNNING_ON_VALGRIND;".} - else: - false +else: + proc valgrind_annotate_happens_before*(x: pointer) = discard + proc valgrind_annotate_happens_after*(x: pointer) = discard + proc valgrind_annotate_happens_before_forget_all*(x: pointer) = discard + + template running_on_valgrind*(): bool = false From 037fbf878f254e70b2e8fa852e61a2549672ccb8 Mon Sep 17 00:00:00 2001 From: zerbina <100542850+zerbina@users.noreply.github.com> Date: Sat, 13 Apr 2024 17:23:16 +0000 Subject: [PATCH 3/3] mention the define in the readme --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index 876491f..5c4238f 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,12 @@ for all the actors to terminate and clean up all the resources used. +### Valgrind + +To make the library aware of being run with helgrind, the program needs to be +built with `-d:usesValgrind`, otherwise false positives could be reported. + + ### More to come Sure.