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.

Sign up for GitHub

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

Jump to bottom

Add support for QNX Neutrino #507

Open

gh-tr wants to merge 6 commits into rust-lang:master from gh-tr:20221220_process_map_fix

gh-tr commented Dec 22, 2022

Add support for QNX Neutrino.
-- Tested on a Neutrino target for both aarch64 and x86_64 running 7.1.

Change Description:

Splits the existing functionality (default) into one module and the nto into another
On QNX, there is a header in the pmap file so added functionality to skip the first line of the file /proc/self/pmap
Bumped libc version to the first version containing a Neutrino target.

gh-tr and others added 3 commits

December 19, 2022 17:03


          Add support for QNX/Neutrino

5c36819


          Initial attempt at adding process map parsing

12afb11


          Split Linux and nto code into separate mods

d528ac4

gh-tr mentioned this pull request

Add symbolize support for qnx-nto #506

Closed


          Fixing automated formatting

Contributor

flba-eb commented Jan 9, 2023

@Veykril: Could you please take a look at this PR? Would it make sense to close #506 in favor of this PR?

flba-eb mentioned this pull request

Add support for QNX Neutrino to standard library rust-lang/rust#106673

Merged


          Merged separate OS implementations into the original source file as t…

75e65e7

…he automated build system is unhappy about naming conventions on 1.42.0

gh-tr force-pushed the 20221220_process_map_fix branch from 9839937 to 75e65e7 Compare

January 11, 2023 15:03

workingjubilee reviewed

View reviewed changes

src/symbolize/gimli/parse_running_mmaps_unix.rs Outdated Show resolved Hide resolved

src/symbolize/gimli/parse_running_mmaps_unix.rs Outdated Show resolved Hide resolved

bors added a commit to rust-lang-ci/rust that referenced this pull request


          Auto merge of rust-lang#106673 - flba-eb:add_qnx_nto_stdlib, r=workin…

864b625

…gjubilee

Add support for QNX Neutrino to standard library

This change:

- adds standard library support for QNX Neutrino (7.1).
- upgrades `libc` to version `0.2.139` which supports QNX Neutrino

`@gh-tr`

⚠️ Backtraces on QNX require rust-lang/backtrace-rs#507 which is not yet merged! (But everything else works without these changes) ⚠️

Tested mainly with a x86_64 virtual machine (see qnx-nto.md) and partially with an aarch64 hardware (some tests fail due to constrained resources).

saethlin pushed a commit to saethlin/miri that referenced this pull request


          Auto merge of #106673 - flba-eb:add_qnx_nto_stdlib, r=workingjubilee

7b3ec15

Add support for QNX Neutrino to standard library

This change:

- adds standard library support for QNX Neutrino (7.1).
- upgrades `libc` to version `0.2.139` which supports QNX Neutrino

`@gh-tr`

⚠️ Backtraces on QNX require rust-lang/backtrace-rs#507 which is not yet merged! (But everything else works without these changes) ⚠️

Tested mainly with a x86_64 virtual machine (see qnx-nto.md) and partially with an aarch64 hardware (some tests fail due to constrained resources).


          Reduce visibility back to pub(super)

84fd7c8

thomcc pushed a commit to tcdi/postgrestd that referenced this pull request


          Auto merge of #106673 - flba-eb:add_qnx_nto_stdlib, r=workingjubilee

1ca9c43

Add support for QNX Neutrino to standard library

This change:

- adds standard library support for QNX Neutrino (7.1).
- upgrades `libc` to version `0.2.139` which supports QNX Neutrino

`@gh-tr`

⚠️ Backtraces on QNX require rust-lang/backtrace-rs#507 which is not yet merged! (But everything else works without these changes) ⚠️

Tested mainly with a x86_64 virtual machine (see qnx-nto.md) and partially with an aarch64 hardware (some tests fail due to constrained resources).

workingjubilee closed this

workingjubilee reopened this

workingjubilee reviewed

View reviewed changes

Contributor

workingjubilee left a comment •

edited

I had half-expected someone else with a bit more experience to come by and review this, so I kept my earlier comments minimal, but it looks like I probably now know the most about QNX Neutrino amongst the libs reviewers, so I might as well sit down with this. Sorry about the holdup.

src/symbolize/gimli/parse_running_mmaps_unix.rs

Comment on lines +56 to +70

    
              fn concat_str(a: &str, b: &str) -> String {

                  let mut e = String::from(a);

                  e += b;

                  e

              }

              fn read_file(file: &str) -> Result<String, String> {

                  let mut proc_self_maps =

                      File::open("/proc/self/maps").map_err(|_| "Couldn't open /proc/self/maps")?;

                      File::open(file).map_err(|_| concat_str("Couldn't open file: ", file))?;

                  let mut buf = String::new();

                  let _bytes_read = proc_self_maps

                      .read_to_string(&mut buf)

                      .map_err(|_| "Couldn't read /proc/self/maps")?;

                  for line in buf.lines() {

                      v.push(line.parse()?);

                  }

                      .map_err(|_| concat_str("Couldn't read file: ", file))?;

                  Ok(buf)

              }

Contributor

workingjubilee Jun 22, 2023

This machinery is a reimplementation of fs::read_to_string. You can import fs by changing

- use super::mystd::fs::File;
+ use super::mystd::fs::{self, File};

If you must, you can use map_err for the Err variant.

src/symbolize/gimli/parse_running_mmaps_unix.rs

