OfekShilon / optview2

User-oriented fork of LLVM's opt-viewer
128 stars 13 forks source link

Links to source code of store/call/invoke are broken #22

Open Remi-Coulom opened 1 year ago

Remi-Coulom commented 1 year ago

I managed to get optview2 to produce annotated html output of my C++ code, and it is working well. However, for all remarks such as "not eliminated because it is clobbered by call", call is an hyperlink, and clicking this link leads to an error page such as: "Unable to locate file ./clang_release/src/joedb/compiler/c_wrapper.cpp" It seems that it is not using the source path for those links.

Remi-Coulom commented 1 year ago

To be more precise, here is the script I use to invoke optview2: https://github.com/Remi-Coulom/joedb/blob/master/compcmake/optview2.sh I am building with cmake, and the output is separate from the source code.

So, the command line for optview2 is: ~/repos/optview2/opt-viewer.py -j4 --output-dir clang_release/optview2 --source-dir ./clang_release ./clang_release/CMakeFiles

I have to set --source-dir to ./clang_release. Otherwise, it does not find the source. I guess that is because the CMakeLists.txt list sources as being "../../src/*".

OfekShilon commented 1 year ago

Hi @Remi-Coulom and thanks for bringing that up!

I'm not sure I understand: after setting --source-dir to ./clang_release - are your links working? If not - are you building some open-source project? Would it be possible to share the full build scenario?

OfekShilon commented 1 year ago

Also: I just pushed a fix to #24 that is possibly related (there was non-uniformity in handling of paths - some were relative and some absolute). Could you please try to reproduce with the lastest scripts?

Remi-Coulom commented 1 year ago

Hi,

Thanks for your reply. I tried with the latest optview2. It did not fix the problem.

Steps to replicate the problem (in Linux with clang +cmake installed, probably no other dependency is required but I am not 100% sure):

git clone git@github.com:Remi-Coulom/joedb.git
cd joedb/compcmake/
./generate.sh clang_release
cd clang_release/
ninja
cd ..
opt-viewer.py -j4 --output-dir clang_release/optview2 --source-dir ./clang_release ./clang_release/CMakeFiles

If you open this file: clangrelease/optview2/...._src_joedb_compiler_minimal_runtime_io.cpp.html on line 84, the link to joedb::Exception::Exception is broken.

Also, in: clangrelease/optview2/...._src_joedb_compiler_Compiler_Options.h.html on line 75, the "invoke" links to a html file that says "Unable to locate file ./clang_release/src/joedb/compiler/Compiler_Options_io.cpp"

OfekShilon commented 1 year ago

@Remi-Coulom Sorry for the delay - could you please verify that links are now working for you?

Remi-Coulom commented 1 year ago

Hi. Thanks for your work. I re-ran my script with the new version and it does not work at all any more. Here is the output I get:

2023-02-11 15:56:39,579 INFO Reading YAML files...
        129 of 129
2023-02-11 15:57:00,708 INFO Rendering index page...
2023-02-11 15:57:00,708 INFO   0 raw remarks
2023-02-11 15:57:00,708 WARNING Not generating report!
2023-02-11 15:57:00,708 INFO Ran for 0:00:21.141231

So the report is not generated at all. This behavior appeared with this commit: 8ca9331 Fix #24 : canonicalize remark.File path, change annotate_external logic to match I have checked that my script works if I run the version of optview2 of the previous commit.

OfekShilon commented 1 year ago

@Remi-Coulom The reason is a mismatch in --source-dir in your original command line was incorrect: it should be joedb. So if you're running from joedb/compcmake It should be something like -

...
opt-viewer.py -j4 --output-dir clang_release/optview2 --source-dir .. ./clang_release/CMakeFiles

(or run from the repo root). I just pushed an additional message about this, in case of "0 raw remarks". Can you please pull and check again?

Remi-Coulom commented 1 year ago

Thanks. I used the command you suggested, and it did produce a report. But the links to source code are all broken with messages such as: Unable to locate file /home/rcoulom/repos/joedb/compcmake/benchmark/benchmarkdb.cpp The actual path to the file is /home/rcoulom/repos/joedb/benchmark/benchmarkdb.cpp. It seems that it ignores the "../" of every source file.

OfekShilon commented 1 year ago

@Remi-Coulom I apologize, my attempts to solve #24 were careless and kept causing these issues. I now reverted them and have an idea how to approach it more carefully - please pull the latest version of the scripts and say if things are back to normal.

The source_dir argument in your case should be where you started the build from, joedb/compcmake/clang_release - either as a relative or absolute path.

Remi-Coulom commented 1 year ago

Thanks. I tried again, and there are still problems.

I'll explain the situation in more details. My project is organized like this: joedb/compcmake: the directory that contains CMakeLists.txt joedb/src: the directory that contains the source code. So, source files in CMakeLists.txt start with "../src/" joedb/compcmake/clang_release: the output directory. It is the directory in which I run cmake, and is different from the directory of CMakeLists.txt. I feel that this difference may be the source of the problem.

I use the directory of CMakeLists.txt as the "--source-dir" parameter. "yaml_dirs_or_files" can be either joedb/compcmake or joedb/compcmake/CMakeFiles, and it seems to be producing the same output.

So I ran optview2 with this command line inside the joedb/compcmake/clang_release directory: opt-viewer.py --output-dir optview2 --source-dir .. . I uploaded the output there: https://www.kayufu.com/files/optview2/index.html

In this file: https://www.kayufu.com/files/optview2/.._.._src_joedb_compiler_Compiler_Options.h.html#L75 If you click on the "call" link on line 75, it produces this error file: https://www.kayufu.com/files/optview2/src_joedb_compiler_c_wrapper.cpp.html#L58

Links to system headers are also broken: https://www.kayufu.com/files/optview2/_usr_bin_.._lib_gcc_x86_64-linux-gnu_9_.._.._.._.._include_c++_9_bits_char_traits.h.html#L372