`CON34-C`: Implementation seems to check incorrect aspects of thread storage duration objects
A pull request for this has already been merged.
- #778 by @lcartey — merged
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 35/100
Research direction
Start with the CON34-C implementation and its existing validation for tss_t values, then compare it with the compliant and non-compliant examples in this issue. The change is done when the intended thread-storage violation is reported and unrelated uninitialized-value cases are not attributed to CON34-C; validate with the rule's available tests.
Written by the indexing model from the issue text.
Description
Affected rules
CON34-C
Description
The documentation seems to describe the risk of passing a tss_t into a thread. The risk here is that the new thread will have no value.
In the "compliant" case it says the appropriate fix is to use tss_get() to get the value in the current thread, then pass that value into the new thread.
In our implementation, we check that any tss_t values are definitely given a value before retrieved and passed into a thread creation statement. This is not the intention of the rule, adds additional implementation complexity, and reveals no violations in MRVA.
Example
tss_t key;
void f1() {
// Should be marked non-compliant: new thread has no value for tss_get(key)
// Currently not reported
thrd_create(..., &key);
}
void f2() {
// While the following code is suspect, it is NOT the intention of the rule is not to disallow this:
tss_t localkey;
tss_set(localkey, malloc(...));
void* v = tss_get(localkey);
thrd_create(..., v);
// The fact that v is uninitialized has nothing to do with threads. For instance, this is also erroneous:
printf("%d", *v);
// Further, the above code has nothing to do with thread storage duration, and isn't necessarily invalid:
static void *shared_buf = malloc(...);
thrd_create(..., shared_buf); // Reasonable pattern
}
- Dominant language
- CodeQL
- Stars
- 227
- Forks
- 82
- Avg merge
- 6d 7h
- Merged PRs (30d)
- 9
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from github/codeql-coding-standards
-
false positive/false negative Stardard-MISRA-C++
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
github/codeql-coding-standards#1172 ·
-
Difficulty-Low false positive/false negative false-negative Impact-Low Standard-MISRA-C
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
Difficulty-Medium false positive/false negative false-positive Impact-Medium Standard-CERT-C
Difficulty 4/5 3-5 days Newbie friendliness 48/100
github/codeql-coding-standards#1200 ·
-
`RULE-0-0-1`: "unreachable statement" false positives due to over-pruning of the control-flow graph Openfalse positive/false negative
Difficulty 4/5 3-5 days Newbie friendliness 48/100
github/codeql-coding-standards#1190 ·
-
false positive/false negative
Difficulty 3/5 1-2 days Newbie friendliness 65/100
github/codeql-coding-standards#1175 ·
All issues in github/codeql-coding-standards
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
ScoopInstaller/Nonportable#639 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
ScoopInstaller/Extras#18800 ·
-
Difficulty 1/5 Under an hour Newbie friendliness 85/100
uqbar-project/website-wollok-ts#84 · 2 comments ·
-
on hold T: core-bug
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
Difficulty 1/5 Under an hour Newbie friendliness 88/100