Complete undefined mode wiring
- SHA
cab975558b8b2e000f027bc663f71623dc6ff949- Parents
-
28ec20f - Tree
9a4e950
cab9755
cab975558b8b2e000f027bc663f71623dc6ff94928ec20f
9a4e950| Status | File | + | - |
|---|---|---|---|
| M |
src/args.rs
|
16 | 13 |
| M |
src/diag.rs
|
6 | 0 |
| M |
src/lib.rs
|
11 | 2 |
| M |
src/main.rs
|
1 | 1 |
| M |
src/resolve.rs
|
45 | 16 |
| M |
tests/cli_diagnostics.rs
|
124 | 0 |
src/args.rsmodified@@ -239,21 +239,15 @@ pub fn parse(argv: &[String]) -> Result<LinkOptions, ArgsError> { | |||
| 239 | .ok_or_else(|| ArgsError::MissingValue("-undefined".into()))?; | 239 | .ok_or_else(|| ArgsError::MissingValue("-undefined".into()))?; |
| 240 | opts.undefined_treatment = match value.as_str() { | 240 | opts.undefined_treatment = match value.as_str() { |
| 241 | "error" => UndefinedTreatment::Error, | 241 | "error" => UndefinedTreatment::Error, |
| 242 | + "warning" => UndefinedTreatment::Warning, | ||
| 243 | + "suppress" => UndefinedTreatment::Suppress, | ||
| 242 | "dynamic_lookup" => UndefinedTreatment::DynamicLookup, | 244 | "dynamic_lookup" => UndefinedTreatment::DynamicLookup, |
| 243 | - "warning" | "suppress" => { | ||
| 244 | - return Err(ArgsError::InvalidValue { | ||
| 245 | - flag: "-undefined".into(), | ||
| 246 | - value: value.clone(), | ||
| 247 | - expected: | ||
| 248 | - "`error` or `dynamic_lookup` (warning/suppress not yet supported)" | ||
| 249 | - .into(), | ||
| 250 | - }); | ||
| 251 | - } | ||
| 252 | _ => { | 245 | _ => { |
| 253 | return Err(ArgsError::InvalidValue { | 246 | return Err(ArgsError::InvalidValue { |
| 254 | flag: "-undefined".into(), | 247 | flag: "-undefined".into(), |
| 255 | value: value.clone(), | 248 | value: value.clone(), |
| 256 | - expected: "`error` or `dynamic_lookup`".into(), | 249 | + expected: "`error`, `warning`, `suppress`, or `dynamic_lookup`" |
| 250 | + .into(), | ||
| 257 | }); | 251 | }); |
| 258 | } | 252 | } |
| 259 | }; | 253 | }; |
@@ -601,15 +595,24 @@ mod tests { | |||
| 601 | } | 595 | } |
| 602 | 596 | ||
| 603 | #[test] | 597 | #[test] |
| 604 | - fn undefined_flag_rejects_unsupported_modes() { | 598 | + fn undefined_flag_records_warning_and_suppress() { |
| 605 | - let err = parse(&argv(&["-undefined", "warning", "main.o"])).unwrap_err(); | 599 | + let warning = parse(&argv(&["-undefined", "warning", "main.o"])).unwrap(); |
| 600 | + assert_eq!(warning.undefined_treatment, UndefinedTreatment::Warning); | ||
| 601 | + | ||
| 602 | + let suppress = parse(&argv(&["-undefined", "suppress", "main.o"])).unwrap(); | ||
| 603 | + assert_eq!(suppress.undefined_treatment, UndefinedTreatment::Suppress); | ||
| 604 | + } | ||
| 605 | + | ||
| 606 | + #[test] | ||
| 607 | + fn undefined_flag_rejects_unknown_modes() { | ||
| 608 | + let err = parse(&argv(&["-undefined", "bogus", "main.o"])).unwrap_err(); | ||
| 606 | assert!(matches!( | 609 | assert!(matches!( |
| 607 | err, | 610 | err, |
| 608 | ArgsError::InvalidValue { | 611 | ArgsError::InvalidValue { |
| 609 | ref flag, | 612 | ref flag, |
| 610 | ref value, | 613 | ref value, |
| 611 | .. | 614 | .. |
| 612 | - } if flag == "-undefined" && value == "warning" | 615 | + } if flag == "-undefined" && value == "bogus" |
| 613 | )); | 616 | )); |
| 614 | } | 617 | } |
| 615 | 618 | ||
src/diag.rsmodified@@ -22,3 +22,9 @@ pub fn warning(msg: &str) { | |||
| 22 | let mut h = stderr.lock(); | 22 | let mut h = stderr.lock(); |
| 23 | let _ = writeln!(h, "afs-ld: warning: {msg}"); | 23 | let _ = writeln!(h, "afs-ld: warning: {msg}"); |
| 24 | } | 24 | } |
| 25 | + | ||
| 26 | +pub fn warning_verbatim(msg: &str) { | ||
| 27 | + let stderr = std::io::stderr(); | ||
| 28 | + let mut h = stderr.lock(); | ||
| 29 | + let _ = writeln!(h, "{msg}"); | ||
| 30 | +} | ||
src/lib.rsmodified@@ -34,8 +34,9 @@ use macho::tbd::{parse_tbd, parse_version, Arch, Platform, Target}; | |||
| 34 | use reloc::arm64::RelocError; | 34 | use reloc::arm64::RelocError; |
| 35 | use resolve::{ | 35 | use resolve::{ |
| 36 | classify_unresolved, drain_fetches, find_archive_by_path, force_load_all, force_load_archive, | 36 | classify_unresolved, drain_fetches, find_archive_by_path, force_load_all, force_load_archive, |
| 37 | - format_duplicate_diagnostic, format_undefined_diagnostic, seed_all, DrainReport, DylibLoadMeta, | 37 | + format_duplicate_diagnostic, format_undefined_diagnostic, |
| 38 | - InputAddError, Inputs, Symbol, SymbolTable, UndefinedTreatment, | 38 | + format_undefined_warning_diagnostic, seed_all, DrainReport, DylibLoadMeta, InputAddError, |
| 39 | + Inputs, Symbol, SymbolTable, UndefinedTreatment, | ||
| 39 | }; | 40 | }; |
| 40 | 41 | ||
| 41 | const DEFAULT_TBD_VERSION: u32 = 1 << 16; | 42 | const DEFAULT_TBD_VERSION: u32 = 1 << 16; |
@@ -424,6 +425,14 @@ impl Linker { | |||
| 424 | &unresolved.errors, | 425 | &unresolved.errors, |
| 425 | ))); | 426 | ))); |
| 426 | } | 427 | } |
| 428 | + if !unresolved.warnings.is_empty() { | ||
| 429 | + crate::diag::warning_verbatim(&format_undefined_warning_diagnostic( | ||
| 430 | + &sym_table, | ||
| 431 | + &inputs, | ||
| 432 | + &referrers, | ||
| 433 | + &unresolved.warnings, | ||
| 434 | + )); | ||
| 435 | + } | ||
| 427 | 436 | ||
| 428 | let mut atom_table = AtomTable::new(); | 437 | let mut atom_table = AtomTable::new(); |
| 429 | let mut objects = Vec::new(); | 438 | let mut objects = Vec::new(); |
src/main.rsmodified@@ -19,7 +19,7 @@ Options: | |||
| 19 | Set LC_BUILD_VERSION payload | 19 | Set LC_BUILD_VERSION payload |
| 20 | -r Relocatable output (deferred; errors) | 20 | -r Relocatable output (deferred; errors) |
| 21 | -bundle Bundle output (deferred; errors) | 21 | -bundle Bundle output (deferred; errors) |
| 22 | - -undefined <error|dynamic_lookup> | 22 | + -undefined <error|warning|suppress|dynamic_lookup> |
| 23 | Control unresolved-symbol treatment | 23 | Control unresolved-symbol treatment |
| 24 | -rpath <path> Add LC_RPATH | 24 | -rpath <path> Add LC_RPATH |
| 25 | -install_name <path> Override dylib install name | 25 | -install_name <path> Override dylib install name |
src/resolve.rsmodified@@ -1296,9 +1296,11 @@ impl DylibId { | |||
| 1296 | pub struct ClassificationReport { | 1296 | pub struct ClassificationReport { |
| 1297 | /// Strong undefineds that triggered errors under `Error` treatment. | 1297 | /// Strong undefineds that triggered errors under `Error` treatment. |
| 1298 | pub errors: Vec<Unresolved>, | 1298 | pub errors: Vec<Unresolved>, |
| 1299 | - /// Strong undefineds that produced warnings under `Warning` treatment. | 1299 | + /// Strong undefineds that produced warnings under `Warning` treatment and |
| 1300 | + /// were promoted to flat-lookup imports for final emission. | ||
| 1300 | pub warnings: Vec<Unresolved>, | 1301 | pub warnings: Vec<Unresolved>, |
| 1301 | - /// Strong undefineds that were silently accepted under `Suppress`. | 1302 | + /// Strong undefineds that were silently accepted under `Suppress` and |
| 1303 | + /// were promoted to flat-lookup imports for final emission. | ||
| 1302 | pub suppressed: Vec<Unresolved>, | 1304 | pub suppressed: Vec<Unresolved>, |
| 1303 | /// Undefineds promoted to flat-lookup DylibImport entries. | 1305 | /// Undefineds promoted to flat-lookup DylibImport entries. |
| 1304 | pub promoted_to_dynamic: Vec<SymbolId>, | 1306 | pub promoted_to_dynamic: Vec<SymbolId>, |
@@ -1372,16 +1374,17 @@ pub fn did_you_mean(table: &SymbolTable, query: &str, budget: usize, max: usize) | |||
| 1372 | /// Format the full undefined-symbol diagnostic block, one entry per | 1374 | /// Format the full undefined-symbol diagnostic block, one entry per |
| 1373 | /// unresolved name: the error line, every referrer, and an optional | 1375 | /// unresolved name: the error line, every referrer, and an optional |
| 1374 | /// did-you-mean hint. | 1376 | /// did-you-mean hint. |
| 1375 | -pub fn format_undefined_diagnostic( | 1377 | +fn format_undefined_diagnostic_with_level( |
| 1376 | table: &SymbolTable, | 1378 | table: &SymbolTable, |
| 1377 | inputs: &Inputs, | 1379 | inputs: &Inputs, |
| 1378 | referrers: &ReferrerLog, | 1380 | referrers: &ReferrerLog, |
| 1379 | unresolved: &[Unresolved], | 1381 | unresolved: &[Unresolved], |
| 1382 | + level: &str, | ||
| 1380 | ) -> String { | 1383 | ) -> String { |
| 1381 | let mut out = String::new(); | 1384 | let mut out = String::new(); |
| 1382 | for u in unresolved { | 1385 | for u in unresolved { |
| 1383 | let name = table.interner.resolve(u.name); | 1386 | let name = table.interner.resolve(u.name); |
| 1384 | - out.push_str(&format!("afs-ld: error: undefined symbol: {name}\n")); | 1387 | + out.push_str(&format!("afs-ld: {level}: undefined symbol: {name}\n")); |
| 1385 | for origin in referrers.get(u.name) { | 1388 | for origin in referrers.get(u.name) { |
| 1386 | if let Some(oi) = inputs.objects.get(origin.0 as usize) { | 1389 | if let Some(oi) = inputs.objects.get(origin.0 as usize) { |
| 1387 | out.push_str(&format!(" referenced by {}\n", oi.path.display())); | 1390 | out.push_str(&format!(" referenced by {}\n", oi.path.display())); |
@@ -1402,6 +1405,24 @@ pub fn format_undefined_diagnostic( | |||
| 1402 | out | 1405 | out |
| 1403 | } | 1406 | } |
| 1404 | 1407 | ||
| 1408 | +pub fn format_undefined_diagnostic( | ||
| 1409 | + table: &SymbolTable, | ||
| 1410 | + inputs: &Inputs, | ||
| 1411 | + referrers: &ReferrerLog, | ||
| 1412 | + unresolved: &[Unresolved], | ||
| 1413 | +) -> String { | ||
| 1414 | + format_undefined_diagnostic_with_level(table, inputs, referrers, unresolved, "error") | ||
| 1415 | +} | ||
| 1416 | + | ||
| 1417 | +pub fn format_undefined_warning_diagnostic( | ||
| 1418 | + table: &SymbolTable, | ||
| 1419 | + inputs: &Inputs, | ||
| 1420 | + referrers: &ReferrerLog, | ||
| 1421 | + unresolved: &[Unresolved], | ||
| 1422 | +) -> String { | ||
| 1423 | + format_undefined_diagnostic_with_level(table, inputs, referrers, unresolved, "warning") | ||
| 1424 | +} | ||
| 1425 | + | ||
| 1405 | /// Format a `DuplicateStrong` insertion error for user consumption. Needs | 1426 | /// Format a `DuplicateStrong` insertion error for user consumption. Needs |
| 1406 | /// the incumbent symbol (from the table) plus the losing second symbol | 1427 | /// the incumbent symbol (from the table) plus the losing second symbol |
| 1407 | /// carried in the error itself. | 1428 | /// carried in the error itself. |
@@ -1442,6 +1463,21 @@ pub fn classify_unresolved( | |||
| 1442 | ) -> ClassificationReport { | 1463 | ) -> ClassificationReport { |
| 1443 | let mut report = ClassificationReport::default(); | 1464 | let mut report = ClassificationReport::default(); |
| 1444 | 1465 | ||
| 1466 | + fn promote_to_flat_lookup(table: &mut SymbolTable, id: SymbolId, name: Istr) { | ||
| 1467 | + table.symbols[id.0 as usize] = Symbol::DylibImport { | ||
| 1468 | + name, | ||
| 1469 | + dylib: DylibId::INVALID, | ||
| 1470 | + ordinal: FLAT_LOOKUP_ORDINAL, | ||
| 1471 | + weak_import: true, | ||
| 1472 | + }; | ||
| 1473 | + table.transitions.push(Transition { | ||
| 1474 | + id, | ||
| 1475 | + from: SymbolKindTag::Undefined, | ||
| 1476 | + to: SymbolKindTag::DylibImport, | ||
| 1477 | + cause: TransitionCause::Replaced, | ||
| 1478 | + }); | ||
| 1479 | + } | ||
| 1480 | + | ||
| 1445 | // Collect undefineds before mutating — avoids double-borrow grief. | 1481 | // Collect undefineds before mutating — avoids double-borrow grief. |
| 1446 | let undefs: Vec<(SymbolId, Istr, bool)> = table | 1482 | let undefs: Vec<(SymbolId, Istr, bool)> = table |
| 1447 | .iter() | 1483 | .iter() |
@@ -1462,23 +1498,16 @@ pub fn classify_unresolved( | |||
| 1462 | } | 1498 | } |
| 1463 | UndefinedTreatment::Warning => { | 1499 | UndefinedTreatment::Warning => { |
| 1464 | report.warnings.push(Unresolved { name, id }); | 1500 | report.warnings.push(Unresolved { name, id }); |
| 1501 | + promote_to_flat_lookup(table, id, name); | ||
| 1502 | + report.promoted_to_dynamic.push(id); | ||
| 1465 | } | 1503 | } |
| 1466 | UndefinedTreatment::Suppress => { | 1504 | UndefinedTreatment::Suppress => { |
| 1467 | report.suppressed.push(Unresolved { name, id }); | 1505 | report.suppressed.push(Unresolved { name, id }); |
| 1506 | + promote_to_flat_lookup(table, id, name); | ||
| 1507 | + report.promoted_to_dynamic.push(id); | ||
| 1468 | } | 1508 | } |
| 1469 | UndefinedTreatment::DynamicLookup => { | 1509 | UndefinedTreatment::DynamicLookup => { |
| 1470 | - table.symbols[id.0 as usize] = Symbol::DylibImport { | 1510 | + promote_to_flat_lookup(table, id, name); |
| 1471 | - name, | ||
| 1472 | - dylib: DylibId::INVALID, | ||
| 1473 | - ordinal: FLAT_LOOKUP_ORDINAL, | ||
| 1474 | - weak_import: true, | ||
| 1475 | - }; | ||
| 1476 | - table.transitions.push(Transition { | ||
| 1477 | - id, | ||
| 1478 | - from: SymbolKindTag::Undefined, | ||
| 1479 | - to: SymbolKindTag::DylibImport, | ||
| 1480 | - cause: TransitionCause::Replaced, | ||
| 1481 | - }); | ||
| 1482 | report.promoted_to_dynamic.push(id); | 1511 | report.promoted_to_dynamic.push(id); |
| 1483 | } | 1512 | } |
| 1484 | } | 1513 | } |
tests/cli_diagnostics.rsmodified@@ -14,6 +14,17 @@ fn have_xcrun() -> bool { | |||
| 14 | .unwrap_or(false) | 14 | .unwrap_or(false) |
| 15 | } | 15 | } |
| 16 | 16 | ||
| 17 | +fn sdk_path() -> Option<String> { | ||
| 18 | + let out = Command::new("xcrun") | ||
| 19 | + .args(["--sdk", "macosx", "--show-sdk-path"]) | ||
| 20 | + .output() | ||
| 21 | + .ok()?; | ||
| 22 | + if !out.status.success() { | ||
| 23 | + return None; | ||
| 24 | + } | ||
| 25 | + Some(String::from_utf8_lossy(&out.stdout).trim().to_string()) | ||
| 26 | +} | ||
| 27 | + | ||
| 17 | fn minimal_main_src() -> &'static str { | 28 | fn minimal_main_src() -> &'static str { |
| 18 | r#" | 29 | r#" |
| 19 | .section __TEXT,__text,regular,pure_instructions | 30 | .section __TEXT,__text,regular,pure_instructions |
@@ -120,6 +131,7 @@ fn help_flag_prints_usage_and_exits_successfully() { | |||
| 120 | assert!(stdout.contains("-map <path>")); | 131 | assert!(stdout.contains("-map <path>")); |
| 121 | assert!(stdout.contains("-no_uuid")); | 132 | assert!(stdout.contains("-no_uuid")); |
| 122 | assert!(stdout.contains("-dead_strip")); | 133 | assert!(stdout.contains("-dead_strip")); |
| 134 | + assert!(stdout.contains("-undefined <error|warning|suppress|dynamic_lookup>")); | ||
| 123 | assert!(stdout.contains("-t, -trace")); | 135 | assert!(stdout.contains("-t, -trace")); |
| 124 | assert!(stdout.contains("-v, --version")); | 136 | assert!(stdout.contains("-v, --version")); |
| 125 | } | 137 | } |
@@ -299,6 +311,118 @@ fn undefined_symbol_diagnostic_is_not_double_prefixed() { | |||
| 299 | let _ = fs::remove_file(obj); | 311 | let _ = fs::remove_file(obj); |
| 300 | } | 312 | } |
| 301 | 313 | ||
| 314 | +#[test] | ||
| 315 | +fn undefined_warning_mode_links_and_warns_once() { | ||
| 316 | + if !have_xcrun() { | ||
| 317 | + eprintln!("skipping: xcrun as unavailable"); | ||
| 318 | + return; | ||
| 319 | + } | ||
| 320 | + let Some(sdk) = sdk_path() else { | ||
| 321 | + eprintln!("skipping: xcrun --show-sdk-path unavailable"); | ||
| 322 | + return; | ||
| 323 | + }; | ||
| 324 | + | ||
| 325 | + let exe = env!("CARGO_BIN_EXE_afs-ld"); | ||
| 326 | + let obj = scratch("missing-warning.o"); | ||
| 327 | + let src = r#" | ||
| 328 | + .section __TEXT,__text,regular,pure_instructions | ||
| 329 | + .globl _main | ||
| 330 | + _main: | ||
| 331 | + bl _missing | ||
| 332 | + ret | ||
| 333 | + .subsections_via_symbols | ||
| 334 | + "#; | ||
| 335 | + if let Err(e) = assemble(src, &obj) { | ||
| 336 | + eprintln!("skipping: assemble failed: {e}"); | ||
| 337 | + return; | ||
| 338 | + } | ||
| 339 | + | ||
| 340 | + let out_path = scratch("missing-warning.out"); | ||
| 341 | + let out = Command::new(exe) | ||
| 342 | + .arg("-undefined") | ||
| 343 | + .arg("warning") | ||
| 344 | + .arg("-syslibroot") | ||
| 345 | + .arg(&sdk) | ||
| 346 | + .arg("-lSystem") | ||
| 347 | + .arg("-o") | ||
| 348 | + .arg(&out_path) | ||
| 349 | + .arg(&obj) | ||
| 350 | + .output() | ||
| 351 | + .expect("afs-ld should run"); | ||
| 352 | + assert!( | ||
| 353 | + out.status.success(), | ||
| 354 | + "-undefined warning should link successfully:\nstderr:\n{}", | ||
| 355 | + String::from_utf8_lossy(&out.stderr) | ||
| 356 | + ); | ||
| 357 | + let stderr = String::from_utf8_lossy(&out.stderr); | ||
| 358 | + assert!( | ||
| 359 | + stderr.contains("afs-ld: warning: undefined symbol: _missing"), | ||
| 360 | + "missing expected undefined-symbol warning:\n{stderr}" | ||
| 361 | + ); | ||
| 362 | + assert!( | ||
| 363 | + !stderr.contains("afs-ld: warning: afs-ld: warning:"), | ||
| 364 | + "warning diagnostic was double-prefixed:\n{stderr}" | ||
| 365 | + ); | ||
| 366 | + assert!(out_path.exists(), "expected linked output to be written"); | ||
| 367 | + | ||
| 368 | + let _ = fs::remove_file(obj); | ||
| 369 | + let _ = fs::remove_file(out_path); | ||
| 370 | +} | ||
| 371 | + | ||
| 372 | +#[test] | ||
| 373 | +fn undefined_suppress_mode_links_silently() { | ||
| 374 | + if !have_xcrun() { | ||
| 375 | + eprintln!("skipping: xcrun as unavailable"); | ||
| 376 | + return; | ||
| 377 | + } | ||
| 378 | + let Some(sdk) = sdk_path() else { | ||
| 379 | + eprintln!("skipping: xcrun --show-sdk-path unavailable"); | ||
| 380 | + return; | ||
| 381 | + }; | ||
| 382 | + | ||
| 383 | + let exe = env!("CARGO_BIN_EXE_afs-ld"); | ||
| 384 | + let obj = scratch("missing-suppress.o"); | ||
| 385 | + let src = r#" | ||
| 386 | + .section __TEXT,__text,regular,pure_instructions | ||
| 387 | + .globl _main | ||
| 388 | + _main: | ||
| 389 | + bl _missing | ||
| 390 | + ret | ||
| 391 | + .subsections_via_symbols | ||
| 392 | + "#; | ||
| 393 | + if let Err(e) = assemble(src, &obj) { | ||
| 394 | + eprintln!("skipping: assemble failed: {e}"); | ||
| 395 | + return; | ||
| 396 | + } | ||
| 397 | + | ||
| 398 | + let out_path = scratch("missing-suppress.out"); | ||
| 399 | + let out = Command::new(exe) | ||
| 400 | + .arg("-undefined") | ||
| 401 | + .arg("suppress") | ||
| 402 | + .arg("-syslibroot") | ||
| 403 | + .arg(&sdk) | ||
| 404 | + .arg("-lSystem") | ||
| 405 | + .arg("-o") | ||
| 406 | + .arg(&out_path) | ||
| 407 | + .arg(&obj) | ||
| 408 | + .output() | ||
| 409 | + .expect("afs-ld should run"); | ||
| 410 | + assert!( | ||
| 411 | + out.status.success(), | ||
| 412 | + "-undefined suppress should link successfully:\nstderr:\n{}", | ||
| 413 | + String::from_utf8_lossy(&out.stderr) | ||
| 414 | + ); | ||
| 415 | + let stderr = String::from_utf8_lossy(&out.stderr); | ||
| 416 | + assert!( | ||
| 417 | + !stderr.contains("undefined symbol: _missing"), | ||
| 418 | + "expected -undefined suppress to omit undefined diagnostic:\n{stderr}" | ||
| 419 | + ); | ||
| 420 | + assert!(out_path.exists(), "expected linked output to be written"); | ||
| 421 | + | ||
| 422 | + let _ = fs::remove_file(obj); | ||
| 423 | + let _ = fs::remove_file(out_path); | ||
| 424 | +} | ||
| 425 | + | ||
| 302 | #[test] | 426 | #[test] |
| 303 | fn trace_flag_prints_loaded_inputs_and_archive_members() { | 427 | fn trace_flag_prints_loaded_inputs_and_archive_members() { |
| 304 | if !have_xcrun() { | 428 | if !have_xcrun() { |