PLC-lang / rusty

Structured Text Parser and LLVM Frontend
GNU Lesser General Public License v3.0
203 stars 52 forks source link

Debug flag inclusion #1065

Closed rohanrayan closed 7 months ago

rohanrayan commented 8 months ago

Hello

I am building a PLC project using the build configuration (plc.json) of rusty.

Is there a way I can specify compiler flags during the build? I specifically want to enable the -g (debug) flags.

Thanks

ghaith commented 8 months ago

Hello You can simply pass -g in the build command or --debug it will be passed through

rohanrayan commented 8 months ago

Do you mean running the following command?

plc -g build

I am getting the following error in that case, am I doing something wrong?

plc: /home/llvm-project-14x/llvm/lib/IR/DIBuilder.cpp:794: llvm::DILocalVariable llvm::DIBuilder::createParameterVariable(llvm::DIScope, llvm::StringRef, unsigned int, llvm::DIFile, unsigned int, llvm::DIType, bool, llvm::DINode::DIFlags, llvm::DINodeArray): Assertion ArgNo && "Expected non-zero argument number for parameter"' failed. plc: /home/llvm-project-14x/llvm/include/llvm/Support/Casting.h:104: static bool llvm::isa_impl_cl<To, const From*>::doit(const From*) [with To = llvm::DIExpression; From = llvm::Metadata]: AssertionVal && "isa<> used on a null pointer"' failed. plc: /home/llvm-project-14x/llvm/include/llvm/Support/Casting.h:104: static bool llvm::isa_impl_cl<To, const From>::doit(const From) [with To = llvm::DIExpression; From = llvm::Metadata]: Assertion `Val && "isa<> used on a null pointer"' failed. Aborted (core dumped)

Note: I have attached the plc.json file that I am using plc.json

ghaith commented 8 months ago

This seems to be a problem in one of the files, can you try to pinpoint the file that causes the problem and share it? It might be a scenario we did not take into account. Side note but probably not related to the debug issue: the lib functions fit better as an include so that they don't get compiled again, but I don't think that's the issue. You can include the full standard lib in a libraries section: https://plc-lang.github.io/rusty/using_rusty/build_configuration.html#libraries You can build the lib using the shell script with ./scripts/build.sh --build --release --package which will place it in the output folder.

rohanrayan commented 8 months ago

@ghaith I am not sure this is related to any PLC code in my particular example. I am not even able to compile some of the standard functions provided by Rusty.

For example, see the plc.json file below plc.json

In this case, I am just trying to compile the numerical_functions.st file provided by Rusty. In this case as well, the plc -g build command fails, while the plain plc build command passes.

Please let me know if you need any further information.

Thanks again for your support!

ghaith commented 8 months ago

Hello @rohanrayan,

It looks like the issue is then in the std functions code and we would be able to reproduce it. I'll try it out next chance i get. We never really compiled the std functions with debug flag so we never encountered any issues there. Thank you for pointing that out.

rohanrayan commented 8 months ago

Hello

Is it possible to know when you plan to work on this and when I can expect some feedback regarding this issue?

Thanks

ghaith commented 8 months ago

Hello I think @mhasel was already checking it out. But I could be mistaken. Regardless it's planned for January/February since it's a major debug functionality that seems to be broken here.

mhasel commented 7 months ago

Hello @rohanrayan

We've looked into the issue of not being able to compile some of the standard-lib files to IR and found the issue was the generation of debug-info for external subroutines, which resulted in the addition of <temporary!> tags. These could not be successfully parsed by the llvm verify call, which happens automatically when generating IR. It is also worth mentioning that this was only an issue when compiling to IR - when generating an object file, LLVM solved this issue for us.

This issue should be fixed with #1072 - however, we were unable to reproduce the error you mentioned. Can you try if the patch in #1072 fixes your issue (either building from source or downloading the plc artifact from the PR-actions) and if not, would you be willing to share the code which results in said error? Either in full or a minimal-reproducible-example would be fine.

rohanrayan commented 7 months ago

@mhasel

I am still not able to build the standard-lib files. Using the same plc.json plc.json

Here is what I did (I have installed rusty standalone. i.e I have not used the docker): -> Check out the "debug-build-err" -> Cargo build and copy schema files (which for some reason is missing from the target folder) -> plc -g build

Exact commands used: git checkout debug-build-err cargo build --release cp -r compiler/plc_project/schema/ ./target/release/ plc -g build

Error generated when executing the plc build: 'plc: /home/llvm-project-14x/llvm/lib/IR/DIBuilder.cpp:794: llvm::DILocalVariable llvm::DIBuilder::createParameterVariable(llvm::DIScope, llvm::StringRef, unsigned int, llvm::DIFile, unsigned int, llvm::DIType, bool, llvm::DINode::DIFlags, llvm::DINodeArray): Assertion `ArgNo && "Expected non-zero argument number for parameter"' failed. Aborted (core dumped)'

Am I doing something wrong?

mhasel commented 7 months ago

Hi!

Your build-commands look correct. I have replicated your steps and used your provided plc.json file (I only updated the file-paths to fit my machine). However, I still cannot reproduce this issue locally. The stdlib files compile to IR for me when passing -g. I have asked someone else on the team to try it and will get back to you once they get back to me.

Until then, you could try either running it in the dev-container or reinstalling the dev toolchain as per the documentation and see if you are able to build it then.

PS: The reason why the schema file is not in the target folder is because that folder is in our .gitignore. The schema file is optional aswell - if it's not present, the compiler will assume the json file is correct and carry on with the compilation.

ghaith commented 7 months ago

@mhasel

I am still not able to build the standard-lib files. Using the same plc.json plc.json

Here is what I did (I have installed rusty standalone. i.e I have not used the docker): -> Check out the "debug-build-err" -> Cargo build and copy schema files (which for some reason is missing from the target folder) -> plc -g build

Exact commands used: git checkout debug-build-err cargo build --release cp -r compiler/plc_project/schema/ ./target/release/ plc -g build

Error generated when executing the plc build: 'plc: /home/llvm-project-14x/llvm/lib/IR/DIBuilder.cpp:794: llvm::DILocalVariable llvm::DIBuilder::createParameterVariable(llvm::DIScope, llvm::StringRef, unsigned int, llvm::DIFile, unsigned int, llvm::DIType, bool, llvm::DINode::DIFlags, llvm::DINodeArray): Assertion `ArgNo && "Expected non-zero argument number for parameter"' failed. Aborted (core dumped)'

Am I doing something wrong?

Hello, I'm also unfortunately not getting them same error even from master Here's what I did:

cargo clean # just to be sure
cargo b --release # we usually use cargo b --workspace --release but I did only --release as this was what you used
cp -r compiler/plc_project/schema/ ./target/release/ # Not usually needed but it avoids the warning. 
cd target/release # I'm assuming this is where you are compiling the application
# add the plc json
./plc -g build
warning: ignoring debug info with an invalid version (0) in build/home/ghaith/rusty/libs/stdlib/iec61131-st/numerical_functions.ll

I'm assuming the warning is because i'm not using @mhasel's fix.

The only difference here is that you called plc without the ./ which i assumed was just a typo or possibly a shell behavior. In case it's not a default shell behavior could you check that you don't have another plc version in your path that might have been called here? You can do that with which plc on a unix based system. In case the plc binary is the correct one, could please share more info about your system? Is it a debian based system? Most of our testing is happening on a debian based system. Locally we run either the docker file or an ubuntu version with llvm14 installed from the repository. At home I have a compiled version of LLVM 14 on an Arch install and I never encountered these issues but I can verify it again. I can also probably verify it on an OpenSuse but I usualy just use the docker image there.

mhasel commented 7 months ago

@mhasel I am still not able to build the standard-lib files. Using the same plc.json plc.json Here is what I did (I have installed rusty standalone. i.e I have not used the docker): -> Check out the "debug-build-err" -> Cargo build and copy schema files (which for some reason is missing from the target folder) -> plc -g build Exact commands used: git checkout debug-build-err cargo build --release cp -r compiler/plc_project/schema/ ./target/release/ plc -g build Error generated when executing the plc build: 'plc: /home/llvm-project-14x/llvm/lib/IR/DIBuilder.cpp:794: llvm::DILocalVariable llvm::DIBuilder::createParameterVariable(llvm::DIScope, llvm::StringRef, unsigned int, llvm::DIFile, unsigned int, llvm::DIType, bool, llvm::DINode::DIFlags, llvm::DINodeArray): Assertion `ArgNo && "Expected non-zero argument number for parameter"' failed. Aborted (core dumped)' Am I doing something wrong?

Hello, I'm also unfortunately not getting them same error even from master Here's what I did:

cargo clean # just to be sure
cargo b --release # we usually use cargo b --workspace --release but I did only --release as this was what you used
cp -r compiler/plc_project/schema/ ./target/release/ # Not usually needed but it avoids the warning. 
cd target/release # I'm assuming this is where you are compiling the application
# add the plc json
./plc -g build
warning: ignoring debug info with an invalid version (0) in build/home/ghaith/rusty/libs/stdlib/iec61131-st/numerical_functions.ll

I'm assuming the warning is because i'm not using @mhasel's fix.

The only difference here is that you called plc without the ./ which i assumed was just a typo or possibly a shell behavior. In case it's not a default shell behavior could you check that you don't have another plc version in your path that might have been called here? You can do that with which plc on a unix based system. In case the plc binary is the correct one, could please share more info about your system? Is it a debian based system? Most of our testing is happening on a debian based system. Locally we run either the docker file or an ubuntu version with llvm14 installed from the repository. At home I have a compiled version of LLVM 14 on an Arch install and I never encountered these issues but I can verify it again. I can also probably verify it on an OpenSuse but I usualy just use the docker image there.

This warning originated from LLVMParseIRInContext due to a debug-info version mismatch. We specified the debug info version as 5, while LLVM 14 produces version-3 info (this can be checked dynamically with inkwell::debug_info::debug_metadata_version). I have updated the patch to align with the debug-info-metadata example provided by inkwell, which fixes this warning.

ghaith commented 7 months ago

This warning originated from LLVMParseIRInContext due to a debug-info version mismatch. We specified the debug info version as 5, while LLVM 14 produces version-3 info (this can be checked dynamically with inkwell::debug_info::debug_metadata_version). I have updated the patch to align with the debug-info-metadata example provided by inkwell, which fixes this warning.

We force version 5 because that's what we internally parse on the runtime side to read the variables. We could possibly make this configurable

mhasel commented 7 months ago

We force version 5 because that's what we internally parse on the runtime side to read the variables. We could possibly make this configurable

Since this only seems to be a problem when emitting IR, we could also dynamically change it depending on compilation flags.

rohanrayan commented 7 months ago

@mhasel @ghaith

Thank you for your response. Sorry for my late reply.

I am still not able to get it to work. I am compiling LLVM from scratch so maybe there is some mismatch there. So I will attach below all the steps that I do during the installation in detail.

Regarding trying the docker, I am not able to build it for some reason. I think this is mainly because of some proxy issues on my end. I will try to fix this and give you feedback as soon as possible.


Steps I followed for installation and running of rusty (Please let me know if anything is not clear):

Create a dir where everything needed for rusty is cloned and compiled

mkdir rusty_test

cd rusty_test

install LLVM 14x from source

git clone https://github.com/llvm/llvm-project.git --branch release/14.x --single-branch --depth 1

cd llvm-project/

mkdir build_release

cd build_release

cmake '-DLLVM_ENABLE_PROJECTS=clang;lld;' -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON -DCMAKE_CXX_FLAGS="-fPIC" -DCMAKE_C_FLAGS="-fPIC" -G "Unix Makefiles" -DLLVM_ENABLE_RTTI=ON ../llvm

make -j 5

% After LLVM installation add this folder to the LLVM_INSTALL_PATH so that the correct version of clang is detected % running "clang --version" gives the following output: % clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1) % Target: x86_64-unknown-linux-gnu % Thread model: posix % InstalledDir: /home/rusty_test/llvm-project/build_release//bin

% For some reason the lld includes are not copied to the build dir during installation % So I do this manually. If I dont do this, rusty is not able to find the includes and % cargo build fails

cp -r ../lld/include/lld/ include/

Install Rusty from sources

cd ~/rusty_test/

git clone https://github.com/PLC-lang/rusty.git

cd rusty/

cargo build --release

% I add the path to plc to my .bashrc, thus I can run 'plc' from any folder in my linux account

export PATH=/home/rusty_test/rusty/target/release:$PATH

Now try to run the plc build command on the plc.json file

plc.json

cd ~/rusty_test

mkdir test

cd test

% Copy the plc.json file here plc -g build

% At this stage, I get errors saying that the schema is not found as follows: % Could not find schema, validation skipped. Original error: GeneralError { message: "Cannot read file '/home/rusty_test/rusty/target/release/schema/plc-json.schema': File not found'", err_no: general__io_err } % plc: /home/rusty_test/llvm-project/llvm/lib/IR/DIBuilder.cpp:794: llvm::DILocalVariable llvm::DIBuilder::createParameterVariable(llvm::DIScope, llvm::StringRef, unsigned int, llvm::DIFile, unsigned int, llvm::DIType, bool, llvm::DINode::DIFlags, llvm::DINodeArray): Assertion `ArgNo && "Expected non-zero argument number for parameter"' failed. % Aborted (core dumped)

% To avoid this error, I copy the schema file manually cd ~/rusty_test/rusty/

cp -r compiler/plc_project/schema/ ./target/release/

% I then again try to build the plc.json cd ~/rusty_test/test/ plc -g build

% This also gives me the following error (as mentioned previously in this thread) % plc: /home/orr6kor/rusty_test/llvm-project/llvm/lib/IR/DIBuilder.cpp:794: llvm::DILocalVariable llvm::DIBuilder::createParameterVariable(llvm::DIScope, llvm::StringRef, unsigned int, llvm::DIFile, unsigned int, llvm::DIType, bool, llvm::DINode::DIFlags, llvm::DINodeArray): Assertion `ArgNo && "Expected non-zero argument number for parameter"' failed. % Aborted (core dumped)

% But running the normal build as follows works plc build

ghaith commented 7 months ago

I don't know if this is an issue, but we usually also build polly with llvm: This is a version of llvm I tried for a multi-arch build, it's not committed but it should work: https://github.com/PLC-lang/rust-llvm-images/blob/f6fc0c4b08b12e43553940f79a00fe92db90141d/linux/Dockerfile.multi#L78

Other differences i'm seeing here is that we don't build with PIC and we force LLVM to use lld as the linker. But I don't see how the last part should have this big of an effect. We also compile LLVM using clang not GCC but I also don't know why this would make much difference. Out of interest, does compiling a C in this clang with debug info as IR produce any issues? clang -S -emit-llvm -g file.c -o file.ll

Also this is how build it on windows (interestingly no polly): https://github.com/PLC-lang/llvm-package-windows/blob/27fbca0d88b422deac1c8ffd545394acafcb7b88/.github/workflows/build.yml#L43

rohanrayan commented 7 months ago

I tried again by compiling with polly and removing PIC and using lld as the linker as follows:

cmake '-DLLVM_ENABLE_PROJECTS=clang;lld;polly' -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_USE_LINKER=lld -G "Unix Makefiles" ../llvm

But this also gives the same error.

I ran the installed clang with a hello_world example and it generates IR without any issues: clang -S -emit-llvm -g test.c -o test.ll

ghaith commented 7 months ago

I compiled llvm with your instruction and I now get the error. I'll try to find out what's different in the build flags between this version of llvm and the one we usually have. On another PC I also run a hand compiled llvm / clang version I have to test compiling the IR there as well or find out why it does not break

ghaith commented 7 months ago

Just for the record since the build method seems to affect the result, this is how debian builds LLVM: https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/14/debian/rules

ghaith commented 7 months ago

I just checked my locally built llvm on my arch desktop and I can compile with no issues. Here is my build command for that:

❯ cmake \
        -G "Ninja" \
        -DCMAKE_BUILD_TYPE=Release \
        -DCMAKE_SYSTEM_NAME=Linux \
        -DCMAKE_C_COMPILER="clang" \
        -DCMAKE_CXX_COMPILER="clang++" \
        -DCMAKE_INSTALL_PREFIX=/home/ghaith/.local/llvm14/ \
        -DLLVM_ENABLE_PROJECTS="clang;lld;polly" \
        -DLLVM_USE_LINKER=lld \
        -DLLVM_INCLUDE_EXAMPLES=OFF \
        -DLLVM_INCLUDE_TESTS=OFF \
        -DLLVM_ENABLE_PIC=OFF \
        ../llvm

Main difference seems to be that PIC is off, and the assertions are off (or defaults). If the error is caused by an assertion this explains it. Do you mind testing if these options work for you? I assume it would still be an issue but at least we can pinpoint the difference.

rohanrayan commented 7 months ago

I tried it now with the build flags you suggested (I removed some because I usually compile with gcc and use Makefiles instead of Ninja). The final flags I used were as follows:

cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release -DCMAKE_SYSTEM_NAME=Linux -DLLVM_ENABLE_PROJECTS="clang;lld;polly" -DLLVM_INCLUDE_EXAMPLES=OFF -DLLVM_INCLUDE_TESTS=OFF -DLLVM_ENABLE_PIC=OFF ../llvm

With this, the plc -g build command works but it issues a warning as follows: warning: ignoring debug info with an invalid version (0) in build/home/rusty_test/rusty/libs/stdlib/iec61131-st/numerical_functions.ll

ghaith commented 7 months ago

Good to know, the warning you got is what @mhasel is working on now. #1072 It's still interesting that either the PIC or the Assertions (more likely) make our debug info not work on IR generation. I'll keep investigating that.

rohanrayan commented 7 months ago

I want to report another issue. When trying to compile the "num_conversion.ll" file (using a similar plc.json file that I have attached before, with just the file name changed) with the debug flags I get the following error (this is with the newly built LLVM with flags mentioned above):

error: "build/home/rusty_test/rusty/libs/stdlib/iec61131-st/num_conversion.ll:1967:8: error: Expected \'!\' here\n!593 = <temporary!> !{}\n ^\n"

Codegen Error: Compilation aborted due to previous errors

ghaith commented 7 months ago

I want to report another issue. When trying to compile the "num_conversion.ll" file (using a similar plc.json file that I have attached before, with just the file name changed) with the debug flags I get the following error (this is with the newly built LLVM with flags mentioned above):

error: "build/home/rusty_test/rusty/libs/stdlib/iec61131-st/num_conversion.ll:1967:8: error: Expected '!' here\n!593 = <temporary!> !{}\n ^\n"

Codegen Error: Compilation aborted due to previous errors

@mhasel I think this is what you fixed in #1072 if i'm not mistaken? @rohanrayan The problem here is we are generating debug information for functions that don't exist. In a normal binary run (not IR) this does not matter because LLVM just removes it. But for IR it gets validated. The fix in #1072 should fix that I believe.

mhasel commented 7 months ago

@mhasel I think this is what you fixed in #1072 if i'm not mistaken? @rohanrayan The problem here is we are generating debug information for functions that don't exist. In a normal binary run (not IR) this does not matter because LLVM just removes it. But for IR it gets validated. The fix in #1072 should fix that I believe.

Yes, this issue should be fixed in #1072.

rohanrayan commented 7 months ago

Just one more question. Is there a way to specify other flags like -fno-inline to the plc build command?

rohanrayan commented 7 months ago

@mhasel I think this is what you fixed in #1072 if i'm not mistaken? @rohanrayan The problem here is we are generating debug information for functions that don't exist. In a normal binary run (not IR) this does not matter because LLVM just removes it. But for IR it gets validated. The fix in #1072 should fix that I believe.

Yes, this issue should be fixed in #1072.

I checked and this commit indeed fixes this issue.

However I find other issues now. Specifically when trying to compile "string_conversion.st", I get the following error:

error: Invalid assignment: cannot assign '__POINTER_TO_CHAR' to 'REF_TO STRING'
    ┌─ /home/rusty_test/rusty/libs/stdlib/iec61131-st/string_conversion.st:118:5
    │
118 │     ptr := &in;
    │     ^^^^^^^^^^ Invalid assignment: cannot assign '__POINTER_TO_CHAR' to 'REF_TO STRING'

Also, please let me know if you want me to create new issues as this thread is getting long.

Thanks