You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
"__VERIFIER_nondet_int", readsAll; (* no args, declare invalidate actions to prevent invalidating globals when extern in regression tests *)
(* no args, declare invalidate actions to prevent invalidating globals *)
"__VERIFIER_atomic_begin", readsAll;
"__VERIFIER_atomic_end", readsAll;
(* prevent base from spawning ARINC processes early, handled by arinc/extract_arinc *)
(* "LAP_Se_SetPartitionMode", writes [2]; *)
"LAP_Se_CreateProcess", writes [2; 3];
"LAP_Se_CreateErrorHandler", writes [2; 3];
"isatty", readsAll;
"setpriority", readsAll;
"getpriority", readsAll;
(* ddverify *)
"spin_lock_init", readsAll;
"spin_lock", readsAll;
"spin_unlock", readsAll;
"spin_unlock_irqrestore", readsAll;
"spin_lock_irqsave", readsAll;
"sema_init", readsAll;
"down_trylock", readsAll;
"up", readsAll;
"ZSTD_customFree", frees [1]; (* only used with extraspecials *)
]
There is duplication between the two and to avoid confusion there must be duplication. If a function is in classify, but not invalidate_actions, then analyses can handle it according to the classification, but base analysis still prints an error about missing function definition and starts invalidating things.
Specifying classify cases using pattern matching is relatively verbose. Five lines of code are used to match one function and its arguments:
invalidate_actions specifications like readsAll, writesAll, etc. are too broad to specify behavior sufficiently precisely. For example:
Write is also used to decide, which arguments need to be spawned as threads. This is almost always unnecessary because C standard library functions don't spawn threads (except for C11 threads, which we don't support yet anyway).
All accesses are assumed to be transitive/deep, i.e. they follow pointers. Again, this is almost never the case for C standard library functions (e.g. free doesn't recursively first free data pointed to inside the outer struct, it only deallocates the direct memory and it's the programmer's job to free inner data structures first).
Shallow accessing is currently enforced by another duplicate match in an analysis (separately):
"realloc", readsFrees [0; 1] [0]; (* read+free first argument, read second argument *)
Argument numbering is error prone with multiple similar arguments. For example: 7e5ab0a.
Since classify is a pattern match, all of the possible cases must be hard-coded together in one place. So all Linux kernel, etc specific library functions must also be there.
Goals
To address all of the above, a new approach should fulfill the following goals:
All specifications relating to one function should be together in one place to avoid duplication and need for synchronization.
Matching the arguments for constructing the classify result shouldn't require any fallback boilerplate per-function. (First-class patterns can achieve this)
Invalidate actions should be specified in the same "pattern" which matches the arguments for classification, essentially "annotating" the corresponding arguments being matched. This avoids any need for specification by argument indices.
Invalidate actions per-argument should be specified by a set of access kinds.
A Spawn access kind should be added.
All access kinds should be usable together with a shallow vs deep flag.
Arguments should be (optionally) nameable to allow distinguishing similar arguments at glance without having to look up the function every time.
Recently during interactive benchmarking, it has really bothered me how messy and inflexible our
LibraryFunctions
specifications areProblems
LibraryFunctions
:classify
is for classifying some functions into particular categories for uniform handling by analyses:analyzer/src/analyses/libraryFunctions.ml
Lines 8 to 98 in fb54fa5
invalidate_actions
is for specifying what base invalidation and race detection do with arguments:analyzer/src/analyses/libraryFunctions.ml
Lines 192 to 518 in fb54fa5
classify
, but notinvalidate_actions
, then analyses can handle it according to the classification, but base analysis still prints an error about missing function definition and starts invalidating things.classify
cases using pattern matching is relatively verbose. Five lines of code are used to match one function and its arguments:analyzer/src/analyses/libraryFunctions.ml
Lines 27 to 31 in fb54fa5
invalidate_actions
specifications likereadsAll
,writesAll
, etc. are too broad to specify behavior sufficiently precisely. For example:Write
is also used to decide, which arguments need to be spawned as threads. This is almost always unnecessary because C standard library functions don't spawn threads (except for C11 threads, which we don't support yet anyway).Free
was added in Add option for ignoring races fromfree
#695 to be able to distinguish memory freeing fromWrite
.free
doesn't recursively first free data pointed to inside the outer struct, it only deallocates the direct memory and it's the programmer's job to free inner data structures first).analyzer/src/analyses/accessAnalysis.ml
Lines 190 to 197 in fb54fa5
invalidate_actions
specifications use inconsistent argument numbering:analyzer/src/analyses/libraryFunctions.ml
Line 219 in fb54fa5
analyzer/src/analyses/libraryFunctions.ml
Line 493 in fb54fa5
invalidate_actions
specifications are unintuitively per access kind, not per argument:analyzer/src/analyses/libraryFunctions.ml
Line 461 in a8fd1c5
classify
is a pattern match, all of the possible cases must be hard-coded together in one place. So all Linux kernel, etc specific library functions must also be there.Goals
To address all of the above, a new approach should fulfill the following goals:
classify
result shouldn't require any fallback boilerplate per-function. (First-class patterns can achieve this)Spawn
access kind should be added.The text was updated successfully, but these errors were encountered: