Code refactor and comment update

Author: harin-ramesh

build_system/src/test.rs

@@ -318,21 +318,20 @@ fn maybe_run_command_in_vm(
     Ok(())
 }
 
-fn no_builtins_tests(env: &Env, args: &TestArg) -> Result<(), String> {
-    // Test that the #![no_builtins] attribute prevents GCC from replacing
-    // code patterns (like loops) with calls to builtins (like memset).
-    // See https://github.com/rust-lang/rustc_codegen_gcc/issues/570
-
+/// Compile a source file to an object file and check if it contains a memset reference.
+fn object_has_memset(
+    env: &Env,
+    args: &TestArg,
+    src_file: &str,
+    obj_file_name: &str,
+) -> Result<bool, String> {
     let cargo_target_dir = Path::new(&args.config_info.cargo_target_dir);
-
-    // Test 1: WITH #![no_builtins] - memset should NOT be present
-    println!("[TEST] no_builtins attribute (with #![no_builtins])");
-    let obj_file = cargo_target_dir.join("no_builtins_test.o");
+    let obj_file = cargo_target_dir.join(obj_file_name);
     let obj_file_str = obj_file.to_str().expect("obj_file to_str");
 
     let mut command = args.config_info.rustc_command_vec();
     command.extend_from_slice(&[
-        &"tests/no_builtins/no_builtins.rs",
+        &src_file,
         &"--emit",
         &"obj",
         &"-O",
@@ -346,47 +345,34 @@ fn no_builtins_tests(env: &Env, args: &TestArg) -> Result<(), String> {
     let nm_output = run_command_with_env(&[&"nm", &obj_file_str], None, Some(env))?;
     let nm_stdout = String::from_utf8_lossy(&nm_output.stdout);
 
-    if nm_stdout.contains("memset") {
-        return Err(format!(
-            "no_builtins test FAILED: Found 'memset' in object file.\n\
+    Ok(nm_stdout.contains("memset"))
+}
+
+fn no_builtins_tests(env: &Env, args: &TestArg) -> Result<(), String> {
+    // Test that the #![no_builtins] attribute prevents GCC from replacing
+    // code patterns (like loops) with calls to builtins (like memset).
+    // See https://github.com/rust-lang/rustc_codegen_gcc/issues/570
+
+    // Test 1: WITH #![no_builtins] - memset should NOT be present
+    println!("[TEST] no_builtins attribute (with #![no_builtins])");
+    let has_memset =
+        object_has_memset(env, args, "tests/no_builtins/no_builtins.rs", "no_builtins_test.o")?;
+    if has_memset {
+        return Err("no_builtins test FAILED: Found 'memset' in object file.\n\
              The #![no_builtins] attribute should prevent GCC from replacing \n\
-             code patterns with builtin calls.\n\
-             nm output:\n{}",
-            nm_stdout
-        ));
+             code patterns with builtin calls."
+            .to_string());
     }
-    println!("[TEST] no_builtins attribute (with #![no_builtins]): PASSED");
 
     // Test 2: WITHOUT #![no_builtins] - memset SHOULD be present
     println!("[TEST] no_builtins attribute (without #![no_builtins])");
-    let obj_file = cargo_target_dir.join("with_builtins_test.o");
-    let obj_file_str = obj_file.to_str().expect("obj_file to_str");
-
-    let mut command = args.config_info.rustc_command_vec();
-    command.extend_from_slice(&[
-        &"tests/no_builtins/with_builtins.rs",
-        &"--emit",
-        &"obj",
-        &"-O",
-        &"--target",
-        &args.config_info.target_triple,
-        &"-o",
-    ]);
-    command.push(&obj_file_str);
-    run_command_with_env(&command, None, Some(env))?;
-
-    let nm_output = run_command_with_env(&[&"nm", &obj_file_str], None, Some(env))?;
-    let nm_stdout = String::from_utf8_lossy(&nm_output.stdout);
-
-    if !nm_stdout.contains("memset") {
-        return Err(format!(
-            "no_builtins test FAILED: 'memset' NOT found in object file.\n\
-             Without #![no_builtins], GCC should replace the loop with memset.\n\
-             nm output:\n{}",
-            nm_stdout
-        ));
+    let has_memset =
+        object_has_memset(env, args, "tests/no_builtins/with_builtins.rs", "with_builtins_test.o")?;
+    if !has_memset {
+        return Err("no_builtins test FAILED: 'memset' NOT found in object file.\n\
+             Without #![no_builtins], GCC should replace the loop with memset."
+            .to_string());
     }
-    println!("[TEST] no_builtins attribute (without #![no_builtins]): PASSED");
 
     Ok(())
 }

src/base.rs

@@ -139,8 +139,8 @@ pub fn compile_codegen_unit(
 
         // NOTE: We need to honor the `#![no_builtins]` attribute to prevent GCC from
         // replacing code patterns (like loops) with calls to builtins (like memset).
-        // This is important for crates like `compiler_builtins` that implement these functions.
-        // See https://github.com/rust-lang/rustc_codegen_gcc/issues/570
+        // The `-fno-tree-loop-distribute-patterns` flag disables the loop distribution pass
+        // that transforms loops into calls to library functions (memset, memcpy, etc.).
         let crate_attrs = tcx.hir_attrs(rustc_hir::CRATE_HIR_ID);
         if find_attr!(crate_attrs, AttributeKind::NoBuiltins) {
             context.add_command_line_option("-fno-tree-loop-distribute-patterns");

tests/no_builtins/no_builtins.rs

@@ -1,6 +1,6 @@
 // Test that the #![no_builtins] attribute is honored.
-// When this attribute is present, GCC should pass -fno-builtin to prevent
-// replacing code patterns (like loops) with calls to builtins (like memset).
+// When this attribute is present, GCC should not replace code patterns
+// (like loops) with calls to builtins (like memset).
 // See https://github.com/rust-lang/rustc_codegen_gcc/issues/570
 //
 // This test is verified by the build system test `--no-builtins-tests` which