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

Need to re-fix Windows manifest embedding. #11207

Closed
vadimcn opened this issue Dec 30, 2013 · 19 comments
Closed

Need to re-fix Windows manifest embedding. #11207

vadimcn opened this issue Dec 30, 2013 · 19 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. O-windows Operating system: Windows

Comments

@vadimcn
Copy link
Contributor

vadimcn commented Dec 30, 2013

The previous attempt (#10878) to fix #10512 has a couple of problems:

  1. Sometimes UpdateResource corrupts executables produced by binutils. (The executable still runs, however tools like dumpbin report corrupted PE header, and gdb crashes when trying to read debug info).
  2. More importantly, this approach does not work for cross-compilation, because, well, you can't call Windows API if the host system is not Windows.
@alexcrichton
Copy link
Member

So that's why I haven't been able to use gdb recently! That also explains why a recent attempt at objdump didn't succeed.

Sounds like we may need to revert the manifest embedding for now?

@vadimcn
Copy link
Contributor Author

vadimcn commented Dec 30, 2013

@alexcrichton: Yes, probably.

@vadimcn
Copy link
Contributor Author

vadimcn commented Dec 30, 2013

Regarding fixing this, I see a few approaches:

  • Have rustc write out a .rc file (resource source file), then use windres tool to compile it to an object file. Pass resulting object file to the linker.
    • Pros: Flexible. Manifest can be modified per-executable (e.g. to change the requested invocation privilege level).
    • Cons: Requires invocation of an external tool.
  • Include a .rc file containing the manifest in rust source tree, compile it using windres during librustc build, include produced object file into librustc via include_bin!(...). When compiling executable targeting Windows, write binary blob into .o file, and pass it to the linker.
    • Pros: Don't need to invoke windres tool on every compilation.
    • Cons: Manifest can't be altered without rebuilding rust compiler.
  • Create .o file containing manifest directly.
    • Pros: Flexible. Can alter manifest as needed.
    • Cons: Complexity. Will need to implement a COFF file writer.
  • Link executable without resources, then insert resource section into the output .exe file (without using Windows' UpdateResource API).
    • Pros: Flexible. Can alter manifest as needed.
    • Cons: Complexity. Need to parse/alter/emit COFF headers. Though probably less complex than approach No3.

Currently we have no need to alter .exe manifests, and if really needed this can always be done afterwards using windres tool, so I am leaning towards option 2.

@alexcrichton
Copy link
Member

Do you know the format of the COFF file? LLVM certainly has the ability to write a COFF file, and we might be able to just use their bindings to emit our own COFF file ourselves.

They seem to have lots and lots of header files and defines related to COFF object files, so seems promising? Of the options, using LLVM to generate an object and then passing that to the linker sounds the most appealing to me.

@vadimcn
Copy link
Contributor Author

vadimcn commented Dec 30, 2013

COFF format is well-documented, but yes, it's complicated.

Regarding LLVM: as far as I know, it is not possible to create PE COFF resource section even in native assembly, much less in LL code. I actually investigated this approach, but ran into inability to emit RVA offsets. Apparently this can only be done by writing .obj file directly.

bors added a commit that referenced this issue Dec 31, 2013
In view of the problems outlined in #11207, I think manifest embedding should be removed, until we find a better solution.  :-(
@yuriks
Copy link
Contributor

yuriks commented Aug 21, 2014

As an additional option, could a combination of options 1 and 2 work? Ship a default compiled resource file with rustc, but allow it to be overriden by the user if he wants to. (In which case he must pay the "cost" of requiring windres for that to work.)

@vadimcn
Copy link
Contributor Author

vadimcn commented Oct 31, 2014

FWIW, UAP installer detection heuristics may be suppsessed by setting this environment variable: __COMPAT_LAYER=RunAsInvoker.

@alexcrichton
Copy link
Member

@vadimcn I'm not sure I ever really fully understood what manifests were or why we were embedding them, but given today's landscape of Rust-on-Windows does this still make sense? Is this something we should do on MSVC as well as MinGW?

@vadimcn
Copy link
Contributor Author

vadimcn commented Sep 28, 2015

Manifests are just metadata about the executable that tells Windows more about its requirements and capabilities. One of the most frequent uses is to specify whether the program expects to run with admin privileges (otherwise Windows will try to guess - for compatibility with pre-Vista executables), but there other features beyond that.

MSVC linker will embed a default manifest by, um, default. As for MinGW - see above.

@alexcrichton
Copy link
Member

Ah ok, so in that case should is the MSVC manifest that's automatically embedded a well known manifest? If so then it sounds like this is still relevant for MinGW and it'd just be one of the strategies you outlined above.

@taralx
Copy link
Contributor

taralx commented Apr 17, 2016

As a note - rustup-setup was just renamed rustup-init because of this issue.

@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 20, 2017
@steveklabnik
Copy link
Member

Triage: not aware of any changes here

@jonas-schievink
Copy link
Contributor

I suggest closing this in favor of rust-lang/rfcs#721 and the embed_resource crate.

@jpoles1
Copy link

jpoles1 commented Jan 13, 2020

@jonas-schievink can you please elaborate on the steps required to embed a manifest using the embed_resource crate?

@jonas-schievink
Copy link
Contributor

@jpoles1 I've never used it, but you can check the docs and crates that use it

@jpoles1
Copy link

jpoles1 commented Jan 13, 2020

@jonas-schievink I read the docs and still unsure of how to embed the manifest. Are you aware of any crates that use the library in this manner? Understand if not, and thanks for your assistance.

@jonas-schievink
Copy link
Contributor

cargo-update (by the same author) uses it: https://github.com/nabijaczleweli/cargo-update/blob/master/build.rs

@jpoles1
Copy link

jpoles1 commented Jan 13, 2020

@jonas-schievink, that's perfect, thanks for the tip!

My specific use case was to programatically enable admin permissions on my .exe file, the steps I used to get this working are as follows:

  1. Add the following to your cargo.toml:
[build-dependencies]
embed-resource = "1.3"
  1. In your project root directory, add a file named build.rs with the following:
extern crate embed_resource;
fn main() {
    embed_resource::compile("app-name-manifest.rc");
}
  1. In your project root directory, add a file named app-name-manifest.rc with the following:
#define RT_MANIFEST 24
1 RT_MANIFEST "app-name.exe.manifest"
  1. In your project root directory, add a file named app-name.exe.manifest with the following:
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
    <trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
        <security>
            <requestedPrivileges>
                <requestedExecutionLevel level="requireAdministrator" uiAccess="false"/>
            </requestedPrivileges>
        </security>
    </trustInfo>
</assembly>
  1. Build your project!

@steveklabnik
Copy link
Member

Great! Let's close in favor of those.

flip1995 pushed a commit to flip1995/rust that referenced this issue Jul 31, 2023
[`needless_pass_by_ref_mut`]: Do not lint if passed as a fn-like argument

Fixes rust-lang#11182 and also fixes rust-lang#11199 (though this is kind of a duplicate)

There's likely a case or two I've missed, so this likely needs a bit more work but it seems to work fine with the tests I've added.

PS, the diff for the test is useless because it iterates over a hashmap before linting. Seems to work fine but we could maybe change this for consistency's sake

changelog: [`needless_pass_by_ref_mut`]: No longer lints if the function is passed as a fn-like argument
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. O-windows Operating system: Windows
Projects
None yet
Development

No branches or pull requests

9 participants