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

Missing lifetime specifier in generated code #1238

Closed
SKyletoft opened this issue Feb 16, 2023 · 6 comments · Fixed by #1254
Closed

Missing lifetime specifier in generated code #1238

SKyletoft opened this issue Feb 16, 2023 · 6 comments · Fixed by #1254
Labels
bug Something isn't working

Comments

@SKyletoft
Copy link

SKyletoft commented Feb 16, 2023

Expected Behavior

There would be a lifetime specifier in the generated function

Actual Behavior

Borrowchecking error
Error log

Steps to Reproduce the Problem

  1. Include this header
  2. generate! the type s2e::S2E

Reproduced in this repo

Specifications

  • Version: autocxx 0.24.0, Rust 1.66.1
  • Platform: Linux NixOS Unstable x64 (nixpkgs), Ubuntu 22.04 x64 (rustup)
@adetaylor
Copy link
Collaborator

Thanks, I'm traveling this week but I'll take a look next week. It would be even better if you could raise a PR with a failing integration test.

@adetaylor adetaylor added the bug Something isn't working label Feb 17, 2023
@SKyletoft
Copy link
Author

I've been trying to figure it out. I have this command (running in an Ubuntu VM with the repo linked above):

autocxx-reduce \
	--problem="error[E0106]" \
	--clang-arg="-std=gnu++17" \
	--clang-arg="-DBOOST_BIND_GLOBAL_PLACEHOLDERS=1" \
	--clang-arg="-DTARGET_PAGE_BITS=12" \
	--clang-arg="-DSE_RAM_OBJECT_BITS=12" \
	-k \
	file \
	-d "safety!(unsafe_ffi)" \
	-d 'generate!("s2e::S2E")' \
	-I "/home/u3836/git/s2e-autocxx-error/s2e/klee/include/" \
	-I "/home/u3836/git/s2e-autocxx-error/s2e/libcoroutine/include/" \
	-I "/home/u3836/git/s2e-autocxx-error/s2e/libcpu/include/" \
	-I "/home/u3836/git/s2e-autocxx-error/s2e/libfsigc++/include/" \
	-I "/home/u3836/git/s2e-autocxx-error/s2e/libq/include/" \
	-I "/home/u3836/git/s2e-autocxx-error/s2e/libs2e/include/" \
	-I "/home/u3836/git/s2e-autocxx-error/s2e/libs2ecore/include/" \
	-I "/home/u3836/git/s2e-autocxx-error/s2e/libs2eplugins/include/" \
	-I "/home/u3836/git/s2e-autocxx-error/s2e/libtcg/include/" \
	-I "/home/u3836/git/s2e-autocxx-error/s2e/libvmi/include/" \
	-I "/home/u3836/git/s2e-autocxx-error/s2e/tools/lib/X8664BitcodeLibrary" \
	--header "/home/u3836/git/s2e-autocxx-error/s2e/libs2ecore/include/s2e/S2E.h"

I had to patch autocxx_engine to use emit a -std=gnu++17 because passing that flag via --clang-arg doesn't seem to be respected by clang given that it follows a -std=c++14.

But this still spits out slightly over 200k lines of C++, so I assume I'm missing something.

@adetaylor
Copy link
Collaborator

Thanks, yeah, autocxx-reduce (when it's working) will shrink that down to just 2-3 lines of code! However, it's quite pernickety to get working, because it has to make assumptions about your local build environment (as you noticed).

Let me see if I can reproduce it using the example repo you gave.

@adetaylor
Copy link
Collaborator

I can reproduce this with your provided standalone repo (thanks for doing that), I made a repro.json and I'm now running it through the reduction pipeline. From here on it's just a matter of time (possibly a week) until it spits out a minimized test case.

@adetaylor
Copy link
Collaborator

Here is the minimized test case spat out by the reduction pipeline:

namespace a {
class b;
}
namespace s2e {
class c;
class f {
  a::b d();
};
class S2E {
public:
  f e;
  a::b &d(c *) const;
};
} // namespace s2e

I expect it can be trimmed down a little more by hand, but that should give me enough to work with to diagnose the problem. I'm in the middle of a few other things right now but will take a look eventually. Thanks for the report, and thanks for making it possible to get a minimal test case like this.

@adetaylor
Copy link
Collaborator

I think I see what's going on here.

The generated code (about which rustc complains) is:

pub unsafe fn S2E_d_autocxx_wrapper_0xa21e7b0ce9f73499 (autocxx_gen_this : & S2E , arg1 : * mut c) -> Pin < & mut b > ;

The code generator assumed that lifetimes could be elided in this case, but they can't, because the the input reference is & and the output reference is &mut. Of course, explicit lifetimes also can't help here - I think we need to reject such functions, at least until all the CppRef stuff enables us to move beyond normal Rust reference semantics.

adetaylor added a commit that referenced this issue Feb 21, 2023
Fixes #1238.

This also takes the opportunity to:
* improve diagnostics for some of the existing cases of functions
  rejected for reference shenanigans;
* be more consistent about how we determine 'reference-ness'
  of return values and parameters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants