VerySleepy / verysleepy

Very Sleepy, a sampling CPU profiler for Windows
http://www.codersnotes.com/sleepy
GNU General Public License v2.0
1.05k stars 103 forks source link

Incorrect line information/highlighting #114

Open k4lizen opened 1 year ago

k4lizen commented 1 year ago

image image image image

The highlighted line, and the line specified, is not actually the line in which the function is defined. I also suspect that the execution times may be on the wrong lines.

image

Here are the source code, executable, .capture, and input file: src.zip I compiled the program with the command: g++ main.cpp -o main.exe -g -Wall

To run the program, run the executable, copy-paste the whole input into the program, and interrupt it when it stops outputting text (Ctrl+C) (the last line should say WAIT).

CyberShadow commented 1 year ago

Could you please clarify what you mean by "The highlighted line, and the line specified, is not actually the line in which the function is defined"?

In any case, it sounds like the problem you are seeing is that the source lines do not match your expectations. However, Very Sleepy itself does not implement decoding of debug information, and merely obtains them from the debug engine / debug information provider used. So, a bug which would cause the wrong line numbers to be reported could be caused by a bug in the compiler, linker, debug information decoder, or something mundane such mismatching debug information from an older build.

For DWARF debug information, Very Sleepy uses the Dr. MinGW project to decode the debug information.

k4lizen commented 1 year ago

Could you please clarify what you mean by "The highlighted line, and the line specified, is not actually the line in which the function is defined"?

When I click the function get_possible_moves in the UI, the line that is highlighted is not the one I would expect. The highlighted line is 225, I expect the line 219 to be highlighted. Also, the tab for "Source line" for the function, has the number 225. (Sorry if some confusion was caused by the "Line 226" picture).

In any case, it sounds like the problem you are seeing is that the source lines do not match your expectations.

Yup.

...or something mundane such mismatching debug information from an older build.

I considered this, but I don't have any idea how this would be the case, considering I only have one executable (and am not saving the older ones), and have reran and recompiled a few times (just now as well). Also, I have no problem with the debugger, the breakpoints are aligned to where I set them etc.

I see now that I get this when selecting main.exe in the process select screen, could this be related?: image

CyberShadow commented 1 year ago

Thank you for the clarification. Could you please post the executable file? Oops, I see it's also included in src.zip, thank you

CyberShadow commented 1 year ago

Okay, I had a look at this and I think it's just that the Very Sleepy UI is a little misleading.

The debug information is OK. llvm-dwarfdump says:

0x0001bf0a:   DW_TAG_subprogram
                DW_AT_external  (true)
                DW_AT_name  ("get_possible_moves")
                DW_AT_decl_file ("E:\Projects\vscode\_competative\CodinGame\photosynthesis\src/main.cpp")
                DW_AT_decl_line (219)
                DW_AT_linkage_name  ("_Z18get_possible_movesb")
                DW_AT_type  (0x00007d20 "std::vector<Ply, std::allocator<Ply> >")
                DW_AT_low_pc    (0x00401e26)
                DW_AT_high_pc   (0x0040229b)
                DW_AT_frame_base    (DW_OP_call_frame_cfa)
                DW_AT_GNU_all_tail_call_sites   (true)
                DW_AT_sibling   (0x0001c148)

In the capture, Symbols.txt has:

0x401e89 "main" "get_possible_moves" "E:/Projects/vscode/_competative/CodinGame/photosynthesis/src/main.cpp" 225
0x401ebf "main" "get_possible_moves" "E:/Projects/vscode/_competative/CodinGame/photosynthesis/src/main.cpp" 232
0x401ecd "main" "get_possible_moves" "E:/Projects/vscode/_competative/CodinGame/photosynthesis/src/main.cpp" 233
0x401ed3 "main" "get_possible_moves" "E:/Projects/vscode/_competative/CodinGame/photosynthesis/src/main.cpp" 233
0x401ee6 "main" "get_possible_moves" "E:/Projects/vscode/_competative/CodinGame/photosynthesis/src/main.cpp" 233
0x401eeb "main" "get_possible_moves" "E:/Projects/vscode/_competative/CodinGame/photosynthesis/src/main.cpp" 233
0x401f29 "main" "get_possible_moves" "E:/Projects/vscode/_competative/CodinGame/photosynthesis/src/main.cpp" 238
...
0x40225a "main" "get_possible_moves" "E:/Projects/vscode/_competative/CodinGame/photosynthesis/src/main.cpp" 293
0x40226b "main" "get_possible_moves" "E:/Projects/vscode/_competative/CodinGame/photosynthesis/src/main.cpp" 294

So, it's not that 225 is the (incorrect) line number of the start of the function, but rather, the top-most sample that Very Sleepy observed within that function was on line 225.

Maybe the column should be renamed to "Line of first sample".

k4lizen commented 1 year ago

Is the knowledge of the line that was sampled useful? Would it not make more sense to keep the column "Souce Line", but take the information from the debug rather than the capture?

CyberShadow commented 1 year ago

There is nothing in the information that Very Sleepy collects that is intrinsically tied to the start of a function. Hypothetically, Very Sleepy could request the debug information engine to tell it the address and line number of the start of each function, though the utility of that information would be purely for cases such as this, and this query would come at additional cost.

k4lizen commented 1 year ago

I think that would be useful. I don't know have an idea on much additional cost it would incur so it's up to you. The "Line of first sample" does give the approximate location of the function, which is at least something. It should however, definitely be renamed then. If you do decide to add the start-of-function line, the "line of first sample" seems unnecessary (at least to me).