github/verilator
1
0
Fork 1
mirror of https://github.com/verilator/verilator.git synced 2025-01-01 04:07:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

Internals: update clang attributes check report conditions (#3997)

Browse Source
This commit is contained in:
Kamil Rakoczy 2023-03-03 00:37:07 +01:00 committed by GitHub
parent 8a5804fc3a
commit 0130c2bceb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 245 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
include/verilated.cpp
View File

@ -2982,7 +2982,7 @@ VerilatedModule::~VerilatedModule() {
// VerilatedVar:: Methods
// cppcheck-suppress unusedFunction // Used by applications
uint32_t VerilatedVarProps::entSize() const {
uint32_t VerilatedVarProps::entSize() const VL_MT_SAFE {
uint32_t size = 1;
switch (vltype()) {
case VLVT_PTR: size = sizeof(void*); break;
@ -3002,7 +3002,7 @@ size_t VerilatedVarProps::totalSize() const {
return size;
}
void* VerilatedVarProps::datapAdjustIndex(void* datap, int dim, int indx) const {
void* VerilatedVarProps::datapAdjustIndex(void* datap, int dim, int indx) const VL_MT_SAFE {
if (VL_UNLIKELY(dim <= 0 || dim > udims())) return nullptr;
if (VL_UNLIKELY(indx < low(dim) || indx > high(dim))) return nullptr;
const int indxAdj = indx - low(dim);
@ -3121,7 +3121,7 @@ void* VerilatedScope::exportFindNullError(int funcnum) VL_MT_SAFE {
return nullptr;
}
void* VerilatedScope::exportFindError(int funcnum) const {
void* VerilatedScope::exportFindError(int funcnum) const VL_MT_SAFE {
// Slowpath - Called only when find has failed
const std::string msg
= (std::string{"Testbench C called '"} + VerilatedImp::exportName(funcnum)

2
include/verilated.h
View File

@ -647,7 +647,7 @@ public: // But internals only - called from VerilatedModule's
VerilatedVar* varFind(const char* namep) const VL_MT_SAFE_POSTINIT;
VerilatedVarNameMap* varsp() const VL_MT_SAFE_POSTINIT { return m_varsp; }
void scopeDump() const;
void* exportFindError(int funcnum) const;
void* exportFindError(int funcnum) const VL_MT_SAFE;
static void* exportFindNullError(int funcnum) VL_MT_SAFE;
static void* exportFind(const VerilatedScope* scopep, int funcnum) VL_MT_SAFE {
if (VL_UNLIKELY(!scopep)) return exportFindNullError(funcnum);

30
include/verilated_sym_props.h
View File

@ -137,35 +137,35 @@ public:
~VerilatedVarProps() = default;
// METHODS
bool magicOk() const { return m_magic == MAGIC; }
VerilatedVarType vltype() const { return m_vltype; }
VerilatedVarType vltype() const VL_MT_SAFE { return m_vltype; }
VerilatedVarFlags vldir() const {
return static_cast<VerilatedVarFlags>(static_cast<int>(m_vlflags) & VLVF_MASK_DIR);
}
uint32_t entSize() const;
uint32_t entSize() const VL_MT_SAFE;
bool isPublicRW() const { return ((m_vlflags & VLVF_PUB_RW) != 0); }
// DPI compatible C standard layout
bool isDpiCLayout() const { return ((m_vlflags & VLVF_DPI_CLAY) != 0); }
int udims() const { return m_udims; }
int udims() const VL_MT_SAFE { return m_udims; }
int dims() const { return m_pdims + m_udims; }
const VerilatedRange& packed() const { return m_packed; }
const VerilatedRange& packed() const VL_MT_SAFE { return m_packed; }
const VerilatedRange& unpacked() const { return m_unpacked[0]; }
// DPI accessors
int left(int dim) const {
int left(int dim) const VL_MT_SAFE {
return dim == 0 ? m_packed.left()
: VL_LIKELY(dim >= 1 && dim <= udims()) ? m_unpacked[dim - 1].left()
: 0;
}
int right(int dim) const {
int right(int dim) const VL_MT_SAFE {
return dim == 0 ? m_packed.right()
: VL_LIKELY(dim >= 1 && dim <= udims()) ? m_unpacked[dim - 1].right()
: 0;
}
int low(int dim) const {
int low(int dim) const VL_MT_SAFE {
return dim == 0 ? m_packed.low()
: VL_LIKELY(dim >= 1 && dim <= udims()) ? m_unpacked[dim - 1].low()
: 0;
}
int high(int dim) const {
int high(int dim) const VL_MT_SAFE {
return dim == 0 ? m_packed.high()
: VL_LIKELY(dim >= 1 && dim <= udims()) ? m_unpacked[dim - 1].high()
: 0;
@ -175,7 +175,7 @@ public:
: VL_LIKELY(dim >= 1 && dim <= udims()) ? m_unpacked[dim - 1].increment()
: 0;
}
int elements(int dim) const {
int elements(int dim) const VL_MT_SAFE {
return dim == 0 ? m_packed.elements()
: VL_LIKELY(dim >= 1 && dim <= udims()) ? m_unpacked[dim - 1].elements()
: 0;
@ -183,7 +183,7 @@ public:
// Total size in bytes (note DPI limited to 4GB)
size_t totalSize() const;
// Adjust a data pointer to access a given array element, NULL if something goes bad
void* datapAdjustIndex(void* datap, int dim, int indx) const;
void* datapAdjustIndex(void* datap, int dim, int indx) const VL_MT_SAFE;
};
//===========================================================================
@ -203,22 +203,22 @@ public:
, m_datap{const_cast<void*>(datap)} {}
~VerilatedDpiOpenVar() = default;
// METHODS
void* datap() const { return m_datap; }
void* datap() const VL_MT_SAFE { return m_datap; }
// METHODS - from VerilatedVarProps
bool magicOk() const { return m_propsp->magicOk(); }
VerilatedVarType vltype() const { return m_propsp->vltype(); }
bool isDpiStdLayout() const { return m_propsp->isDpiCLayout(); }
const VerilatedRange& packed() const { return m_propsp->packed(); }
const VerilatedRange& unpacked() const { return m_propsp->unpacked(); }
int udims() const { return m_propsp->udims(); }
int left(int dim) const { return m_propsp->left(dim); }
int right(int dim) const { return m_propsp->right(dim); }
int udims() const VL_MT_SAFE { return m_propsp->udims(); }
int left(int dim) const VL_MT_SAFE { return m_propsp->left(dim); }
int right(int dim) const VL_MT_SAFE { return m_propsp->right(dim); }
int low(int dim) const { return m_propsp->low(dim); }
int high(int dim) const { return m_propsp->high(dim); }
int increment(int dim) const { return m_propsp->increment(dim); }
int elements(int dim) const { return m_propsp->elements(dim); }
size_t totalSize() const { return m_propsp->totalSize(); }
void* datapAdjustIndex(void* datap, int dim, int indx) const {
void* datapAdjustIndex(void* datap, int dim, int indx) const VL_MT_SAFE {
return m_propsp->datapAdjustIndex(datap, dim, indx);
}
};

138
nodist/clang_check_attributes
View File

@ -239,6 +239,7 @@ class CallAnnotationsValidator:
self._call_location: Optional[FunctionInfo] = None
self._caller: Optional[FunctionInfo] = None
self._level: int = 0
self._constructor_context: int = 0
def compile_and_analyze_file(self, source_file: str,
compiler_args: list[str],
@ -300,6 +301,67 @@ class CallAnnotationsValidator:
annotations = VlAnnotations.from_nodes_list(children)
return (True, refd, annotations, children)
def check_mt_safe_call(self, node: clang.cindex.Cursor,
refd: clang.cindex.Cursor,
annotations: VlAnnotations):
is_mt_safe = False
if annotations.is_mt_safe_call():
is_mt_safe = True
elif not annotations.is_mt_unsafe_call():
# Check whether the object the method is called on is mt-safe
def find_object_ref(node):
try:
node = next(node.get_children())
if node.kind == CursorKind.DECL_REF_EXPR:
# Operator on an argument or local object
return node
if node.kind != CursorKind.MEMBER_REF_EXPR:
return None
if node.referenced and node.referenced.kind == CursorKind.FIELD_DECL:
# Operator on a member object
return node
node = next(node.get_children())
if node.kind == CursorKind.UNEXPOSED_EXPR:
node = next(node.get_children())
return node
except StopIteration:
return None
refn = find_object_ref(node)
if self._constructor_context and not refn:
# we are in constructor and no object reference means
# we are calling local method. It is MT safe
# only if this method is also only calling local methods or
# MT-safe methods
self.iterate_children(refd.get_children(),
self.dispatch_node_inside_definition)
is_mt_safe = True
# class/struct member
elif refn and refn.kind == CursorKind.MEMBER_REF_EXPR and refn.referenced:
refn = refn.referenced
refna = VlAnnotations.from_nodes_list(refn.get_children())
if refna.guarded or self._constructor_context:
is_mt_safe = True
# variable
elif refn and refn.kind == CursorKind.DECL_REF_EXPR and refn.referenced:
# This is probably a local or an argument.
# Calling methods on local pointers or references is MT-safe,
# but on argument pointers or references is not.
if "*" not in refn.type.spelling and "&" not in refn.type.spelling:
is_mt_safe = True
# local variable
if refn.referenced.kind == CursorKind.VAR_DECL:
is_mt_safe = True
elif refn and refn.kind == CursorKind.CALL_EXPR:
if self._constructor_context:
# call to local function from constructor context
# safe if this function also calling local methods or
# MT-safe methods
self.dispatch_call_node(refn)
is_mt_safe = True
return is_mt_safe
# Call handling
def process_method_call(self, node: clang.cindex.Cursor,
@ -310,43 +372,7 @@ class CallAnnotationsValidator:
# MT-safe context
if ctx.is_mt_safe_context():
is_mt_safe = False
if annotations.is_mt_safe_call():
is_mt_safe = True
elif not annotations.is_mt_unsafe_call():
# Check whether the object the method is called on is mt-safe
def find_object_ref(node):
try:
node = next(node.get_children())
if node.kind == CursorKind.DECL_REF_EXPR:
# Operator on an argument or local object
return node
if node.kind != CursorKind.MEMBER_REF_EXPR:
return None
if node.referenced and node.referenced.kind == CursorKind.FIELD_DECL:
# Operator on a member object
return node
node = next(node.get_children())
if node.kind == CursorKind.UNEXPOSED_EXPR:
node = next(node.get_children())
return node
except StopIteration:
return None
refn = find_object_ref(node)
# class/struct member
if refn and refn.kind == CursorKind.MEMBER_REF_EXPR and refn.referenced:
refn = refn.referenced
refna = VlAnnotations.from_nodes_list(refn.get_children())
if refna.guarded:
is_mt_safe = True
# variable
elif refn and refn.kind == CursorKind.DECL_REF_EXPR and refn.referenced:
# This is probably a local or an argument. Assume it's safe.
is_mt_safe = True
if not is_mt_safe:
if not self.check_mt_safe_call(node, refd, annotations):
self.emit_diagnostic(
FunctionInfo.from_node(refd, refd, annotations),
DiagnosticKind.NON_MT_SAFE_CALL_IN_MT_SAFE_CTX)
@ -380,7 +406,13 @@ class CallAnnotationsValidator:
assert self._call_location
ctx = self._call_location.annotations
# Constructors are always OK in MT-safe context.
# Constructors are OK in MT-safe context
# only if they call local methods or MT-safe functions.
if ctx.is_mt_safe_context() or self._constructor_context:
self._constructor_context += 1
self.iterate_children(refd.get_children(),
self.dispatch_node_inside_definition)
self._constructor_context -= 1
# pure context
if ctx.is_pure_context():
@ -412,6 +444,8 @@ class CallAnnotationsValidator:
or refd.kind == CursorKind.CXX_METHOD
and refd.is_static_method()):
self.process_function_call(refd, annotations)
self.iterate_children(node.get_children(),
self.dispatch_node_inside_definition)
return
# Function pointer
if refd.kind in [
@ -419,18 +453,26 @@ class CallAnnotationsValidator:
CursorKind.PARM_DECL
]:
self.process_function_call(refd, annotations)
self.iterate_children(node.get_children(),
self.dispatch_node_inside_definition)
return
# Non-static class methods
if refd.kind == CursorKind.CXX_METHOD:
self.process_method_call(node, refd, annotations)
self.iterate_children(node.get_children(),
self.dispatch_node_inside_definition)
return
# Conversion method (e.g. `operator int()`)
if refd.kind == CursorKind.CONVERSION_FUNCTION:
self.process_method_call(node, refd, annotations)
self.iterate_children(node.get_children(),
self.dispatch_node_inside_definition)
return
# Constructors
if refd.kind == CursorKind.CONSTRUCTOR:
self.process_constructor_call(refd, annotations)
self.iterate_children(node.get_children(),
self.dispatch_node_inside_definition)
return
# Ignore other callables
@ -461,9 +503,6 @@ class CallAnnotationsValidator:
assert refd is not None
prev_caller = self._caller
prev_call_location = self._call_location
def_annotations = VlAnnotations.from_nodes_list(node_children)
if not (def_annotations.is_empty() or def_annotations == annotations):
@ -485,8 +524,7 @@ class CallAnnotationsValidator:
self.iterate_children(node_children,
self.dispatch_node_inside_definition)
self._call_location = prev_call_location
self._caller = prev_caller
self._call_location = None
return None
@ -704,8 +742,7 @@ class TopDownSummaryPrinter():
info: FunctionInfo
calees: set[FunctionInfo]
mismatch: Optional[FunctionInfo] = None
non_mt_safe_call_in_mt_safe: Optional[FunctionInfo] = None
non_pure_call_in_pure: Optional[FunctionInfo] = None
reason: Optional[DiagnosticKind] = None
def __init__(self):
self._is_first_group = True
@ -727,15 +764,12 @@ class TopDownSummaryPrinter():
if func is None:
func = TopDownSummaryPrinter.FunctionCallees(diag.source, set())
self._funcs[usr] = func
func.reason = diag.kind
if diag.kind == DiagnosticKind.ANNOTATIONS_DEF_DECL_MISMATCH:
func.mismatch = diag.target
else:
func.calees.add(diag.target)
self._unsafe_in_safe.add(diag.target.usr)
if diag.kind == DiagnosticKind.NON_MT_SAFE_CALL_IN_MT_SAFE_CTX:
func.non_mt_safe_call_in_mt_safe = diag.target
elif diag.kind == DiagnosticKind.NON_PURE_CALL_IN_PURE_CTX:
func.non_pure_call_in_pure = diag.target
def print_summary(self, root_dir: str):
row_groups: dict[str, list[list[str]]] = {}
@ -748,11 +782,11 @@ class TopDownSummaryPrinter():
row_group = []
name = f"\"{func_info.name}\" "
if func.mismatch:
if func.reason == DiagnosticKind.ANNOTATIONS_DEF_DECL_MISMATCH:
name += "declaration does not match definition"
elif func.non_mt_safe_call_in_mt_safe:
elif func.reason == DiagnosticKind.NON_MT_SAFE_CALL_IN_MT_SAFE_CTX:
name += "is mtsafe but calls non-mtsafe function(s)"
elif func.non_pure_call_in_pure:
elif func.reason == DiagnosticKind.NON_PURE_CALL_IN_PURE_CTX:
name += "is pure but calls non-pure function(s)"
else:
name += "for unknown reason (please add description)"

2
test_regress/t/t_a5_attributes_src.pl
View File

@ -36,7 +36,7 @@ sub check {
tee => 1,
cmd => ["python3", "$root/nodist/clang_check_attributes --verilator-root=$root --cxxflags='$clang_args' $precompile_args $srcfiles_str"]);
file_grep($Self->{run_log_filename}, "Number of functions reported unsafe: 14");
file_grep($Self->{run_log_filename}, "Number of functions reported unsafe: 27");
}
run_clang_check();

110
test_regress/t/t_dist_attributes_bad.h
View File

@ -273,4 +273,114 @@ public:
}
};
static void static_function() {}
class StaticClass {
public:
static void static_class_function() {}
};
class ConstructorCallsUnsafeLocalFunction {
public:
void unsafe_function() VL_MT_UNSAFE{};
ConstructorCallsUnsafeLocalFunction() { unsafe_function(); }
};
class ConstructorCallsStaticFunctionNoAnnotation {
public:
ConstructorCallsStaticFunctionNoAnnotation() { static_function(); }
};
class ConstructorCallsLocalFunction {
public:
void local_function() {}
ConstructorCallsLocalFunction() { local_function(); }
};
class ConstructorCallsLocalFunctionCallsGlobal {
public:
void local_function() { static_function(); }
ConstructorCallsLocalFunctionCallsGlobal() { local_function(); }
};
class SafeFunction {
public:
void safe_function() VL_MT_SAFE {}
};
class UnsafeFunction {
public:
void unsafe_function() VL_MT_UNSAFE {}
};
class ConstructorWithPointer {
public:
ConstructorWithPointer(SafeFunction* p) { p->safe_function(); }
};
class ConstructorWithReference {
public:
ConstructorWithReference(SafeFunction& p) { p.safe_function(); }
};
class ConstructorWithUnsafePointer {
public:
ConstructorWithUnsafePointer(UnsafeFunction* p) { p->unsafe_function(); }
};
class ConstructorWithUnsafeReference {
public:
ConstructorWithUnsafeReference(UnsafeFunction& p) { p.unsafe_function(); }
};
class ConstructorCallsLocalCallsGlobal {
void local_function2() { static_function(); }
void local_function() { local_function2(); }
public:
ConstructorCallsLocalCallsGlobal() { local_function(); }
};
class ConstructorCallsLocalCallsClassGlobal {
void local_function2() { StaticClass::static_class_function(); }
void local_function() { local_function2(); }
public:
ConstructorCallsLocalCallsClassGlobal() { local_function(); }
};
class TestClassConstructor {
void safe_function_unsafe_constructor_bad() VL_MT_SAFE {
ConstructorCallsUnsafeLocalFunction f{};
};
void safe_function_static_constructor_bad() VL_MT_SAFE {
ConstructorCallsStaticFunctionNoAnnotation f{};
};
void safe_function_local_function_global_bad() VL_MT_SAFE {
ConstructorCallsLocalFunctionCallsGlobal f{};
}
void safe_function_local_function_constructor_good() VL_MT_SAFE {
ConstructorCallsLocalFunction f{};
}
void safe_function_calls_constructor_with_pointer_good() VL_MT_SAFE {
SafeFunction* i = new SafeFunction{};
ConstructorWithPointer f{i};
}
void safe_function_calls_constructor_with_reference_good() VL_MT_SAFE {
SafeFunction i;
ConstructorWithReference f{i};
}
void safe_function_calls_constructor_with_unsafepointer_bad() VL_MT_SAFE {
UnsafeFunction* i = new UnsafeFunction{};
ConstructorWithUnsafePointer f{i};
}
void safe_function_calls_constructor_with_unsafereference_bad() VL_MT_SAFE {
UnsafeFunction i;
ConstructorWithUnsafeReference f{i};
}
void safe_function_calls_constructor_local_calls_global_bad() VL_MT_SAFE {
ConstructorCallsLocalCallsGlobal f{};
}
void safe_function_calls_constructor_local_calls_class_global_bad() VL_MT_SAFE {
ConstructorCallsLocalCallsClassGlobal f{};
}
};
#endif // T_DIST_ATTRIBUTES_BAD_H_

30
test_regress/t/t_dist_attributes_bad.out
View File

@ -724,6 +724,34 @@ t/t_dist_attributes_bad.cpp:75: [release] TestClass
t/t_dist_attributes_bad.h:124: [] TestClass::scm_ua_VL_REQUIRES(VerilatedMutex &) [declaration]
t/t_dist_attributes_bad.cpp:75: [requires] TestClass::scm_ua_VL_REQUIRES(VerilatedMutex &)
%Error: "TestClassConstructor::safe_function_calls_constructor_local_calls_class_global_bad()" is mtsafe but calls non-mtsafe function(s)
t/t_dist_attributes_bad.h:381: [mt_safe] TestClassConstructor::safe_function_calls_constructor_local_calls_class_global_bad()
t/t_dist_attributes_bad.h:280: [] StaticClass::static_class_function()
%Error: "TestClassConstructor::safe_function_calls_constructor_local_calls_global_bad()" is mtsafe but calls non-mtsafe function(s)
t/t_dist_attributes_bad.h:378: [mt_safe] TestClassConstructor::safe_function_calls_constructor_local_calls_global_bad()
t/t_dist_attributes_bad.h:276: [] static_function()
%Error: "TestClassConstructor::safe_function_calls_constructor_with_unsafepointer_bad()" is mtsafe but calls non-mtsafe function(s)
t/t_dist_attributes_bad.h:370: [mt_safe] TestClassConstructor::safe_function_calls_constructor_with_unsafepointer_bad()
t/t_dist_attributes_bad.h:311: [mt_unsafe] UnsafeFunction::unsafe_function()
%Error: "TestClassConstructor::safe_function_calls_constructor_with_unsafereference_bad()" is mtsafe but calls non-mtsafe function(s)
t/t_dist_attributes_bad.h:374: [mt_safe] TestClassConstructor::safe_function_calls_constructor_with_unsafereference_bad()
t/t_dist_attributes_bad.h:311: [mt_unsafe] UnsafeFunction::unsafe_function()
%Error: "TestClassConstructor::safe_function_local_function_global_bad()" is mtsafe but calls non-mtsafe function(s)
t/t_dist_attributes_bad.h:356: [mt_safe] TestClassConstructor::safe_function_local_function_global_bad()
t/t_dist_attributes_bad.h:276: [] static_function()
%Error: "TestClassConstructor::safe_function_static_constructor_bad()" is mtsafe but calls non-mtsafe function(s)
t/t_dist_attributes_bad.h:353: [mt_safe] TestClassConstructor::safe_function_static_constructor_bad()
t/t_dist_attributes_bad.h:276: [] static_function()
%Error: "TestClassConstructor::safe_function_unsafe_constructor_bad()" is mtsafe but calls non-mtsafe function(s)
t/t_dist_attributes_bad.h:350: [mt_safe] TestClassConstructor::safe_function_unsafe_constructor_bad()
t/t_dist_attributes_bad.h:285: [mt_unsafe] ConstructorCallsUnsafeLocalFunction::unsafe_function()
%Error: "ifh_test_caller_func_VL_MT_SAFE(VerilatedMutex &)" is mtsafe but calls non-mtsafe function(s)
t/t_dist_attributes_bad.cpp:53: [mt_safe] ifh_test_caller_func_VL_MT_SAFE(VerilatedMutex &)
t/t_dist_attributes_bad.h:94: [] ifh_NO_ANNOTATION(VerilatedMutex &)
@ -1157,4 +1185,4 @@ t/t_dist_attributes_bad.cpp:60: [mt_unsafe_one] sfc_VL_
t/t_dist_attributes_bad.cpp:60: [release] sfc_VL_RELEASE(VerilatedMutex &)
t/t_dist_attributes_bad.cpp:60: [release] sfc_VL_RELEASE_SHARED(VerilatedMutex &)
t/t_dist_attributes_bad.cpp:60: [requires] sfc_VL_REQUIRES(VerilatedMutex &)
Number of functions reported unsafe: 220
Number of functions reported unsafe: 224