[andrew-aitchison]

Update tools/MT-Unsafe.py to find all the unsafe functions listed in man elf_flagelf

• this man page lists several unsafe functions on one source line.

Update addons/threadsafety.py to include these functions.

[danmar]

I would like that the PR title is not chopped like this.

it wouldn't hurt with a line in releasenotes to mention this improvement.

testing: I am not sure what we can test but feel free to create a regression test if you can. Provide a made up input to MT-Unsafe.py and check that it outputs certain symbols?

[andrew-aitchison]

Title fixed.
The other two suggestions are also good.

[andrew-aitchison]

testing: I am not sure what we can test but feel free to create a regression test if you can. Provide a made up input to MT-Unsafe.py and check that it outputs certain symbols?

I have an input for MT-Unsafe.py and an expected output.
testrunner is probably not a good place to run a python script; where should I add this to the regression tests ?

[danmar]

testrunner is probably not a good place to run a python script; where should I add this to the regression tests ?

that is correct.

I suggest some pytest testing in addons/test folder. there is no threadsafety_test.py there now which I spontanously feel is weird. but there is a threadsafety folder there I don't know how that is used.

https://github.com/cppcheck-opensource/cppcheck/tree/main/addons/test

[andrew-aitchison]

I suggest some pytest testing in addons/test folder. there is no threadsafety_test.py there now which I spontanously feel is weird. but there is a threadsafety folder there I don't know how that is used.

Hmm. Looks like I did that. I'll have to figure out what I was thinking ...

[dmcppcheck]

AI review

The new .BR parsing in tools/MT-Unsafe.py turns troff escapes it doesn't handle into bad entries. Real man pages use \% and quoted arguments, and the base version simply skipped such lines. The PR now outputs names like '%pthread_attr_setaffinity_np', '"strtok"' and 'foo_r"and"bar'. Since this output gets merged into addons/threadsafety.py, those entries would be wrong there. Separately, the new regression input test/tools/MT-Unsafe.3 plus its .expected file is not run by any test, so it does not protect against regressions yet. The six elf_flag* names added to addons/threadsafety.py look correct.

Testing

This change is Python only, so I didn't build the C++ binaries.

• python3 tools/MT-Unsafe.py test/tools/MT-Unsafe.3 | diff - test/tools/MT-Unsafe.3.expected gives no diff, as the PR intends.
• The same file gzipped (gzip -c test/tools/MT-Unsafe.3 > t.3.gz) gives output that differs from .expected: the '' type entry is missing. This is minor because it only affects the types list.
• I wrote scratch/a.3 with attribute-table rows in the styles man-pages uses, and ran it through both base and PR:

.TS
allbox;
lbx lb lb
l l l.
Interface	Attribute	Value
T{
.na
.nh
.BR \%pthread_attr_setaffinity_np (),
.BR \%pthread_attr_getaffinity_np ()
T}	Thread safety	MT-Unsafe env
T{
.BR "strtok" ()
T}	Thread safety	MT-Unsafe race:strtok
T{
.BR foo_r " and " bar ()
T}	Thread safety	MT-Unsafe race:x
.TE

python3 pr/tools/MT-Unsafe.py a.3 outputs these API entries:

    '"strtok"',
    '%pthread_attr_getaffinity_np',
    '%pthread_attr_setaffinity_np',
    'foo_r"and"bar',

The .gz version of the file gives the same result. Base output contained only foo_r from these rows, so base missed them but produced no garbage.

---

Automated review of 856869c by Claude. The pull request was built and tested locally.

[dmcppcheck]

AI review

This character class only strips ~ ( ) space ' \\ and newline, so other troff syntax ends up in the function names.

• .BR \%pthread_attr_setaffinity_np (), gives '%pthread_attr_setaffinity_np'. The \% hyphenation escape is common in current man-pages.
• .BR "strtok" () gives '"strtok"'.
• .BR foo_r " and " bar () gives 'foo_r"and"bar'.

The old regex just skipped these lines; now they produce invalid identifiers that would be merged into threadsafety.py. Extracting identifiers with something like re.findall(r'[A-Za-z_]\w*(?=\s*\\?\(|\s*"?\s*\(|,|$)', ...), or at least stripping \% and " before the split, would avoid this.

[dmcppcheck]

AI review

Nothing runs this comparison: no pytest or CI step uses MT-Unsafe.3 / MT-Unsafe.3.expected, so it is only a manual check. A small pytest that runs the script and compares its stdout with the expected file (as danmar suggested for addons/test) would make it a real regression test. Note that running it on a gzipped copy of this page gives different output: the '' type entry is missing.