> On Dec 1, 2020, at 6:16 PM, Jeff Law <l...@redhat.com> wrote:
>>>> From c2573c6c8552b7b4c2eedb0684ce48b5c11436ec Mon Sep 17 00:00:00 2001
>>>> From: qing zhao <qinz...@gcc.gnu.org>
>>>> Date: Thu, 19 Nov 2020 16:46:50 +0100
>>>> Subject: [PATCH] rtl-optimization: Fix data flow maintenance bug in
>>>> reg-stack.c [pr97777]
>>>> 
>>>> reg-stack pass does not maintain the data flow information correctly.
>>>> call df_insn_rescan_all after the transformation is done.
>>>> 
>>>>        gcc/
>>>> PR rtl-optimization/97777
>>>> * reg-stack.c (rest_of_handle_stack_regs): call
>>>> df_insn_rescan_all if reg_to_stack return true.
>>>> 
>>>> gcc/testsuite/
>>>> PR rtl-optimization/97777
>>>> * gcc.target/i386/pr97777.c: New test.
>>> I'd like to see more analysis here.
>>> 
>>> ie, precisely what data is out of date and why?
>> 
>> For the simple testing case, what happened is, for the insn #6:
>> 
>> (gdb) call debug_rtx(insn)
>> (insn 6 26 7 2 (set (reg:XF 8 st [84])
>>         (reg:XF 9 st(1) [85])) "t.c":4:10 134 {*movxf_internal}
>>      (expr_list:REG_EQUAL (const_double:XF 0.0 [0x0.0p+0])
>>         (nil)))
>> 
>> After the following statement in reg-stack.c:
>>    3080           control_flow_insn_deleted |= subst_stack_regs (insn,
>> &regstack);
>> 
>> This insn # 6 becomes:
>> (gdb) call debug_rtx(insn)
>> (insn 6 26 7 2 (set (reg:XF 8 st)
>>         (reg:XF 8 st)) "t.c":4:10 134 {*movxf_internal}
>>      (expr_list:REG_EQUAL (const_double:XF 0.0 [0x0.0p+0])
>>         (nil)))
>> 
>> However, there is no any df maintenance routine (for example,
>> df_insn_rescan, etc) is called for this changed insn.
> So we are clearing the x87 registers with that option.  Hence the change
> in reg-stack behavior.  I'm a bit surprised by this as I don't see
> clearing the x87 registers as particularly helpful from a security
> standpoint.  But I haven't followed that discussion closely.

Even with the option -fzero-call-used-regs=used-gpr (without clearing any x87 
registers), 
We have the same compiler time error. 

The first thing that the new pass “zero_call_used_regs” does is:

df_analyze();

And the compiler error happens inside this call. 

From the following passes list:

          NEXT_PASS (pass_stack_regs);
          PUSH_INSERT_PASSES_WITHIN (pass_stack_regs)
              NEXT_PASS (pass_split_before_regstack);
              NEXT_PASS (pass_stack_regs_run);
          POP_INSERT_PASSES ()
      POP_INSERT_PASSES ()
      NEXT_PASS (pass_late_compilation);
      PUSH_INSERT_PASSES_WITHIN (pass_late_compilation)
          NEXT_PASS (pass_zero_call_used_regs);
          NEXT_PASS (pass_compute_alignments);
          NEXT_PASS (pass_variable_tracking);
          NEXT_PASS (pass_free_cfg);
          NEXT_PASS (pass_machine_reorg);
          NEXT_PASS (pass_cleanup_barriers);
          NEXT_PASS (pass_delay_slots);
          NEXT_PASS (pass_split_for_shorten_branches);
          NEXT_PASS (pass_convert_to_eh_region_ranges);
          NEXT_PASS (pass_shorten_branches);
          NEXT_PASS (pass_set_nothrow_function_flags);
          NEXT_PASS (pass_dwarf2_frame);
          NEXT_PASS (pass_final);

We can see that the new pass “zero_call_used_regs” immediately follows pass 
“stack_regs”.

And all other passes that follows “stack_regs” do not call “df_analyze()”.


> 
>> 
>> As I checked, the transformation for this pass “stack” is quite
>> complicated. In addition to the above register replacement,
>> New insns might be inserted, and control flow might be changed, but
>> for most of the transformations applied in this pass,
>> There is no corresponding df maintenance routine is added for deferred
>> df rescanning.
> But this has been the case essentially for ever with reg-stack.  So
> what's unclear to me is why it's suddenly a problem now.

Previously, without the new pass “zero_call_used_regs”, all the passes 
following “stack_regs”
do not call df_analyze, therefore never expose this bug. 

The new pass “zero_call_used_regs” is the first pass that follows “stack_regs” 
and call “df_analyze”, 
Therefore triggered this old bug.

Instead of adding -fzero-call-used-regs option, if I change final.c as 
following to add “df_analyze” at
The beginning of the pass “pass_compute_alignments”:

qinzhao@gcc10:~/Work/write_gcc/gcc$ git diff final.c
diff --git a/gcc/final.c b/gcc/final.c
index fc9a05e335f..4955fc3fdcb 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -639,6 +639,7 @@ compute_alignments (void)
   basic_block bb;
   align_flags max_alignment;
 
+  df_analyze();
   label_align.truncate (0);
 
   max_labelno = max_label_num ();


We will have the exactly same bug as adding -fzero-call-used-regs:
qinzhao@gcc10:~/Bugs/b_97777$ sh t
/home/qinzhao/Install/latest_write/bin/gcc -O -ffinite-math-only -S t.c
during RTL pass: alignments
t.c: In function ‘foo’:
t.c:5:1: internal compiler error: in df_refs_verify, at df-scan.c:3991
    5 | }
      | ^
0xc3f98a df_refs_verify
        ../../write_gcc/gcc/df-scan.c:3991
0xc3fbfb df_insn_refs_verify
        ../../write_gcc/gcc/df-scan.c:4075
0xc3fdd9 df_bb_verify
        ../../write_gcc/gcc/df-scan.c:4107
0xc4039c df_scan_verify()
        ../../write_gcc/gcc/df-scan.c:4228
0xc26a66 df_verify()
        ../../write_gcc/gcc/df-core.c:1818
0xc252d7 df_analyze_1
        ../../write_gcc/gcc/df-core.c:1214
0xc25698 df_analyze()
        ../../write_gcc/gcc/df-core.c:1290
0xd4c706 compute_alignments()
        ../../write_gcc/gcc/final.c:642
0xd4d004 execute
        ../../write_gcc/gcc/final.c:826
Please submit a full bug report,
with preprocessed source if appropriate.

Qing

> 
> Jeff
> 

Reply via email to