Skip to content

fix dwarf_find_line returning the wrong line number#95

Merged
jrfonseca merged 1 commit intojrfonseca:mainfrom
oltolm:lineno
Sep 25, 2025
Merged

fix dwarf_find_line returning the wrong line number#95
jrfonseca merged 1 commit intojrfonseca:mainfrom
oltolm:lineno

Conversation

@oltolm
Copy link
Copy Markdown
Contributor

@oltolm oltolm commented Sep 2, 2025

In my tests I encountered a case where drmingw would return the wrong line number for an exe produced by GCC. I have fixed it.

@oltolm
Copy link
Copy Markdown
Contributor Author

oltolm commented Sep 4, 2025

I noticed 2 other problems.

  1. The line number is off by one for DWARF5.
  2. search_func only looks at DW_AT_low_pc and DW_AT_high_pc, but does not look at DW_AT_ranges.

Will send a patch to fix it.

@jrfonseca
Copy link
Copy Markdown
Owner

Regarding off-by-one errors, I'm not sure it's relevant, but be aware that drmingw adjust the symbol address by one to try to get the calling code source line, as opposed of the return code source line (often the next line), as seen in

drmingw/src/common/log.cpp

Lines 355 to 363 in 616ab1c

/*
* When we walk into the callers, StackFrame.AddrPC.Offset will not
* contain the calling function's address, but rather the return
* address. This could be the next statement, or sometimes (for
* no-return functions) a completely different function, so nudge the
* address by one byte to ensure we get the information about the
* calling statement itself.
*/
nudge = -1;

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 20, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.06%. Comparing base (616ab1c) to head (88ff8a5).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/mgwhelp/dwarf_find.cpp 75.00% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #95      +/-   ##
==========================================
- Coverage   51.11%   51.06%   -0.05%     
==========================================
  Files          15       15              
  Lines        2160     2162       +2     
  Branches      824      828       +4     
==========================================
  Hits         1104     1104              
- Misses        809      810       +1     
- Partials      247      248       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@oltolm
Copy link
Copy Markdown
Contributor Author

oltolm commented Sep 20, 2025

I think I fixed it. I was wrong about the off-by-one error. With regard to DW_AT_ranges maybe I will send a patch later.

@jrfonseca jrfonseca merged commit 992a456 into jrfonseca:main Sep 25, 2025
7 checks passed
@jrfonseca
Copy link
Copy Markdown
Owner

I don't fully understand why the early break was removed, but it shouldn't be a problem in practice.

@oltolm
Copy link
Copy Markdown
Contributor Author

oltolm commented Sep 25, 2025

Thanks for merging. The early break was removed because it is not the case that the line information is in one big sequence. It is split in multiple sequences and you have to search all of them.

@jrfonseca
Copy link
Copy Markdown
Owner

I see. Thanks for the explanation.

@oltolm
Copy link
Copy Markdown
Contributor Author

oltolm commented Sep 25, 2025

I just tested it again and now it produces wrong results. I don't know why. Please revert, sorry. Next time I will test it more extensively.

@oltolm oltolm deleted the lineno branch September 25, 2025 19:25
jrfonseca added a commit that referenced this pull request Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants