dump-ast example program hits assertion: `!PA.isArgIdent(Idx)' failed. #92
Comments
|
Anyway, probably you aren't holding it wrong. The reality is just that PASTA does some sketchy things to achieve its goals. Here's how I would proceed: Patch the following code into auto fd = open("/tmp/source.cpp", O_TRUNC | O_CREAT | O_WRONLY, 0666);
write(fd, ast->preprocessed_code.data(), ast->preprocessed_code.size());
close(fd);Then build and run, and attach |
|
Thanks for the explanation and next steps. The "source.cpp" is too large to paste here, so I created a repo here: https://github.com/seelabs/pasta-debugging/tree/main. "source.cpp" is here: https://github.com/seelabs/pasta-debugging/blob/main/source.cpp and the gdb transcript is here: https://github.com/seelabs/pasta-debugging/blob/main/gdb.txt with both I also looked at source.cpp around line 1716508. Here's what appears there: |
|
@pgoodman I found a simple way to reproduce the error.
So the culprit is I'll note that the problem is not specific to |
|
Okay this is a bug in one of our patches: We've made this patch with the intent of implicitly converting things like |
|
Thanks! I made the suggested change and that successfully ran against the minimal repro. I then tried against my real codebase and got: I don't know if that's related to disabling the "unknown attribute" patch or not. (I know you don't have time to dig into this now, I just thought I'd report it anyway). |
|
The missing builtin looks to be a GCC-specific intrinsic that Clang doesn't seem to have. There are a number of others that are related, and could be harvested from this list. Fixing this involves adding entries into this file. This mirrors how Clang adds them, but we do it retroactively and in a slightly hacky way that generally gets the job done without having to patch Clang. Could you try making the following additions to our BuiltinsX86.h file, and testing them out? If they work, please submit a PR :-) |
|
I seem to be getting the same error as before. I modified the file |
|
Oh I'm a dummy. I shouldn't have wrapped all those builtin names with double quotes. Can you try with this? |
|
Still no go, but a different error: I went into gdb and looked at the offending |
|
From our dumped The actual line it's pointing at is the one with |
|
Try this? |
|
No go, but a different error: |
|
Getting closer :-) Seems like we're missing that intrinsic too. |
|
Just a quick update: I'm attempting to add the needed intrinsics. I'm having trouble finding the documentation I need for these missing functions (the gcc manual doesn't seem to have them) and also the correct strings for the types. I'll spend time after work to poke at this, but it may be a while before I'm able to make a patch (but hopefully I'll learn something along the way). |
|
Oh I'm making a script for this! I'll be bulk grabbing them from CBMC, which has a lot of forward declarations. It turns out there is some other stuff in the Python API that has required fixing in order for me to make the script, so it's taking me a bit of time :-) |
|
My in-progress work is in this branch. |
|
Here's what I mean from taking the declarations from CBMC. The idea is to auto-generate the |
|
@seelabs Can you try with this version of BuiltinsX86.h, and let me know if it makes any more progress? Thanks! |
|
Yep, progress, but it looks like we're still missing some: |
|
Hi @seelabs, I've updated the previously linked |
|
I still get an error, but a different one. It's now complaining about |
|
I may need to patch clang if I want to generically handle (re)definitions of possible intrinsics. This will take deeper investigation. |
|
Note to self: Try adding a check for /// canRedefineFunction - checks if a function can be redefined. Currently,
/// only extern inline functions can be redefined, and even then only in
/// GNU89 mode.
static bool canRedefineFunction(const FunctionDecl *FD,
const LangOptions& LangOpts) {
return ((FD->hasAttr<GNUInlineAttr>() || LangOpts.GNUInline) &&
!LangOpts.CPlusPlus &&
FD->isInlineSpecified() &&
FD->getStorageClass() == SC_Extern);
} |
|
@pgoodman Just a quick note that I'll be traveling Thursday afternoon-Sunday and will be AFK. |
|
Alright I've recently been making progress on this issue. I've been moving our various clang patches into our pasta branch of trail-of-forks/llvm-project. This has been going on as I port things over to Clang 17. |
|
@seelabs Alright, the llvm17 branch of PASTA now works with |
* Initial changes prior to running bootstrap on clang 17 * Basic support for clang 17 for BootstrapMacros. * More changed for llvm17. I got confused with the new arg parsing; didn't realize that they've introduced new overloads. * Bootstrapped macro file. * Bootstrapped LLVM17, using LLVM17. * Bootstrapped to LLVM17. * Try to integrate printing the attribute itself. * Use '~' as our way of handling builtins. * Fix bindings * Vendored LLVM and nanobind. * Disable mlir in vendored llvm * Try to fix the workflow * More tweaks to try to make CI / submodule configuration work. * Move quotes around * Making a big fatal error message to understand how ci sees things * Accidentally pointed cmake at the wrong directory. * I don't think cmake liked those backslashes * Fix up the execute process * Minor fix. * Made nanobind fetch recursive. Share the install directory between pasta and its vendored deps if pasta is being installed. * Update vendoring of LLVM/Clang to build with C++17. * Try to track the pasta branch of llvm-project * Make llvm-project track its pasta branch. * Updates pasta to track llvm17 branch. * Fix out-of-bounds access, triggered when including stop_token. XREF issue #92. * Fixes some issues in discovering the isysroot. * Update to latest llvm-project * Update to latest llvm-project * Update to latest llvm-project pasta branch. Has fixes for annotation attributtes. * Add some keywords to align tokens for regonitition. Fix a dumb typo in the llvm vendoring cmake * Fiddle with typeloc visitors in bounds. * Re-bootstrap with some nullable types. Fix some bugs in token alignment and bounds. * Disable debug printing * Workflow fix * Update LLVM to fix bug. Simplify pastaConfig.cmake.in. Add more nullable methods * Disable python bindings in ci.
|
@seelabs if you have the time, can you test the |
|
@pgoodman Nice! I'll test this after work today. |
|
@pgoodman I built the master branch and ran the following command (similar to what I ran before, but using I get the following error: I'll try to play with this some more later. I know we ran into similar errors earlier (I did not patch Edit: Just for grins, I tried to use the clang that past built ( |
|
Excellent. It seems I didn't sufficiently patch clang. I thought I had covered this "definition of builtin function" error but didn't. This gives me something to go on :-) |
|
First, just to confirm, are you building the |
|
I built the tip of master ( Which I think is the same as the readme except for the install path. |
|
That should be fine :-) And things are still broken in the same way? If so, can you confirm that vendor/llvm-project/src is at or after 4ab1f8b? |
|
vendor/llvm-project/src is at Edit: Let me try repulling the repo and seeing if I can figure out how to get the right version before I build. |
|
I just now re-pulled and confirmed that vendor/llvm-project/src is at |
|
I just recompiled and re-ran. I'm still seeing the error: |
|
Alright, I'm finally reproducing this: With: |
|
Alright, I think that one issue is that I'm leaning in the direction of totally disabling the diagnostic in Clang, rather than making it conditional on something being a pasta builtin. |
|
Urgh, |
|
Nice! I'm glad you're able to repro this locally, that's a big step for sure. |
|
Dealing with |
I read about pasta on your blog post and decide to play with it. I am not familiar with the internals of libclang or llvm, so this issue may be user error.
I successfully built pasta and was able to run the demo programs. When I tried the
dump-astexample on a toy example it worked as expected. However, when I tried it against my real project, it hits an assert. In particular, when I run it against this project: https://github.com/XRPLF/rippled using the following command line:It hits the following assert:
I attempted to debug this. I built the debug version of the libraries and went into gdb and attempted to try to find the offending code so I could create a minimal repro, but unfortunately my unfamiliarity with llvm is an obstacle. Two things we may be able to use as staring points.
In SemaType.cpp:8299
So the offending attribute is called "access".
In ParseDeclCXX.cpp:2107
What I'm trying to do here is find the name of the file that contains the offending code. Some searching pointed me to
getPresumedLoc, but clearly<pasta-input>isn't useful to find the offending code. Perhaps you have a suggestion to find the code?Also, I should mention I was only playing with pasta. Pasta obviously works and it's very possible I'm "holding it wrong". I'm not expecting help here, but thought I'd report the issue in case it was a possible bug in pasta (and will help trying to debug it after regular work hours if it's helpful to you).
The text was updated successfully, but these errors were encountered: