Support the rustc metadata for AIX #107583

EsmeYi · 2023-02-02T04:33:04Z

Support the rustc metadata for rlibs and dylibs on AIX.
XCOFF is the object file format on AIX.

rustbot · 2023-02-02T04:33:11Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Nilstrieb (or someone else) soon.

Please see the contribution instructions for more information.

Noratrieb · 2023-02-02T10:56:37Z

Marking was waiting-on-author as it currently doesn't build and seems to require upstream changes in object first.
@rustbot author

philipc · 2023-02-03T04:40:27Z

compiler/rustc_codegen_ssa/src/back/metadata.rs

@@ -93,6 +101,27 @@ pub(super) fn search_for_section<'a>(
        .map_err(|e| format!("failed to read {} section in '{}': {}", section, path.display(), e))
 }

+pub(super) fn get_metadata_xcoff<'a>(path: &Path, data: &'a [u8]) -> Result<&'a [u8], String> {
+    let buf = search_for_section(path, data, ".info")?;


How do we know that the first entry in the .info section is our metadata? Are we sure that rustc is the only source of data in .info? According to the documentation, we should be using a symbol to locate it instead:

The n_name field is preserved by the binder. An application-defined unique name in this field can be used to filter access to only those comment section strings intended for the application.

For some object files, multiple segments may contain sections with the same name. However as I know, with XCOFF there may be multiple sections with type info, but their names must be different. Therefore, the ".info" section can be specific for metadata.

My understanding is that .info may contain multiple comments, and we need to use a symbol to locate the comment in .info that rustc added.

I see.
We've added a symbol with value of 4, which specifies the offset in comment section, to refer to the rust metadata.

// Add a symbol referring to .info section. // Storage class will be handled in object crate. file.add_symbol(Symbol { name: "info".into(), value: 4, size: 0, kind: SymbolKind::Unknown, scope: SymbolScope::Dynamic, weak: false, section: SymbolSection::Section(section), flags: SymbolFlags::Xcoff { n_sclass : xcoff::C_INFO }, });

Do you mean that we should use the value in the symbol to locate the metadata instead of reading the first entry directly? In the meanwhile, we can change the name 'info' to 'rustc' to make the symbol be the identifier?

Based on the documentation, yes.

philipc · 2023-02-03T04:42:21Z

compiler/rustc_codegen_ssa/src/back/metadata.rs

+    // Add a symbol referring to .info section.
+    // Storage class will be handled in object crate.
+    file.add_symbol(Symbol {
+        name: "info".into(),


Maybe this should be the identifier that we use for section names in other formats (.rmeta and .rustc).

For the .rustc section the symbol has a unique name derived from the crate hash I believe.

philipc · 2023-02-03T04:44:05Z

compiler/rustc_codegen_ssa/src/back/metadata.rs

+        name: "info".into(),
+        value: 4,
+        size: 0,
+        kind: object::SymbolKind::Null,


Change to SymbolKind::Unknown and use SymbolFlags::Xcoff to set this to C_INFO.

philipc · 2023-02-03T04:48:07Z

compiler/rustc_codegen_ssa/src/back/metadata.rs

+        b".info".to_vec(),
+        SectionKind::Debug,
+    );
+    file.add_file_symbol(".file".into());


The documentation says that .file is used when there is a file auxiliary symbol. Ideally all handling of .file would be internal to the object crate, so here we would specify the actual file name we want.

bzEq · 2023-03-20T03:11:29Z

compiler/rustc_codegen_ssa/src/back/metadata.rs

@@ -60,16 +60,24 @@ impl MetadataLoader for DefaultMetadataLoader {
                    let data = entry
                        .data(data)
                        .map_err(|e| format!("failed to parse rlib '{}': {}", path.display(), e))?;
-                    return search_for_section(path, data, ".rmeta");
+                    if target.is_like_aix {


Though xcoff is bound to AIX, I think checking the format is more precise.

We don't have format info in this function. While according to your comments, I changed such checks in fn create_wrapper_file() and fn create_compressed_metadata_file(). Thanks.

bzEq · 2023-03-20T03:11:42Z

compiler/rustc_codegen_ssa/src/back/metadata.rs

-    fn get_dylib_metadata(&self, _target: &Target, path: &Path) -> Result<MetadataRef, String> {
-        load_metadata_with(path, |data| search_for_section(path, data, ".rustc"))
+    fn get_dylib_metadata(&self, target: &Target, path: &Path) -> Result<MetadataRef, String> {
+        if target.is_like_aix {


Same as above.

compiler/rustc_codegen_ssa/src/back/metadata.rs

bzEq · 2023-03-20T03:20:22Z

compiler/rustc_codegen_ssa/src/back/metadata.rs

+    let buf = search_for_section(path, data, ".info")?;
+    let offset = file.symbol_address_by_name("rustc")?;
+    // The first 4 bytes from offset are length field for rustc metadata.
+    if offset + 4 >= buf.len() {


This should be handled by object crate. For AIX toolchain, a C_INFO symbol's offset inside .info section doesn't count the first 4-bytes length field. When reading the object file, we can set C_INFO ObjectSymbol's address the offset into .info section and the size can be initialized from the first 4-bytes length field.

This should be handled by object crate.

I think it will be a little wired if we add an interface in https://github.com/gimli-rs/object/blob/master/src/read/traits.rs, which is specific for XCOFF platform.

compiler/rustc_codegen_ssa/src/back/metadata.rs

EsmeYi · 2023-04-18T05:51:35Z

The PR is expected to pass CI checks after the release of the new version of Object crate https://github.com/gimli-rs/object which may include features of writing a XCOFF object file.
Hi @philipc, what is your plan to release a new version of Object?

philipc · 2023-04-18T07:42:05Z

Sorry I forgot to tell you. A new version was released a few days ago.

EsmeYi · 2023-04-20T03:11:30Z

hi @bjorn3, could you help to updates object to 0.31.0 for ar_archive_writer and thorin-dwp? Thanks!

ecnelises · 2023-04-20T03:16:38Z

I posted PR to bump object in rust-lang/ar_archive_writer#6. And I see @philipc has one for thorin: rust-lang/thorin#24.

bjorn3 · 2023-04-20T11:59:58Z

compiler/rustc_codegen_ssa/src/back/metadata.rs

+///   From XCOFF's view, (2) creates a csect entry in the symbol table, the
+///   symbol created by (3) is a info symbol for the preceding csect. Thus
+///   two symbols are preserved during linking and we can use the second symbol
+///   to reference the metadata.


There are two kinds of object files created. One (create_wrapper_file) is for use in rlibs and we do not want to preserve metadata in there. It is only wrapped in an rlib to prevent linkers from complaining about files that are not valid object files in archives. The other kind (create_compressed_metadata_file) contains compressed metadata and is used for rust dylibs. This one should be preserved.

AIX linker will abort if it finds lib.rmeta is a valid XCOFF file but only has .info section (no .bss, .data or .text). Fortunately, we can just add a stub .text section to make linker happy:

file.add_section(Vec::new(), b".text".to_vec(), SectionKind::Text);

ecnelises · 2023-04-27T09:50:10Z

backtrace uses object 0.30, maybe due to some inconsistency between 0.30 and 0.31? If so, we have to also bump object to 0.31.0 in backtrace (rust-lang/backtrace-rs#522 )

philipc · 2023-04-27T10:12:42Z

library/std/Cargo.toml

@@ -27,10 +27,10 @@ addr2line = { version = "0.19.0", optional = true, default-features = false }
 rustc-demangle = { version = "0.1.21", features = ['rustc-dep-of-std'] }
 miniz_oxide = { version = "0.6.0", optional = true, default-features = false }
 [dependencies.object]
-version = "0.30.0"
+version = "0.31.0"


Does it build without this change? backtrace-rs doesn't have xcoff support yet anyway.

bors · 2023-04-30T04:19:34Z

☔ The latest upstream changes (presumably #111001) made this pull request unmergeable. Please resolve the merge conflicts.

philipc · 2023-05-11T08:28:25Z

object is being bumped in #111413

bzEq

LGTM.

EsmeYi · 2023-05-31T09:54:37Z

@rustbot ready

Noratrieb · 2023-05-31T20:16:36Z

r? @bjorn3
Sorry for not swapping earlier

compiler/rustc_codegen_ssa/src/back/metadata.rs

bjorn3 · 2023-06-05T07:23:33Z

@bors r+

bors · 2023-06-05T07:23:35Z

📌 Commit e31661c has been approved by bjorn3

It is now in the queue for this repository.

bors · 2023-06-05T10:42:09Z

⌛ Testing commit e31661c with merge 7452822...

bors · 2023-06-05T13:30:59Z

☀️ Test successful - checks-actions
Approved by: bjorn3
Pushing 7452822 to master...

rust-timer · 2023-06-05T14:46:18Z

Finished benchmarking commit (7452822): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.9%	[-1.9%, -1.9%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.9%	[-5.3%, -4.5%]	4
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 646.615s -> 646.59s (-0.00%)

rustbot assigned Noratrieb Feb 2, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 2, 2023

EsmeYi mentioned this pull request Feb 2, 2023

write/xcoff: add support. gimli-rs/object#482

Merged

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 2, 2023

philipc reviewed Feb 3, 2023

View reviewed changes

workingjubilee added the O-aix OS: Big Blue's Advanced Interactive eXecutive.. label Feb 5, 2023

This comment has been minimized.

Sign in to view

bzEq reviewed Mar 20, 2023

View reviewed changes

compiler/rustc_codegen_ssa/src/back/metadata.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

EsmeYi marked this pull request as draft April 12, 2023 02:43

EsmeYi force-pushed the aix_xcoff_metadata branch from 9840671 to dfcd17d Compare April 13, 2023 10:00

This comment has been minimized.

Sign in to view

bjorn3 reviewed Apr 20, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

philipc reviewed Apr 27, 2023

View reviewed changes

EsmeYi force-pushed the aix_xcoff_metadata branch from 8c80cd8 to 48fc761 Compare May 4, 2023 05:24

This comment has been minimized.

Sign in to view

EsmeYi force-pushed the aix_xcoff_metadata branch from a8dbb96 to d7c51b6 Compare May 23, 2023 08:17

This comment has been minimized.

Sign in to view

EsmeYi force-pushed the aix_xcoff_metadata branch from d7c51b6 to ab983c8 Compare May 23, 2023 08:20

This comment has been minimized.

Sign in to view

Support rust metadata for AIX.

18fdca3

EsmeYi force-pushed the aix_xcoff_metadata branch from ab983c8 to 18fdca3 Compare May 23, 2023 08:24

EsmeYi marked this pull request as ready for review May 23, 2023 09:07

EsmeYi requested review from philipc, bzEq, ecnelises and bjorn3 May 30, 2023 03:14

bzEq approved these changes May 30, 2023

View reviewed changes

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 31, 2023

rustbot assigned bjorn3 and unassigned Noratrieb May 31, 2023

bjorn3 reviewed Jun 2, 2023

View reviewed changes

compiler/rustc_codegen_ssa/src/back/metadata.rs Show resolved Hide resolved

compiler/rustc_codegen_ssa/src/back/metadata.rs Outdated Show resolved Hide resolved

Address @bjorn3's comments.

e31661c

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 5, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 5, 2023

bors merged commit 7452822 into rust-lang:master Jun 5, 2023

rustbot added this to the 1.72.0 milestone Jun 5, 2023

Support the rustc metadata for AIX #107583

Support the rustc metadata for AIX #107583

Conversation

EsmeYi commented Feb 2, 2023

rustbot commented Feb 2, 2023

Noratrieb commented Feb 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

EsmeYi commented Apr 18, 2023

philipc commented Apr 18, 2023

EsmeYi commented Apr 20, 2023

ecnelises commented Apr 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

ecnelises commented Apr 27, 2023

Choose a reason for hiding this comment

bors commented Apr 30, 2023

This comment has been minimized.

philipc commented May 11, 2023

This comment has been minimized.

This comment has been minimized.

bzEq left a comment

Choose a reason for hiding this comment

EsmeYi commented May 31, 2023

Noratrieb commented May 31, 2023

bjorn3 commented Jun 5, 2023

bors commented Jun 5, 2023

bors commented Jun 5, 2023

bors commented Jun 5, 2023

rust-timer commented Jun 5, 2023

Overall result: no relevant changes - no action needed