elf.parse: resolve overflow issue in hash sections

[quake]

I found an overflow error when running the fuzz test (test case), this PR try to fix the issue by using wrapping operator.

[m4b]

Hmmm looks like macos CI is failing (for likely unrelated reasons ??)

[quake]

weird, trying revert commit and trigger the CI again

[quake]

Hmmm looks like macos CI is failing (for likely unrelated reasons ??)

I'm sure the CI error has nothing to do with this commit, I reverted the commit and got same error.

[m4b]

it looks like /Library/Developer/CommandLineTools/usr/bin/dyldinfo doesn't exist anymore, so that's the failure

[willglynn]

Ah, I believe that's moved from …/dyldinfo args to /usr/bin/xcrun dyldinfo args.

[m4b]

Oh you sweet, sweet cherub @willglynn , swooping in and saving the day; @quake this is annoying, but would you mind changing https://github.com/m4b/goblin/blob/e94ca6b81b3b3a87ed5efc94493b790fd1085355/tests/compare_dyldinfos.rs#L4 to xcrun dyldinfo ?

[m4b]

actually it may be more complicated, maybe a separate PR is better

[quake]

I will create a separate PR to try xcrun, but I suspect that's not the causing, since test output reports:

cargo dyldinfo ["-lazy_bind", "-bind", "/Library/Developer/CommandLineTools/usr/bin/dyldinfo"] output:
err: BadMagic(3405691582)

and /Library/Developer/CommandLineTools/usr/bin/dyldinfo runs well.

it looks like the magic header of dyldinfo is 0xcafebabe (3405691582), and it's ignored in parse_magic_and_ctx fn.

[m4b]

Oh wow you're right; fwiw, it should probably be updated to not hard code anyway, since it can fail on other machines, something perhaps like:

diff --git a/tests/compare_dyldinfos.rs b/tests/compare_dyldinfos.rs
index 0c9dcaf..806e004 100644
--- a/tests/compare_dyldinfos.rs
+++ b/tests/compare_dyldinfos.rs
@@ -1,7 +1,19 @@
 use std::process;
 
+fn get_realpath(cmd: &str) -> String {
+    let output = process::Command::new("/usr/bin/xcrun")
+        .arg("-f")
+        .arg(cmd)
+        .output()
+        .expect("can get realpath");
+    std::str::from_utf8(&output.stdout)
+        .expect("output is valid utf8")
+        .to_string()
+}
+
 pub fn compare(args: Vec<&str>) {
-    let apple = process::Command::new("/Library/Developer/CommandLineTools/usr/bin/dyldinfo")
+    let apple = process::Command::new("/usr/bin/xcrun")
+        .arg("dyldinfo")
         .args(&args)
         .output()
         .expect("run Apple dyldinfo");
@@ -39,39 +51,28 @@ pub fn compare(args: Vec<&str>) {
 #[cfg(target_os = "macos")]
 #[test]
 fn compare_binds() {
-    compare(vec![
-        "-bind",
-        "/Library/Developer/CommandLineTools/usr/bin/dyldinfo",
-    ]);
-    compare(vec![
-        "-bind",
-        "/Library/Developer/CommandLineTools/usr/bin/clang",
-    ]);
+    let dyldinfo = get_realpath("dyldinfo");
+    let clang = get_realpath("dyldinfo");
+    compare(vec!["-bind", &dyldinfo]);
+    compare(vec!["-bind", &clang]);
     compare(vec!["-bind", "/usr/bin/tmutil"]);
 }
 
 #[cfg(target_os = "macos")]
 #[test]
 fn compare_lazy_binds() {
-    compare(vec![
-        "-lazy_bind",
-        "/Library/Developer/CommandLineTools/usr/bin/dyldinfo",
-    ]);
-    compare(vec![
-        "-lazy_bind",
-        "/Library/Developer/CommandLineTools/usr/bin/clang",
-    ]);
+    let dyldinfo = get_realpath("dyldinfo");
+    let clang = get_realpath("dyldinfo");
+    compare(vec!["-lazy_bind", &dyldinfo]);
+    compare(vec!["-lazy_bind", &clang]);
     compare(vec!["-lazy_bind", "/usr/bin/tmutil"]);
 }
 
 #[cfg(target_os = "macos")]
 #[test]
 fn compare_combined_options() {
-    compare(vec![
-        "-lazy_bind",
-        "-bind",
-        "/Library/Developer/CommandLineTools/usr/bin/dyldinfo",
-    ]);
+    let dyldinfo = get_realpath("dyldinfo");
+    compare(vec!["-lazy_bind", "-bind", &dyldinfo]);
 }
 
 #[cfg(not(target_os = "macos"))]

[m4b]

@lzutao perhaps you have some thoughts on behavior of overflow wrapping, you did the work on the gnu hash rewrite

[m4b]

@quake are you still interested in getting this merged? let me know whenever you have a chance, just triaging old PRs here, thanks!