Comment on lines +96 to +99

+              #[cfg(not(target_os = "nto"))]
+              pub fn config() -> (&'static str, usize) {
+                  ("/proc/self/maps", 0)
+              }

Contributor

workingjubilee Jun 22, 2023

This moves to runtime what can be known at compile-time. Please describe a static and a const instead, with appropriate names. If you wish, you can put them in a mod config {}.

src/symbolize/gimli/parse_running_mmaps_unix.rs

Comment on lines +75 to +78

+                  parse_maps_lines(&content, skip)
+              }
+              fn parse_maps_lines(content: &str, skip: usize) -> Result<Vec<MapsEntry>, String> {

Contributor

workingjubilee Jun 22, 2023

I believe this is still functionally identical if we simply delete these lines?

Suggested change

      
                parse_maps_lines(&content, skip)
          
            }
          
            fn parse_maps_lines(content: &str, skip: usize) -> Result<Vec<MapsEntry>, String> {

src/symbolize/gimli/parse_running_mmaps_unix.rs

@@ @@ -162,7 +243,7 @@ fn check_maps_entry_parsing_64bit() { @@
                   assert_eq!(
                       "7f5985f46000-7f5985f48000 rw-p 00039000 103:06 76021795                  \
-                              /usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2"
+                      /usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2"

Contributor

workingjubilee Jun 22, 2023

Is there a reason for diffing this line?

src/symbolize/gimli/parse_running_mmaps_unix.rs

               #[test]
               fn check_maps_entry_parsing_64bit() {
                   assert_eq!(
                       "ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0                  \
-                              [vsyscall]"
+                      [vsyscall]"

Contributor

workingjubilee Jun 22, 2023

Is there a reason for diffing this line?

src/symbolize/gimli/parse_running_mmaps_unix.rs

Comment on lines +179 to +190

+                      let vaddr_str = parts.next().ok_or("Couldn't find virtual address")?;
+                      let size_str = parts.next().ok_or("Couldn't find size")?;
+                      let _flags_str = parts.next().ok_or("Couldn't find flags")?;
+                      let prot_str = parts.next().ok_or("Couldn't find protection")?;
+                      let _maxprot_str = parts.next().ok_or("Couldn't find maximum protection")?;
+                      let dev_str = parts.next().ok_or("Couldn't find device")?;
+                      let ino_str = parts.next().ok_or("Couldn't find inode")?;
+                      let offset_str = parts.next().ok_or("Couldn't find offset")?;
+                      let _rsv_str = parts.next().ok_or("Couldn't find reserved pages")?;
+                      let _guardsize_str = parts.next().ok_or("Couldn't find guard size")?;
+                      let _refcnt_str = parts.next().ok_or("Couldn't find reference count")?;
+                      let _mapcnt_str = parts.next().ok_or("Couldn't find mapped count")?;

Contributor

workingjubilee Jun 22, 2023 •

edited

Just to be clear, on QNX Neutrino these are all required fields? Is there somewhere this format is documented for Neutrino? It seems to be a superset of the other format described here?

src/symbolize/gimli/parse_running_mmaps_unix.rs

-                  Ok(v)
+              pub(super) fn parse_maps() -> Result<Vec<MapsEntry>, String> {
+                  let (file, skip) = config();
+                  let content = read_file(file)?;

Contributor

workingjubilee Jun 22, 2023

Suggested change

      
                let content = read_file(file)?;
          
                let content = fs::read_to_string(file).map_err(|e| e.to_string())?;

src/symbolize/gimli/parse_running_mmaps_unix.rs

+                          |s: &str| usize::from_str_radix(&s[2..], 16).map_err(|_| "Couldn't parse hex number");
+                      let address = { (hex(vaddr_str)?, hex(vaddr_str)? + hex(size_str)?) };
+                      // TODO: Probably a rust'ier way of doing this

Contributor

workingjubilee Jun 22, 2023 •

edited

You could probably cook something up using array::from_fn but I'm not going to block this on that. If you decide not to, though, please remove the TODO.

workingjubilee self-assigned this

workingjubilee closed this

workingjubilee reopened this

workingjubilee mentioned this pull request

infer_current_exe: Avoid picking non-path from maps #563

Closed

RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request


          Auto merge of #106673 - flba-eb:add_qnx_nto_stdlib, r=workingjubilee

c1218ff

Add support for QNX Neutrino to standard library

This change:

- adds standard library support for QNX Neutrino (7.1).
- upgrades `libc` to version `0.2.139` which supports QNX Neutrino

`@gh-tr`

⚠️ Backtraces on QNX require rust-lang/backtrace-rs#507 which is not yet merged! (But everything else works without these changes) ⚠️

Tested mainly with a x86_64 virtual machine (see qnx-nto.md) and partially with an aarch64 hardware (some tests fail due to constrained resources).

RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request


          Auto merge of #106673 - flba-eb:add_qnx_nto_stdlib, r=workingjubilee

b576765

Add support for QNX Neutrino to standard library

This change:

- adds standard library support for QNX Neutrino (7.1).
- upgrades `libc` to version `0.2.139` which supports QNX Neutrino

`@gh-tr`

⚠️ Backtraces on QNX require rust-lang/backtrace-rs#507 which is not yet merged! (But everything else works without these changes) ⚠️

Tested mainly with a x86_64 virtual machine (see qnx-nto.md) and partially with an aarch64 hardware (some tests fail due to constrained resources).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment