Correctly propagate pc ranges for blocks and local variables #1226

kupsch · 2022-03-17T18:39:18Z

fix valid pc address ranges for local variables that are declared in a
sub-block (brace, try and catch blocks) of their function
fix Context class's constructors so they correctly initialize and copy
all member (use in-class initialize and default constructors)

hainest

The Context constructor also doesn't initialize the Dwarf_Die members. They're aggregates for a C interface, so it probably doesn't matter. But it also initializes the members out of order, so that's a barrel of chuckles waiting to happen. I say we use in-class initializers and smack the class with the rule-of-zero hammer.

kupsch · 2022-03-18T19:12:40Z

I made initialized all members in-class and removed the constructors. The Dwarf_Die member were always set before used, but for safety, they are now all value (zero) initialized. The member were initialized in order (the compiler warns if this is not the case).

hainest

LGTM. I would suggest a slightly less ambiguous commit title. Something like 'Correctly propagate address ranges for lexical, try, and catch blocks'.

- fix valid pc address ranges for local variables that are declared in a sub-block (brace, try and catch blocks) of their function - fix Context class's constructors so they correctly initialize and copy all member (use in-class initialize and default constructors)

* Update dependency versions in base container config This should have been part of #1211 * Docker: adding a workflow for release (#1219) Currently we do the entire build from scratch for release, and this strategy was chosen for the cleanest build. However we can use the same strategy as we do for testing and take advantage of the base container. This PR adds a workflow to do that Signed-off-by: vsoch <[email protected]> Co-authored-by: vsoch <[email protected]> * Remove usage of DW_AT_MIPS_linkage_name (#1223) This isn't part of DWARF4 or DWARF5. Co-authored-by: Tim Haines <[email protected]> * Docker: testing workflow to run libabigail (#1220) Run libabigail's abidiff to detect ABI breakages between the current PR and the last successful build as well as the current PR and the last release. * testing workflow to run libabigail! I am not sure if the artifact is extracted relative or not, so will need to update the workflow to account for that. This is also a new strategy that will try to do the new build/retrieval of artifacts from previous containers - have not tried this yet! Signed-off-by: vsoch <[email protected]> Co-authored-by: vsoch <[email protected]> Co-authored-by: Tim Haines <[email protected]> * Correctly propagate pc ranges for blocks and local variables (#1226) - fix valid pc address ranges for local variables that are declared in a sub-block (brace, try and catch blocks) of their function - fix Context class's constructors so they correctly initialize and copy all member (use in-class initialize and default constructors) * Fix warnings with cmake's MINSIZEREL build type (#1235) - Explicitly define optimization flags for cmake's MINSIZEREL build type. Cmake by default includes -DNDEBUG. This results in assert being defined to do nothing. Dyninst use asserts as a fatal error reporting mechanism and expects assert to not return if the condition is always false. If assert can return then many warnings are produced due to this unexpect path. - Remove defining NDEBUG for REL build types if using the MSC compiler * Remove BUILD_RT option (#1238) When not enabled, the dyninstAPI_RT/cmake_install.cmake isn't present. This has been broken since it was implemented by e1afc69 in 2016. * Add parsing of names for inlined functions in DWARF (#1237) * Clean up and enhance debug messages * Add parsing of names for inlined functions Co-authored-by: Tim Haines <[email protected]> * Remove void pointer arithmetic when using Valgrind annotations (#1236) gcc allows this as an extension by considering sizeof(void) to be 1 (http://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html). * Docker: use more OS packages for dependencies (#1221) * Docker: use more OS packages for dependencies This should have been part of #1211. * Docker: don't build Dyninst through spack for the environment (#1222) This results in two installations: one in /opt/dyninst-dev/.spack/include and one in /opt/dyninst-dev/install/dyninst. The former is not updated when testing a PR, so causes public header conflicts. * Docker: use external-tests instead of testsuite in base image (#1209) This is in preparation for using -Werror when building Dyninst in the Github workflow. The test suite does not currently build cleanly, so we need a different set of tests. * Docker: make compile warnings fatal (#1242) * Remove unused git files (#1244) .gitmodules hasn't been used since the test suite was moved to its own repository. .github_changelog_generate hasn't been used since v10.0.0 * Improve compiler diagnostic suppression handling (#1239) - Create compiler specific diagnostic suppression macros to suppress a type of warning for a region of code that are consistently defined based on the build environment. - Replace current multi-line preprocessor #if and #pragma statements with one macro. - Add suppression for warnings in instructionAPI/src/Register.C when using gcc 6-8. * Remove MSC compiler warning suppressions (#1239) Likely no longer needed with current MSC compiler and will hide problems if MSC is used. - Removed diagnostic suppressing pragmas - Removed compiler command line suppression options * Fix frame-larger-than warning (#1239) - Increase frame size max when using gcc 6 for non-debug builds and for all debug builds (needed for rhel's gcc) to compile instructionAPI/src/AMDGPU/cdna2/InstructionDecoder-amdgpu-cdna2.C * Add cmake option to disable diagnostic suppressions (#1239) - DYNINST_DISABLE_DIAGNOSTIC_SUPPRESSIONS option: if set, disable all warning suppressions and frame size limit overrides * Add compiler warning related cmake options (#1239) - DYNINST_WARNINGS_AS_ERRORS option: if set, treat warnings as errors - DYNINST_EXTRA_WARNINGS option: additional warning options to test for and use if valid with the current compiler * Remove unneeded #pragma's (#1240) - Remove once and interface pragmas - Replace use of "external/stdint-win.h" and "external/inttypes.h" with <stdint.h> and <inttypes.h> * remove unused files containing pragmas (#1240) * Cleanup (remove) ancient linux kernel support (#1241) - Remove check and message for ancient linux kernel - Remove support for old mechanism to find process's tasks * Use bfd linker for LTO (#1248) The gold linker crashes and is no longer supported. * Add cmake options for C/C++ language standards (#1246) Add cmake options to set C/C++ language standard versions used to build to facilitate testing. - DYNINST_C_LANGUAGE_STANDARD cmake option: C Standard version - DYNINST_CXX_LANGUAGE_STANDARD cmake option: C++ Standard version * Make dyninstAPI_RT files build with standard C (#1246) - use same language standards as the rest of Dyninst - define _DEFAULT_SOURCE in some dyninstAPI_RT source files so functions and macros used are defined when using standard C * Fix format string errors in stackwalk/callchecker.C (#1250) These are only present when SW_ANALYSIS_STEPPER=OFF. * Redo finalization to get correct function boundiaries when (#1249) there are many tail call correction * Fix dyninstAPI_RT files to build with older glibc (#1252) Replace feature test macro _DEFAULT_SOURCE with _GNU_SOURCE to support older version of glibc * Remove unused build options (#1253) * Remove BUILD_DOCS This isn't sufficient to create the documentation as some of it is contained in MS Word files. * Remove BUILD_TARBALLS We don't distrubute tarballs anymore. That's handled by GitHub. * start of work to refactor the docs to use readthedocs this is the first pass to format the previous latex into rst. I have done the conversion and only started to go through ensuring that content is preserved (meaning I did not miss anything) and all the code blocks are formatted (after the automated conversion they indeed are not!) I will want a few more hours to finish this up, and then we need to discuss deployment. E.g., I recommend readthedocs so you can automate deployment and keep versioned docs. We can also deploy to github pages (I can make a workflow) but I do not have a good suggestion for versioning things that way. I also have not added in an ability to still render the pdfs if that is desired, which I think should be possibly. Finally, it would be nice if some of these docs could render from docstrings - I know how to do this for Python so I wonder if Cpp is that much different. It is probably terrible because it is cpp, but what can you do? Signed-off-by: vsoch <[email protected]> * adding missing figures and re-creating tables in format that will render Signed-off-by: vsoch <[email protected]> * use different version of checkout to get around checkout bug Signed-off-by: vsoch <[email protected]> * try fix for gha Signed-off-by: vsoch <[email protected]> Signed-off-by: vsoch <[email protected]> Co-authored-by: Tim Haines <[email protected]> Co-authored-by: vsoch <[email protected]> Co-authored-by: Tim Haines <[email protected]> Co-authored-by: kupsch <[email protected]> Co-authored-by: Xiaozhu Meng <[email protected]>

kupsch added bug Symtab This issue is directly related to SymtabAPI DWARF Item is related to DWARF parsing labels Mar 17, 2022

kupsch assigned hainest Mar 17, 2022

kupsch mentioned this pull request Mar 17, 2022

The symbol information ignores additional restrictions on range provided by surrounding DW_TAG_lexical_block #1225

Closed

hainest reviewed Mar 18, 2022

View reviewed changes

hainest approved these changes Mar 19, 2022

View reviewed changes

kupsch force-pushed the kupsch/fix-variable-ranges branch from 1624c57 to 43603d1 Compare March 21, 2022 06:12

kupsch changed the title ~~fix variable ranges~~ Correctly propagate pc ranges for blocks and local variables Mar 21, 2022

kupsch merged commit 8454ff5 into master Mar 21, 2022

kupsch deleted the kupsch/fix-variable-ranges branch May 6, 2022 03:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correctly propagate pc ranges for blocks and local variables #1226

Correctly propagate pc ranges for blocks and local variables #1226

kupsch commented Mar 17, 2022 •

edited

Loading

hainest left a comment

kupsch commented Mar 18, 2022

hainest left a comment

Correctly propagate pc ranges for blocks and local variables #1226

Correctly propagate pc ranges for blocks and local variables #1226

Conversation

kupsch commented Mar 17, 2022 • edited Loading

hainest left a comment

Choose a reason for hiding this comment

kupsch commented Mar 18, 2022

hainest left a comment

Choose a reason for hiding this comment

kupsch commented Mar 17, 2022 •

edited

Loading