Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add debug test and run at CI #29

Merged
merged 2 commits into from
Jan 31, 2024
Merged

Conversation

Shaikh-Ubaid
Copy link
Member

@Shaikh-Ubaid Shaikh-Ubaid commented Jan 29, 2024

@Shaikh-Ubaid
Copy link
Member Author

This needs to be rebased after merging/completion of #28.

@certik
Copy link
Contributor

certik commented Jan 29, 2024

It failed:

+ lfortran -g --debug-with-line-column tests/expr2.f90
sh: 1: llvm-dwarfdump: not found
Error in creating the files used to generate the debug information. This might be caused because either `llvm-dwarfdump` or `Python` are not available. Please activate the CONDA environment and compile again.
+ ls -l

But the CI succeeded. So we need to investigate how to make the CI fail (maybe LFortran returns 0 exit code), and then we need to fix it.

@Shaikh-Ubaid
Copy link
Member Author

Now it prints:

On Ubuntu:

+ lfortran -g --debug-with-line-column tests/expr2.f90
25

On Mac:

25
            __mamba_xctivate "${@}"
            ;;
        install|update|upgrade|remove|uninstall)
            __mamba_exe "${@}" || \return
total 104
            __mamba_xctivate reactivate
-rw-r--r--   1 runner  staff   1065 Jan 29 22:19 LICENSE
            ;;
-rw-r--r--   1 runner  staff     73 Jan 29 22:19 README.md
        self-update)
-rw-r--r--   1 runner  staff    109 Jan 29 22:19 environment.yml
            __mamba_exe "${@}" || \return
-rwxr-xr-x   1 runner  staff  49768 Jan 29 22:24 expr2.out
drwxr-xr-x   3 runner  staff     96 Jan 29 22:24 expr2.out.dSYM
-rw-r--r--   1 runner  staff   1992 Jan 29 22:24 expr2.out.tmp.o
-rw-r--r--   1 runner  staff   1615 Jan 29 22:24 expr2_ldd.txt
-rw-r--r--   1 runner  staff     96 Jan 29 22:24 expr2_lines.dat
-rw-r--r--   1 runner  staff     60 Jan 29 22:24 expr2_lines.dat.txt
-rw-r--r--   1 runner  staff     20 Jan 29 22:24 expr2_lines.txt
drwxr-xr-x  40 runner  staff   1280 Jan 29 22:24 lfortran
drwxr-xr-x   3 runner  staff     96 Jan 29 22:19 tests

Seems like it worked.

@Shaikh-Ubaid
Copy link
Member Author

So we need to investigate how to make the CI fail (maybe LFortran returns 0 exit code), and then we need to fix it.

(lf) ubaid@ubaids-MacBook-Pro lfortran % git status
On branch main
Your branch is up to date with 'origin/main'.

Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   src/bin/lfortran.cpp
        deleted:    src/libasr/dwarf_convert.py

no changes added to commit (use "git add" and/or "git commit -a")
(lf) ubaid@ubaids-MacBook-Pro lfortran % git diff
diff --git a/src/bin/lfortran.cpp b/src/bin/lfortran.cpp
index 00471ccab..0e825f0e7 100644
--- a/src/bin/lfortran.cpp
+++ b/src/bin/lfortran.cpp
@@ -1610,6 +1610,7 @@ int link_executable(const std::vector<std::string> &infiles,
                     "the debug information. This might be caused because either"
                     " `llvm-dwarfdump` or `Python` are not available. "
                     "Please activate the CONDA environment and compile again.\n";
+                std::cerr << "status is: " << status << std::endl;
                 return status;
             }
         }
@@ -2370,7 +2371,9 @@ int main(int argc, char *argv[])
     LCompilers::print_stack_on_segfault();
 #endif
     try {
-        return main_app(argc, argv);
+        int status = main_app(argc, argv);
+        std::cerr << "Main app returned: " << status << std::endl;
+        return status;
     } catch(const LCompilers::LCompilersException &e) {
         std::cerr << "Internal Compiler Error: Unhandled exception" << std::endl;
         std::vector<LCompilers::StacktraceItem> d = e.stacktrace_addresses();
diff --git a/src/libasr/dwarf_convert.py b/src/libasr/dwarf_convert.py
deleted file mode 100755

With the above git diff, we get the status (returned by system()) as

% lfortran examples/expr2.f90 -g --debug-with-line-column
sh: /Users/ubaid/Desktop/OpenSource/lfortran/src/bin/../libasr/dwarf_convert.py: No such file or directory
Error in creating the files used to generate the debug information. This might be caused because either `llvm-dwarfdump` or `Python` are not available. Please activate the CONDA environment and compile again.
status is: 32512
Main app returned: 32512
% echo $?
0

I think although the status returned by LFortran is not zero, but it is a multiple of 128 and/or 256. Hence I think bash is probably considering it as 0 (modulo with 128 or 256).

Python 3.12.1 | packaged by conda-forge | (main, Dec 23 2023, 08:01:35) [Clang 16.0.6 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> 32512 % 256
0
>>> 32512 % 128
0
>>> 

@Shaikh-Ubaid
Copy link
Member Author

Found the following here:

Note - Linux supports exit codes between 0-255. An exit value greater than 255 returns an exit code modulo 256. For example, exit 3809 gives an exit code of 225 (3809 % 256 = 225).

@Shaikh-Ubaid Shaikh-Ubaid force-pushed the test_debug branch 2 times, most recently from 7c390f2 to 3329116 Compare January 29, 2024 22:54
@Shaikh-Ubaid Shaikh-Ubaid marked this pull request as ready for review January 29, 2024 22:54
@Shaikh-Ubaid Shaikh-Ubaid requested a review from certik January 29, 2024 22:55
@certik
Copy link
Contributor

certik commented Jan 29, 2024

This is a bug in LFortran. The return integer must be in the range 0..255, otherwise it gets clamped and can become 0, as in this case.

.github/workflows/CI.yml Outdated Show resolved Hide resolved
tests/expr2.f90 Outdated Show resolved Hide resolved
@certik certik marked this pull request as draft January 29, 2024 23:09
@Shaikh-Ubaid Shaikh-Ubaid force-pushed the test_debug branch 2 times, most recently from e6b956d to 71d7f03 Compare January 29, 2024 23:41
@Shaikh-Ubaid
Copy link
Member Author

On my local system M1 mac:

LFortran v0.32.0:

(lf_32) ubaid@ubaids-MacBook-Pro lfortran % which lfortran
/Users/ubaid/conda_root/envs/lf_32/bin/lfortran
(lf_32) ubaid@ubaids-MacBook-Pro lfortran % lfortran --version
LFortran version: 0.32.0
Platform: macOS ARM
Default target: arm64-apple-darwin22.4.0
(lf_32) ubaid@ubaids-MacBook-Pro lfortran % cat examples/expr2.f90 
program expr2
implicit none

integer :: x

x = (2+3)*5
print *, x

error stop

end program
(lf_32) ubaid@ubaids-MacBook-Pro lfortran % lfortran examples/expr2.f90 
25
ERROR STOP
(lf_32) ubaid@ubaids-MacBook-Pro lfortran % lfortran examples/expr2.f90 -g --debug-with-line-column
expr2.out.dSYM/Contents/Resources/DWARF/expr2.out: No available targets are compatible with triple "aarch64-unknown-unknown"
sh: /Users/ubaid/conda_root/envs/lf_32/bin/../include/lfortran/impure/../dwarf_convert.py: No such file or directory
Error in creating the files used to generate the debug information. This might be caused because either `llvm-dwarfdump` or `Python` are not available. Please activate the CONDA environment and compile again.

LFortran v0.33.0:

(lf_conda) ubaid@ubaids-MacBook-Pro lfortran % which lfortran
/Users/ubaid/conda_root/envs/lf_conda/bin/lfortran
(lf_conda) ubaid@ubaids-MacBook-Pro lfortran % lfortran --version
LFortran version: 0.33.0
Platform: macOS ARM
Default target: arm64-apple-darwin22.4.0
(lf_conda) ubaid@ubaids-MacBook-Pro lfortran % cat examples/expr2.f90 
program expr2
implicit none

integer :: x

x = (2+3)*5
print *, x

error stop

end program
(lf_conda) ubaid@ubaids-MacBook-Pro lfortran % lfortran examples/expr2.f90 
25
ERROR STOP
(lf_conda) ubaid@ubaids-MacBook-Pro lfortran % lfortran examples/expr2.f90 -g --debug-with-line-column
expr2.out.dSYM/Contents/Resources/DWARF/expr2.out: No available targets are compatible with triple "aarch64-unknown-unknown"
25
  File "examples/expr2.f90", line 9
    error stop
ERROR STOP

tests/expr2.f90 Outdated
x = (2+3)*5
print *, x

error stop
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall I remove this error stop so that the CI passes?

@certik
Copy link
Contributor

certik commented Jan 30, 2024

It works:

+ lfortran -g --debug-with-line-column tests/expr2.f90
25
  File "tests/expr2.f90", line 9
    error stop
ERROR STOP
Error: Process completed with exit code 1.

We just need to figure out how to make it "succeed".

Comment on lines 272 to 273
lfortran -g --debug-with-line-column tests/expr2.f90
ls -l
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think we need to ponder if the above is a good way of adding this test. Plus we also need to fix lfortran to exit with appropriate code when the exit code is > 255 (or < 0).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to compare somehow the text that it prints.

@Shaikh-Ubaid
Copy link
Member Author

We just need to figure out how to make it "succeed".

Sure. At the moment, I think we atleast know that the latest lfortran version works with debugging support on linux and macos. Is it safe to close the issue lfortran/lfortran#3190? Or shall we close it after we figue out how to test the debugging support?

@certik
Copy link
Contributor

certik commented Jan 30, 2024

I think it's safe to close the LFortran issue. There is no bug in LFortran, nor in the conda package.

But after that let's test this properly, to ensure it does not break.

@Shaikh-Ubaid
Copy link
Member Author

I think it's safe to close the LFortran issue. There is no bug in LFortran, nor in the conda package.

But after that let's test this properly, to ensure it does not break.

Sure, Thanks! It seems we have an issue open for it here #27.

@certik
Copy link
Contributor

certik commented Jan 30, 2024

Let's do this: #27 (comment)

@Shaikh-Ubaid
Copy link
Member Author

Let's do this: #27 (comment)

Yes, sure. Submitted lfortran/lfortran#3263 that will allow the test to run when --no-llvm is provided but --skip-run-with-dbg is not provided (so that we can run the test).

@Shaikh-Ubaid
Copy link
Member Author

Submitted lfortran/lfortran#3263 that will allow the test to run when --no-llvm is provided but --skip-run-with-dbg is not provided (so that we can run the test).

After this gets merged, shall we simply checkout the commit id and use ./run_tests.py --no-llvm and run the test. Or do we need to make a release and then checkout the release like we did here

git clone https://github.com/lfortran/lfortran.git
cd lfortran
git fetch https://github.com/lfortran/lfortran.git --tags -f
git checkout v${{ env.LFORTRAN_VERSION }}

If we make a release then we might need to update at lfortran-feedstock (and wait until it gets available to install via conda). Later update the latest version here in this repository.

run: |
git clone https://github.com/lfortran/lfortran.git
cd lfortran
git checkout 925797482746846763531d5d5ea2cf9995359b63
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now I simply used checking out the specific commit. It seems this might not work, since the reference outputs generated by the lfortran v0.33.03 and the reference outputs expected at this commit 925797482746846763531d5d5ea2cf9995359b63 would not be the same (the ref tests might have got updated by the time of this commit).

@certik
Copy link
Contributor

certik commented Jan 31, 2024

I can make a release. Just ensure locally that the reference tests now behave correctly, and ping me if they do. I'll then make a release.

@Shaikh-Ubaid
Copy link
Member Author

Just ensure locally that the reference tests now behave correctly, and ping me if they do

Yes, they behave as expected.

(lf) ubaid@ubaids-MacBook-Pro lfortran % ./run_tests.py --no-llvm --skip-cpptranslate
...
errors/runtime_stacktrace_01.f90 * run_dbg ✓ 
TESTS PASSED

@certik
Copy link
Contributor

certik commented Jan 31, 2024

Perfect, thanks! Here is the new release:

https://github.com/lfortran/lfortran/releases/tag/v0.33.1

@Shaikh-Ubaid
Copy link
Member Author

From the CI, for both linux and macos:

...
errors/runtime_stacktrace_01.f90 * run_dbg ✓ 
TESTS PASSED

It works!

@Shaikh-Ubaid Shaikh-Ubaid marked this pull request as ready for review January 31, 2024 07:10
@Shaikh-Ubaid Shaikh-Ubaid merged commit 4fcfea1 into lfortran:main Jan 31, 2024
8 checks passed
@Shaikh-Ubaid Shaikh-Ubaid deleted the test_debug branch January 31, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants