Fix errors reported by valgrind#827
Open
gottfriedleibniz wants to merge 10 commits intoSDL-Hercules-390:developfrom
Open
Fix errors reported by valgrind#827gottfriedleibniz wants to merge 10 commits intoSDL-Hercules-390:developfrom
gottfriedleibniz wants to merge 10 commits intoSDL-Hercules-390:developfrom
Conversation
'pszTUNDevice = strdup( DEF_NETDEV );' is definitely lost. pszOATFilename check included to follow convention.
1. ckd_dasd_init_handler invokes cckd_sf_parse_sfn 2. ckd_dasd_init_handler invokes ckd64_dasd_init_handler 3. ckd64_dasd_init_handler invokes cckd_sf_parse_sfn Leaving previous dasdsfn dangling. Given how 'dasdsfn' is managed (e.g., in attach_device and dasd_close_device), it may be more elegant to compare the previous dasdsfn to the current pathname and throwing a warning if they are different.
To avoid instances of memcmp'ing random stack data with curr_psw.
ARCH_DEP( maddr_l ) may reference tlb.asb and tlb.common - ensure newregs in program_return does not reference garbage stack data.
To avoid feeding random stack data to syscalls later on (e.g., write in cckd64_write).
Uses of string.h functions and regexec assume the string is null delimited. This may not be true with some _PANMSG msg uses.
When CPU_BITMAP is __uint128_t and F_CPU_BITMAP is '"%16.16"PRIX64', random/clobbered stack data will be referenced in the associated libc formatting routines used by fwritemsg due to the different stack alignment requirements of the variadic arguments.
panel_command may be invoked (e.g., 'LPARNUM BASIC') prior to the panel display thread being initialized. This can lead to history_add being invoked prior to history_init (causing dangling pointers).
gottfriedleibniz
requested review from
Fish-Git,
Juergen-Git and
Peter-J-Jansen
as code owners
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR investigated running Hercules on the test suite, VM/370 Community Edition, etc., under a variety of valgrind (mainly memcheck) configurations. To quickly reproduce these findings, func_exec_program_core in the libtool script can be modified, e.g:
Much of my testing was surface-level, and I did not thoroughly exhaust all code paths, features, or interactions (esp. around networking or long-running scenarios that perform meaningful work). There may be more wins available1.
A few changes in this PR can be implemented differently, e.g., see 'cckddasd.c: ensure dasdsfn is free'd', so care is required.
Footnotes
If this PR remains open for a longer period, I’ll include additional findings here. Valgrind slows things down considerably, which makes testing anything real-world a bit frustrating (I haven’t discovered anything meaningful with ASan yet). ↩