Skip to content

Conversation

@ZequanWu
Copy link
Contributor

This change allows us to pass creduce options to creduce-clang-crash.py script. With this, --n is no longer needed to specify the number of cores, so removed the flag.

The motivation is #87933 (comment) suggests that disabling creduce renaming passes helps people to further reduce crash manually.

@ZequanWu ZequanWu requested review from amykhuang and tru May 14, 2024 16:17
@llvmbot llvmbot added the clang Clang issues not falling into any other category label May 14, 2024
@llvmbot
Copy link
Member

llvmbot commented May 14, 2024

@llvm/pr-subscribers-clang

Author: Zequan Wu (ZequanWu)

Changes

This change allows us to pass creduce options to creduce-clang-crash.py script. With this, --n is no longer needed to specify the number of cores, so removed the flag.

The motivation is #87933 (comment) suggests that disabling creduce renaming passes helps people to further reduce crash manually.


Full diff: https://github.com/llvm/llvm-project/pull/92141.diff

1 Files Affected:

  • (modified) clang/utils/creduce-clang-crash.py (+10-19)
diff --git a/clang/utils/creduce-clang-crash.py b/clang/utils/creduce-clang-crash.py
index 4d0c8224d8b49..e2c999196f36c 100755
--- a/clang/utils/creduce-clang-crash.py
+++ b/clang/utils/creduce-clang-crash.py
@@ -1,5 +1,6 @@
 #!/usr/bin/env python3
 """Calls C-Reduce to create a minimal reproducer for clang crashes.
+Unknown arguments are treated at creduce options.
 
 Output files:
   *.reduced.sh -- crash reproducer with minimal arguments
@@ -70,7 +71,7 @@ def write_to_script(text, filename):
 
 
 class Reduce(object):
-    def __init__(self, crash_script, file_to_reduce, core_number):
+    def __init__(self, crash_script, file_to_reduce, creduce_flags):
         crash_script_name, crash_script_ext = os.path.splitext(crash_script)
         file_reduce_name, file_reduce_ext = os.path.splitext(file_to_reduce)
 
@@ -83,8 +84,7 @@ def __init__(self, crash_script, file_to_reduce, core_number):
         self.clang_args = []
         self.expected_output = []
         self.needs_stack_trace = False
-        self.creduce_flags = ["--tidy"]
-        self.creduce_flags = ["--n", str(core_number)]
+        self.creduce_flags = ["--tidy"] + creduce_flags
 
         self.read_clang_args(crash_script, file_to_reduce)
         self.read_expected_output()
@@ -412,13 +412,13 @@ def reduce_clang_args(self):
         print("Reduced command:", reduced_cmd)
 
     def run_creduce(self):
+        full_creduce_cmd = (
+            [creduce_cmd] + self.creduce_flags + [self.testfile, self.file_to_reduce]
+        )
         print("\nRunning C-Reduce...")
+        verbose_print(quote_cmd(full_creduce_cmd))
         try:
-            p = subprocess.Popen(
-                [creduce_cmd]
-                + self.creduce_flags
-                + [self.testfile, self.file_to_reduce]
-            )
+            p = subprocess.Popen(full_creduce_cmd)
             p.communicate()
         except KeyboardInterrupt:
             # Hack to kill C-Reduce because it jumps into its own pgid
@@ -458,26 +458,17 @@ def main():
         help="The path to the `creduce` executable. "
         "Required if `creduce` is not in PATH environment.",
     )
-    parser.add_argument(
-        "--n",
-        dest="core_number",
-        type=int,
-        default=max(4, multiprocessing.cpu_count() // 2),
-        help="Number of cores to use.",
-    )
     parser.add_argument("-v", "--verbose", action="store_true")
-    args = parser.parse_args()
-
+    args, creduce_flags = parser.parse_known_args()
     verbose = args.verbose
     llvm_bin = os.path.abspath(args.llvm_bin) if args.llvm_bin else None
     creduce_cmd = check_cmd("creduce", None, args.creduce)
     clang_cmd = check_cmd("clang", llvm_bin, args.clang)
-    core_number = args.core_number
 
     crash_script = check_file(args.crash_script[0])
     file_to_reduce = check_file(args.file_to_reduce[0])
 
-    r = Reduce(crash_script, file_to_reduce, core_number)
+    r = Reduce(crash_script, file_to_reduce, creduce_flags)
 
     r.simplify_clang_args()
     r.write_interestingness_test()

Copy link
Collaborator

@amykhuang amykhuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@ZequanWu ZequanWu merged commit 5adfcb0 into llvm:main May 14, 2024
@ZequanWu ZequanWu deleted the creduce-script branch May 14, 2024 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants