public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Add back the command-line options -O[0123sg]
@ 2018-08-25  6:44 Yichun Zhang (agentzh)
  0 siblings, 0 replies; only message in thread
From: Yichun Zhang (agentzh) @ 2018-08-25  6:44 UTC (permalink / raw)
  To: systemtap; +Cc: Yichun Zhang (agentzh)

It is often desired to use a different optimization option or level to
compile the generated kernel module or dyinst DSO module. For example,
for simple .stp scripts, use of -O1 is more than 12% faster than the
default -O2 option while -Og is more than 17% faster than -O2, which is
beneficial for speeding up user stap tools' development, debugging,
and testing. Also, some optimization options lead to better debuginfo
which helps debugging those kernel modules and DSO modules themselves.
Furthermore, -Os can help generating smaller modules if the size is of
special interest, or -O3 for even faster generated code. Having choices
is almost always a good thing.

Stap does reject the -O0 option in the "kernel" runtime mode since the
generated kernel module source won't even compile with -O0. It also
rejects all the -OX options for the "bpf" runtime since it makes no
sense (yet) in that context.

Also added a "use at your own risk" note to the usage text for these -OX
options since these are considered as a "guru" or "unsafe" feature anyway.

Added some tests to cover the use of various optimization options for
both the kernel and the dyninst runtimes.
---
 buildrun.cxx                                 |  11 +-
 cmdline.h                                    |   2 +-
 session.cxx                                  |  35 +++
 session.h                                    |   3 +
 testsuite/systemtap.base/cmdline_opt_O.exp   | 387 +++++++++++++++++++++++++++
 testsuite/systemtap.base/cmdline_opt_O_1.stp |   1 +
 6 files changed, 432 insertions(+), 7 deletions(-)
 create mode 100644 testsuite/systemtap.base/cmdline_opt_O.exp
 create mode 100644 testsuite/systemtap.base/cmdline_opt_O_1.stp

diff --git a/buildrun.cxx b/buildrun.cxx
index 79b355123..d91d41f25 100644
--- a/buildrun.cxx
+++ b/buildrun.cxx
@@ -225,6 +225,9 @@ compile_dyninst (systemtap_session& s)
       "-Wno-tautological-compare", "-pthread", "-lrt", "-fPIC", "-shared",
     };
 
+  if (!s.cc_flags.empty())
+    cmd.insert(cmd.end(), s.cc_flags.begin(), s.cc_flags.end());
+
   // BZ855981/948279.  Since dyninst/runtime.h includes __sync_* calls,
   // the compiler may generate different code for it depending on -march.
   // For example, if the default is i386, we may get references to auxiliary
@@ -490,12 +493,8 @@ compile_pass (systemtap_session& s)
   // Generate eh_frame for self-backtracing
   o << "EXTRA_CFLAGS += -fasynchronous-unwind-tables" << endl;
 
-  // We used to allow the user to override default optimization when so
-  // requested by adding a -O[0123s] so they could determine the
-  // time/space/speed tradeoffs themselves, but we cannot guantantee that
-  // the (un)optimized code actually compiles and/or generates functional
-  // code, so we had to remove it.
-  // o << "EXTRA_CFLAGS += " << s.gcc_flags << endl; // Add -O[0123s]
+  if (!s.cc_flags.empty())
+    o << "EXTRA_CFLAGS += " << cmdstr_join(s.cc_flags) << endl; // Add -Ox
 
   // o << "CFLAGS += -fno-unit-at-a-time" << endl;
 
diff --git a/cmdline.h b/cmdline.h
index 9d3784aea..918e3fc5e 100644
--- a/cmdline.h
+++ b/cmdline.h
@@ -70,7 +70,7 @@ enum {
 
 // NB: when adding new options, consider very carefully whether they
 // should be restricted from stap clients (after --client-options)!
-#define STAP_SHORT_OPTIONS "hVvtp:I:e:E:o:R:r:a:m:kgPc:x:D:bs:uqiwl:d:L:FS:B:J:jWG:T:"
+#define STAP_SHORT_OPTIONS "hVvtp:I:e:E:o:O:R:r:a:m:kgPc:x:D:bs:uqiwl:d:L:FS:B:J:jWG:T:"
 
 extern struct option stap_long_options[];
 
diff --git a/session.cxx b/session.cxx
index 04904c258..d70768ad3 100644
--- a/session.cxx
+++ b/session.cxx
@@ -619,6 +619,16 @@ systemtap_session::usage (int exitcode)
     "              %s\n"
     "   -o FILE    send script output to file, instead of stdout. This supports\n" 
     "              strftime(3) formats for FILE\n"
+    "   -O[0123sg] optimization to use for C code. Passed to gcc in pass 4\n"
+    "              (not applicable to the bpf runtime). defaults to -O2.\n"
+    "              -O0 Fast compilation (not applicable to the kernel runtime)\n"
+    "              -O1 Optimize, takes a bit more time\n"
+    "              -O2 Optimize more, takes more time\n"
+    "              -O3 Optimize even more, takes even more time\n"
+    "              -Os Optimize for size, like -O2 but tuned for small code size\n"
+    "              -Og Optimize debugging experience, do not interfere with debugging\n"
+    "              use of non-default optimization levels may break the build.\n"
+    "              use it at your own risk.\n"
     "   -E SCRIPT  run the SCRIPT in addition to the main script specified\n"
     "              through -e or a script file\n"
     "   -c CMD     start the probes, run CMD, and exit when it finishes\n"
@@ -736,6 +746,7 @@ systemtap_session::parse_cmdline (int argc, char * const argv [])
   struct rlimit our_rlimit;
   bool sysroot_option_seen = false;
   string kernel_release_value;
+  std::string O_flag;
 
   while (true)
     {
@@ -850,6 +861,13 @@ systemtap_session::parse_cmdline (int argc, char * const argv [])
           output_file = string (optarg);
           break;
 
+        case 'O':
+          assert(optarg);
+          assert_regexp_match("-O parameter", optarg, "^[0-3sg]$");
+          O_flag = string ("-O") + optarg;
+          cc_flags.push_back (O_flag);
+          break;
+
         case 'R':
           assert(optarg);
           if (client_options) { cerr << _F("ERROR: %s invalid with %s", "-R", "--client-options") << endl; return 1; }
@@ -1662,6 +1680,23 @@ systemtap_session::parse_cmdline (int argc, char * const argv [])
         }
     }
 
+  if (!O_flag.empty())
+    {
+      if (runtime_mode == bpf_runtime)
+        {
+          cerr << _F("Option %s not supported for the bpf runtime.",
+                     O_flag.c_str()) << endl;
+          return 1;
+        }
+
+      if (runtime_mode == kernel_runtime && O_flag == "-O0")
+        {
+          cerr << _("Option -O0 does not work with the kernel runtime.")
+               << endl;
+          return 1;
+        }
+    }
+
   for (std::set<std::string>::iterator it = additional_unwindsym_modules.begin();
        it != additional_unwindsym_modules.end();
        it++)
diff --git a/session.h b/session.h
index 3f160cc6b..bf1c410fc 100644
--- a/session.h
+++ b/session.h
@@ -328,6 +328,9 @@ public:
   // Skip bad $ vars
   bool skip_badvars;
 
+  // Extra C compiler options for the kernel and dyninst modes
+  std::vector<std::string> cc_flags;
+
   // NB: It is very important for all of the above (and below) fields
   // to be cleared in the systemtap_session ctor (session.cxx).
 
diff --git a/testsuite/systemtap.base/cmdline_opt_O.exp b/testsuite/systemtap.base/cmdline_opt_O.exp
new file mode 100644
index 000000000..e844b8f18
--- /dev/null
+++ b/testsuite/systemtap.base/cmdline_opt_O.exp
@@ -0,0 +1,387 @@
+set test "cmdline_opt_O"
+set testpath "$srcdir/$subdir"
+
+if {! [installtest_p]} { untested "$test"; return }
+
+# --- TEST 1 ---
+
+set subtest1 "TEST 1: bad level"
+foreach runtime [get_runtime_list] {
+    if {$runtime eq ""} {
+        set runtime "kernel"
+    }
+    set test_name "$test: $subtest1 ($runtime)"
+
+    set cmd "stap --disable-cache -Ow --runtime=$runtime '$srcdir/$subdir/${test}_1.stp'"
+    send_log "executing: $cmd\n"
+    set pipe [open "| sh -c {$cmd}" r]
+    set out [read $pipe]
+    set failed 0
+    if {[catch {close $pipe} stderr] != 0} {
+        if {$stderr ne "" && [string index $stderr end] ne "\n"} {
+            append stderr "\n"
+        }
+        global errorCode
+        if {"CHILDSTATUS" == [lindex $errorCode 0]} {
+            set failed [lindex $errorCode 2]
+        }
+    }
+
+
+    if {$failed} {
+        pass "${test_name}: exit code should be non-zero"
+    } else {
+        fail "${test_name}: exit code should be non-zero but is zero"
+    }
+
+    set exp_stderr "ERROR: Safety pattern mismatch for -O parameter ('w' vs. '^\[0-3sg\]\$') rc=1\n"
+    regsub -all -- {\n} $exp_stderr {\n} escaped_exp_stderr
+    if {$stderr eq $exp_stderr} {
+        pass "${test_name}: stderr matches \"$escaped_exp_stderr\""
+    } else {
+        fail "${test_name}: stderr fails to match \"$escaped_exp_stderr\": got \"$stderr\""
+    }
+}
+
+# --- TEST 2 ---
+
+set subtest2 "TEST 2: -O0"
+foreach runtime [get_runtime_list] {
+    if {$runtime eq ""} {
+        set runtime "kernel"
+    }
+    set test_name "$test: $subtest2 ($runtime)"
+
+    set cmd "stap --disable-cache -O0 -vvv --runtime=$runtime '$srcdir/$subdir/${test}_1.stp'"
+    send_log "executing: $cmd\n"
+    set pipe [open "| sh -c {$cmd}" r]
+    set out [read $pipe]
+    set failed 0
+    if {[catch {close $pipe} stderr] != 0} {
+        if {$stderr ne "" && [string index $stderr end] ne "\n"} {
+            append stderr "\n"
+        }
+        global errorCode
+        if {"CHILDSTATUS" == [lindex $errorCode 0]} {
+            set failed [lindex $errorCode 2]
+        }
+    }
+
+    if {$runtime eq "dyninst"} {
+        set out_pat "^hello world\\n\\Z"
+        regsub -all -- {\n} $out_pat {\n} escaped_out_pat
+        if {[regexp -lineanchor -- $out_pat $out]} {
+            pass "${test_name}: stdout matches \"$escaped_out_pat\""
+        } else {
+            fail "${test_name}: stdout fails to match \"$escaped_out_pat\": got \"$out\""
+        }
+    }
+    if {$runtime eq "kernel"} {
+
+        if {$failed} {
+            pass "${test_name}: exit code should be non-zero"
+        } else {
+            fail "${test_name}: exit code should be non-zero but is zero"
+        }
+
+        set exp_stderr "Option -O0 does not work with the kernel runtime.\n"
+        regsub -all -- {\n} $exp_stderr {\n} escaped_exp_stderr
+        if {$stderr eq $exp_stderr} {
+            pass "${test_name}: stderr matches \"$escaped_exp_stderr\""
+        } else {
+            fail "${test_name}: stderr fails to match \"$escaped_exp_stderr\": got \"$stderr\""
+        }
+
+    } else {
+        if {$failed} {
+            fail "${test_name}: exit code should be zero but is $failed"
+        } else {
+            pass "${test_name}: exit code is zero"
+        }
+    }
+}
+
+# --- TEST 3 ---
+
+set subtest3 "TEST 3: -O1"
+foreach runtime [get_runtime_list] {
+    if {$runtime eq ""} {
+        set runtime "kernel"
+    }
+    set test_name "$test: $subtest3 ($runtime)"
+
+    set cmd "stap --disable-cache -O1 -vvv --runtime=$runtime '$srcdir/$subdir/${test}_1.stp'"
+    send_log "executing: $cmd\n"
+    set pipe [open "| sh -c {$cmd}" r]
+    set out [read $pipe]
+    set failed 0
+    if {[catch {close $pipe} stderr] != 0} {
+        if {$stderr ne "" && [string index $stderr end] ne "\n"} {
+            append stderr "\n"
+        }
+        global errorCode
+        if {"CHILDSTATUS" == [lindex $errorCode 0]} {
+            set failed [lindex $errorCode 2]
+        }
+    }
+
+    if {$runtime eq "kernel"} {
+        set out_pat "\\ygcc \[^\\n\]*? -O2 \[^\\n\]*? '?-O1'? .*?^hello world\\n\\Z"
+        regsub -all -- {\n} $out_pat {\n} escaped_out_pat
+        if {[regexp -lineanchor -- $out_pat $out]} {
+            pass "${test_name}: stdout matches \"$escaped_out_pat\""
+        } else {
+            fail "${test_name}: stdout fails to match \"$escaped_out_pat\": got \"$out\""
+        }
+    } else {
+        set out_pat "^hello world\\n\\Z"
+        regsub -all -- {\n} $out_pat {\n} escaped_out_pat
+        if {[regexp -lineanchor -- $out_pat $out]} {
+            pass "${test_name}: stdout matches \"$escaped_out_pat\""
+        } else {
+            fail "${test_name}: stdout fails to match \"$escaped_out_pat\": got \"$out\""
+        }
+    }
+
+    set stderr_pat "^Pass 5: run completed in "
+    regsub -all -- {\n} $stderr_pat {\n} escaped_stderr_pat
+    if {[regexp -lineanchor -- $stderr_pat $stderr]} {
+        pass "${test_name}: stderr matches \"$escaped_stderr_pat\""
+    } else {
+        fail "${test_name}: stderr fails to match \"$escaped_stderr_pat\": got \"$stderr\""
+    }
+
+    if {$failed} {
+        fail "${test_name}: exit code should be zero but is $failed"
+    } else {
+        pass "${test_name}: exit code is zero"
+    }
+}
+
+# --- TEST 4 ---
+
+set subtest4 "TEST 4: -O2"
+foreach runtime [get_runtime_list] {
+    if {$runtime eq ""} {
+        set runtime "kernel"
+    }
+    set test_name "$test: $subtest4 ($runtime)"
+
+    set cmd "stap --disable-cache -O2 -vvv --runtime=$runtime '$srcdir/$subdir/${test}_1.stp'"
+    send_log "executing: $cmd\n"
+    set pipe [open "| sh -c {$cmd}" r]
+    set out [read $pipe]
+    set failed 0
+    if {[catch {close $pipe} stderr] != 0} {
+        if {$stderr ne "" && [string index $stderr end] ne "\n"} {
+            append stderr "\n"
+        }
+        global errorCode
+        if {"CHILDSTATUS" == [lindex $errorCode 0]} {
+            set failed [lindex $errorCode 2]
+        }
+    }
+
+    if {$runtime eq "kernel"} {
+        set out_pat "\\ygcc \[^\\n\]*? -O2 \[^\\n\]*? '?-O2'? .*?^hello world\\n\\Z"
+        regsub -all -- {\n} $out_pat {\n} escaped_out_pat
+        if {[regexp -lineanchor -- $out_pat $out]} {
+            pass "${test_name}: stdout matches \"$escaped_out_pat\""
+        } else {
+            fail "${test_name}: stdout fails to match \"$escaped_out_pat\": got \"$out\""
+        }
+    } else {
+        set out_pat "^hello world\\n\\Z"
+        regsub -all -- {\n} $out_pat {\n} escaped_out_pat
+        if {[regexp -lineanchor -- $out_pat $out]} {
+            pass "${test_name}: stdout matches \"$escaped_out_pat\""
+        } else {
+            fail "${test_name}: stdout fails to match \"$escaped_out_pat\": got \"$out\""
+        }
+    }
+
+    set stderr_pat "^Pass 5: run completed in "
+    regsub -all -- {\n} $stderr_pat {\n} escaped_stderr_pat
+    if {[regexp -lineanchor -- $stderr_pat $stderr]} {
+        pass "${test_name}: stderr matches \"$escaped_stderr_pat\""
+    } else {
+        fail "${test_name}: stderr fails to match \"$escaped_stderr_pat\": got \"$stderr\""
+    }
+
+    if {$failed} {
+        fail "${test_name}: exit code should be zero but is $failed"
+    } else {
+        pass "${test_name}: exit code is zero"
+    }
+}
+
+# --- TEST 5 ---
+
+set subtest5 "TEST 5: -O3"
+foreach runtime [get_runtime_list] {
+    if {$runtime eq ""} {
+        set runtime "kernel"
+    }
+    set test_name "$test: $subtest5 ($runtime)"
+
+    set cmd "stap --disable-cache -O3 -vvv --runtime=$runtime '$srcdir/$subdir/${test}_1.stp'"
+    send_log "executing: $cmd\n"
+    set pipe [open "| sh -c {$cmd}" r]
+    set out [read $pipe]
+    set failed 0
+    if {[catch {close $pipe} stderr] != 0} {
+        if {$stderr ne "" && [string index $stderr end] ne "\n"} {
+            append stderr "\n"
+        }
+        global errorCode
+        if {"CHILDSTATUS" == [lindex $errorCode 0]} {
+            set failed [lindex $errorCode 2]
+        }
+    }
+
+    if {$runtime eq "kernel"} {
+        set out_pat "\\ygcc \[^\\n\]*? -O2 \[^\\n\]*? '?-O3'? .*?^hello world\\n\\Z"
+        regsub -all -- {\n} $out_pat {\n} escaped_out_pat
+        if {[regexp -lineanchor -- $out_pat $out]} {
+            pass "${test_name}: stdout matches \"$escaped_out_pat\""
+        } else {
+            fail "${test_name}: stdout fails to match \"$escaped_out_pat\": got \"$out\""
+        }
+    } else {
+        set out_pat "^hello world\\n\\Z"
+        regsub -all -- {\n} $out_pat {\n} escaped_out_pat
+        if {[regexp -lineanchor -- $out_pat $out]} {
+            pass "${test_name}: stdout matches \"$escaped_out_pat\""
+        } else {
+            fail "${test_name}: stdout fails to match \"$escaped_out_pat\": got \"$out\""
+        }
+    }
+
+    set stderr_pat "^Pass 5: run completed in "
+    regsub -all -- {\n} $stderr_pat {\n} escaped_stderr_pat
+    if {[regexp -lineanchor -- $stderr_pat $stderr]} {
+        pass "${test_name}: stderr matches \"$escaped_stderr_pat\""
+    } else {
+        fail "${test_name}: stderr fails to match \"$escaped_stderr_pat\": got \"$stderr\""
+    }
+
+    if {$failed} {
+        fail "${test_name}: exit code should be zero but is $failed"
+    } else {
+        pass "${test_name}: exit code is zero"
+    }
+}
+
+# --- TEST 6 ---
+
+set subtest6 "TEST 6: -Os"
+foreach runtime [get_runtime_list] {
+    if {$runtime eq ""} {
+        set runtime "kernel"
+    }
+    set test_name "$test: $subtest6 ($runtime)"
+
+    set cmd "stap --disable-cache -Os -vvv --runtime=$runtime '$srcdir/$subdir/${test}_1.stp'"
+    send_log "executing: $cmd\n"
+    set pipe [open "| sh -c {$cmd}" r]
+    set out [read $pipe]
+    set failed 0
+    if {[catch {close $pipe} stderr] != 0} {
+        if {$stderr ne "" && [string index $stderr end] ne "\n"} {
+            append stderr "\n"
+        }
+        global errorCode
+        if {"CHILDSTATUS" == [lindex $errorCode 0]} {
+            set failed [lindex $errorCode 2]
+        }
+    }
+
+    if {$runtime eq "kernel"} {
+        set out_pat "\\ygcc \[^\\n\]*? -O2 \[^\\n\]*? '?-Os'? .*?^hello world\\n\\Z"
+        regsub -all -- {\n} $out_pat {\n} escaped_out_pat
+        if {[regexp -lineanchor -- $out_pat $out]} {
+            pass "${test_name}: stdout matches \"$escaped_out_pat\""
+        } else {
+            fail "${test_name}: stdout fails to match \"$escaped_out_pat\": got \"$out\""
+        }
+    } else {
+        set out_pat "^hello world\\n\\Z"
+        regsub -all -- {\n} $out_pat {\n} escaped_out_pat
+        if {[regexp -lineanchor -- $out_pat $out]} {
+            pass "${test_name}: stdout matches \"$escaped_out_pat\""
+        } else {
+            fail "${test_name}: stdout fails to match \"$escaped_out_pat\": got \"$out\""
+        }
+    }
+
+    set stderr_pat "^Pass 5: run completed in "
+    regsub -all -- {\n} $stderr_pat {\n} escaped_stderr_pat
+    if {[regexp -lineanchor -- $stderr_pat $stderr]} {
+        pass "${test_name}: stderr matches \"$escaped_stderr_pat\""
+    } else {
+        fail "${test_name}: stderr fails to match \"$escaped_stderr_pat\": got \"$stderr\""
+    }
+
+    if {$failed} {
+        fail "${test_name}: exit code should be zero but is $failed"
+    } else {
+        pass "${test_name}: exit code is zero"
+    }
+}
+
+# --- TEST 7 ---
+
+set subtest7 "TEST 7: -Og"
+foreach runtime [get_runtime_list] {
+    if {$runtime eq ""} {
+        set runtime "kernel"
+    }
+    set test_name "$test: $subtest7 ($runtime)"
+
+    set cmd "stap --disable-cache -Og -vvv --runtime=$runtime '$srcdir/$subdir/${test}_1.stp'"
+    send_log "executing: $cmd\n"
+    set pipe [open "| sh -c {$cmd}" r]
+    set out [read $pipe]
+    set failed 0
+    if {[catch {close $pipe} stderr] != 0} {
+        if {$stderr ne "" && [string index $stderr end] ne "\n"} {
+            append stderr "\n"
+        }
+        global errorCode
+        if {"CHILDSTATUS" == [lindex $errorCode 0]} {
+            set failed [lindex $errorCode 2]
+        }
+    }
+
+    if {$runtime eq "kernel"} {
+        set out_pat "\\ygcc \[^\\n\]*? -O2 \[^\\n\]*? '?-Og'? .*?^hello world\\n\\Z"
+        regsub -all -- {\n} $out_pat {\n} escaped_out_pat
+        if {[regexp -lineanchor -- $out_pat $out]} {
+            pass "${test_name}: stdout matches \"$escaped_out_pat\""
+        } else {
+            fail "${test_name}: stdout fails to match \"$escaped_out_pat\": got \"$out\""
+        }
+    } else {
+        set out_pat "^hello world\\n\\Z"
+        regsub -all -- {\n} $out_pat {\n} escaped_out_pat
+        if {[regexp -lineanchor -- $out_pat $out]} {
+            pass "${test_name}: stdout matches \"$escaped_out_pat\""
+        } else {
+            fail "${test_name}: stdout fails to match \"$escaped_out_pat\": got \"$out\""
+        }
+    }
+
+    set stderr_pat "^Pass 5: run completed in "
+    regsub -all -- {\n} $stderr_pat {\n} escaped_stderr_pat
+    if {[regexp -lineanchor -- $stderr_pat $stderr]} {
+        pass "${test_name}: stderr matches \"$escaped_stderr_pat\""
+    } else {
+        fail "${test_name}: stderr fails to match \"$escaped_stderr_pat\": got \"$stderr\""
+    }
+
+    if {$failed} {
+        fail "${test_name}: exit code should be zero but is $failed"
+    } else {
+        pass "${test_name}: exit code is zero"
+    }
+}
diff --git a/testsuite/systemtap.base/cmdline_opt_O_1.stp b/testsuite/systemtap.base/cmdline_opt_O_1.stp
new file mode 100644
index 000000000..abf806a42
--- /dev/null
+++ b/testsuite/systemtap.base/cmdline_opt_O_1.stp
@@ -0,0 +1 @@
+probe begin { log("hello world") exit() }
-- 
2.11.0.295.gd7dffce

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2018-08-25  6:44 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-25  6:44 [PATCH] Add back the command-line options -O[0123sg] Yichun Zhang (agentzh)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).