[PATCH] Check dominator info in compute_dominance_frontiers

[PATCH] Check dominator info in compute_dominance_frontiers

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 1986 bytes --]

Hi,

during development of a patch I ran into a case where 
compute_dominance_frontiers was called with incorrect dominance info.

The result was a segmentation violation somewhere in the bitmap code 
while executing this bitmap_set_bit in compute_dominance_frontiers_1:
...
                   if (!bitmap_set_bit (&frontiers[runner->index],
                                        b->index))
                     break;
...

The segmentation violation happens because runner->index is 0, and 
frontiers[0] is uninitialized.

[ The initialization in update_ssa looks like this:
...
      dfs = XNEWVEC (bitmap_head, last_basic_block_for_fn (cfun));
       FOR_EACH_BB_FN (bb, cfun)
         bitmap_initialize (&dfs[bb->index], &bitmap_default_obstack);
       compute_dominance_frontiers (dfs);
...

FOR_EACH_BB_FN skips over the entry-block and the exit-block, so dfs[0] 
(frontiers[0] in compute_dominance_frontiers_1) is not initialized.

We could add initialization by making the entry/exit-block bitmap_heads 
empty and setting the obstack to a reserved obstack bitmap_no_obstack 
for which allocation results in an assert. ]

AFAIU, the immediate problem is not that frontiers[0] is uninitialized, 
but that the loop reaches the state of runner->index == 0, due to the 
incorrect dominance info.

The patch adds an assert to the loop in compute_dominance_frontiers_1, 
to make the failure mode cleaner and easier to understand.

I think we wouldn't catch all errors in dominance info with this assert. 
So the patch also contains an ENABLE_CHECKING-enabled verify_dominators 
call at the start of compute_dominance_frontiers. I'm not sure if:
- adding the verify_dominators call is too costly in runtime.
- the verify_dominators call should be inside or outside the
   TV_DOM_FRONTIERS measurement.
- there is a level of ENABLE_CHECKING that is more appropriate for the
   verify_dominators call.

Is this ok for trunk if bootstrap and reg-test on x86_64 succeeds?

Thanks,
- Tom

[-- Attachment #2: 0002-Check-dominator-info-in-compute_dominance_frontiers.patch --]
[-- Type: text/x-patch, Size: 1227 bytes --]

Check dominator info in compute_dominance_frontiers

2015-06-22  Tom de Vries  <tom@codesourcery.com>

	* cfganal.c (compute_dominance_frontiers_1): Add assert.
	(compute_dominance_frontiers): Verify dominators if ENABLE_CHECKING.
---
 gcc/cfganal.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/gcc/cfganal.c b/gcc/cfganal.c
index b8d67bc..0e0e2bb 100644
--- a/gcc/cfganal.c
+++ b/gcc/cfganal.c
@@ -1261,6 +1261,11 @@ compute_dominance_frontiers_1 (bitmap_head *frontiers)
 	      domsb = get_immediate_dominator (CDI_DOMINATORS, b);
 	      while (runner != domsb)
 		{
+		  /* If you're running into this assert, the dominator info is
+		     incorrect.  Try enabling the verify_dominators call at the
+		     start of compute_dominance_frontiers.  */
+		  gcc_assert (runner != ENTRY_BLOCK_PTR_FOR_FN (cfun));
+
 		  if (!bitmap_set_bit (&frontiers[runner->index],
 				       b->index))
 		    break;
@@ -1276,6 +1281,10 @@ compute_dominance_frontiers_1 (bitmap_head *frontiers)
 void
 compute_dominance_frontiers (bitmap_head *frontiers)
 {
+#if ENABLE_CHECKING
+  verify_dominators (CDI_DOMINATORS);
+#endif
+
   timevar_push (TV_DOM_FRONTIERS);
 
   compute_dominance_frontiers_1 (frontiers);
-- 
1.9.1

Re: [PATCH] Check dominator info in compute_dominance_frontiers

[Richard Biener]

On Mon, Jun 22, 2015 at 10:04 AM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> Hi,
>
> during development of a patch I ran into a case where
> compute_dominance_frontiers was called with incorrect dominance info.
>
> The result was a segmentation violation somewhere in the bitmap code while
> executing this bitmap_set_bit in compute_dominance_frontiers_1:
> ...
>                   if (!bitmap_set_bit (&frontiers[runner->index],
>                                        b->index))
>                     break;
> ...
>
> The segmentation violation happens because runner->index is 0, and
> frontiers[0] is uninitialized.
>
> [ The initialization in update_ssa looks like this:
> ...
>      dfs = XNEWVEC (bitmap_head, last_basic_block_for_fn (cfun));
>       FOR_EACH_BB_FN (bb, cfun)
>         bitmap_initialize (&dfs[bb->index], &bitmap_default_obstack);
>       compute_dominance_frontiers (dfs);
> ...
>
> FOR_EACH_BB_FN skips over the entry-block and the exit-block, so dfs[0]
> (frontiers[0] in compute_dominance_frontiers_1) is not initialized.
>
> We could add initialization by making the entry/exit-block bitmap_heads
> empty and setting the obstack to a reserved obstack bitmap_no_obstack for
> which allocation results in an assert. ]
>
> AFAIU, the immediate problem is not that frontiers[0] is uninitialized, but
> that the loop reaches the state of runner->index == 0, due to the incorrect
> dominance info.
>
> The patch adds an assert to the loop in compute_dominance_frontiers_1, to
> make the failure mode cleaner and easier to understand.
>
> I think we wouldn't catch all errors in dominance info with this assert. So
> the patch also contains an ENABLE_CHECKING-enabled verify_dominators call at
> the start of compute_dominance_frontiers. I'm not sure if:
> - adding the verify_dominators call is too costly in runtime.
> - the verify_dominators call should be inside or outside the
>   TV_DOM_FRONTIERS measurement.
> - there is a level of ENABLE_CHECKING that is more appropriate for the
>   verify_dominators call.
>
> Is this ok for trunk if bootstrap and reg-test on x86_64 succeeds?

I don't think these kind of asserts are good.  A segfault is good by itself
(so you can just add the comment if you like).

Likewise the verify_dominators call is too expensive and misplaced.

If then the call belongs in the dom_computed[] == DOM_OK early-out
in calculate_dominance_info (eventually also for the case where we
end up only computing the fast-query stuff).

Richard.

> Thanks,
> - Tom

Re: [PATCH] Check dominator info in compute_dominance_frontiers

[Tom de Vries]

On 22/06/15 12:14, Richard Biener wrote:
> On Mon, Jun 22, 2015 at 10:04 AM, Tom de Vries <Tom_deVries@mentor.com> wrote:
>> Hi,
>>
>> during development of a patch I ran into a case where
>> compute_dominance_frontiers was called with incorrect dominance info.
>>
>> The result was a segmentation violation somewhere in the bitmap code while
>> executing this bitmap_set_bit in compute_dominance_frontiers_1:
>> ...
>>                    if (!bitmap_set_bit (&frontiers[runner->index],
>>                                         b->index))
>>                      break;
>> ...
>>
>> The segmentation violation happens because runner->index is 0, and
>> frontiers[0] is uninitialized.
>>
>> [ The initialization in update_ssa looks like this:
>> ...
>>       dfs = XNEWVEC (bitmap_head, last_basic_block_for_fn (cfun));
>>        FOR_EACH_BB_FN (bb, cfun)
>>          bitmap_initialize (&dfs[bb->index], &bitmap_default_obstack);
>>        compute_dominance_frontiers (dfs);
>> ...
>>
>> FOR_EACH_BB_FN skips over the entry-block and the exit-block, so dfs[0]
>> (frontiers[0] in compute_dominance_frontiers_1) is not initialized.
>>
>> We could add initialization by making the entry/exit-block bitmap_heads
>> empty and setting the obstack to a reserved obstack bitmap_no_obstack for
>> which allocation results in an assert. ]
>>
>> AFAIU, the immediate problem is not that frontiers[0] is uninitialized, but
>> that the loop reaches the state of runner->index == 0, due to the incorrect
>> dominance info.
>>
>> The patch adds an assert to the loop in compute_dominance_frontiers_1, to
>> make the failure mode cleaner and easier to understand.
>>
>> I think we wouldn't catch all errors in dominance info with this assert. So
>> the patch also contains an ENABLE_CHECKING-enabled verify_dominators call at
>> the start of compute_dominance_frontiers. I'm not sure if:
>> - adding the verify_dominators call is too costly in runtime.
>> - the verify_dominators call should be inside or outside the
>>    TV_DOM_FRONTIERS measurement.
>> - there is a level of ENABLE_CHECKING that is more appropriate for the
>>    verify_dominators call.
>>
>> Is this ok for trunk if bootstrap and reg-test on x86_64 succeeds?
>
> I don't think these kind of asserts are good.  A segfault is good by itself
> (so you can just add the comment if you like).
>

The segfault is not guaranteed to trigger, because it works on 
uninitialized data. Instead, we may end up modifying valid memory and 
silently generating wrong code or causing sigsegvs (which will be 
difficult to track back this error). So I don't think doing nothing is 
an option here. If we're not going to add this assert, we should 
initialize the uninitialized data in such a way that we are guaranteed 
to detect the error. The scheme I proposed above would take care of 
that. Should I implement that instead?

> Likewise the verify_dominators call is too expensive and misplaced.
>
> If then the call belongs in the dom_computed[] == DOM_OK early-out
> in calculate_dominance_info

OK, like this:
...
diff --git a/gcc/dominance.c b/gcc/dominance.c
index a9e042e..1827eda9 100644
--- a/gcc/dominance.c
+++ b/gcc/dominance.c
@@ -646,7 +646,12 @@ calculate_dominance_info (enum cdi_direction dir)
    bool reverse = (dir == CDI_POST_DOMINATORS) ? true : false;

    if (dom_computed[dir_index] == DOM_OK)
-    return;
+    {
+#if ENABLE_CHECKING
+      verify_dominators (CDI_DOMINATORS);
+#endif
+      return;
+    }

    timevar_push (TV_DOMINANCE);
    if (!dom_info_available_p (dir))
...

I didn't fully understand your comment, do you want me to test this?

Thanks,
- Tom

> (eventually also for the case where we
> end up only computing the fast-query stuff).

Re: [PATCH] Check dominator info in compute_dominance_frontiers

[Richard Biener]

On Mon, Jun 22, 2015 at 1:33 PM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 22/06/15 12:14, Richard Biener wrote:
>>
>> On Mon, Jun 22, 2015 at 10:04 AM, Tom de Vries <Tom_deVries@mentor.com>
>> wrote:
>>>
>>> Hi,
>>>
>>> during development of a patch I ran into a case where
>>> compute_dominance_frontiers was called with incorrect dominance info.
>>>
>>> The result was a segmentation violation somewhere in the bitmap code
>>> while
>>> executing this bitmap_set_bit in compute_dominance_frontiers_1:
>>> ...
>>>                    if (!bitmap_set_bit (&frontiers[runner->index],
>>>                                         b->index))
>>>                      break;
>>> ...
>>>
>>> The segmentation violation happens because runner->index is 0, and
>>> frontiers[0] is uninitialized.
>>>
>>> [ The initialization in update_ssa looks like this:
>>> ...
>>>       dfs = XNEWVEC (bitmap_head, last_basic_block_for_fn (cfun));
>>>        FOR_EACH_BB_FN (bb, cfun)
>>>          bitmap_initialize (&dfs[bb->index], &bitmap_default_obstack);
>>>        compute_dominance_frontiers (dfs);
>>> ...
>>>
>>> FOR_EACH_BB_FN skips over the entry-block and the exit-block, so dfs[0]
>>> (frontiers[0] in compute_dominance_frontiers_1) is not initialized.
>>>
>>> We could add initialization by making the entry/exit-block bitmap_heads
>>> empty and setting the obstack to a reserved obstack bitmap_no_obstack for
>>> which allocation results in an assert. ]
>>>
>>> AFAIU, the immediate problem is not that frontiers[0] is uninitialized,
>>> but
>>> that the loop reaches the state of runner->index == 0, due to the
>>> incorrect
>>> dominance info.
>>>
>>> The patch adds an assert to the loop in compute_dominance_frontiers_1, to
>>> make the failure mode cleaner and easier to understand.
>>>
>>> I think we wouldn't catch all errors in dominance info with this assert.
>>> So
>>> the patch also contains an ENABLE_CHECKING-enabled verify_dominators call
>>> at
>>> the start of compute_dominance_frontiers. I'm not sure if:
>>> - adding the verify_dominators call is too costly in runtime.
>>> - the verify_dominators call should be inside or outside the
>>>    TV_DOM_FRONTIERS measurement.
>>> - there is a level of ENABLE_CHECKING that is more appropriate for the
>>>    verify_dominators call.
>>>
>>> Is this ok for trunk if bootstrap and reg-test on x86_64 succeeds?
>>
>>
>> I don't think these kind of asserts are good.  A segfault is good by
>> itself
>> (so you can just add the comment if you like).
>>
>
> The segfault is not guaranteed to trigger, because it works on uninitialized
> data. Instead, we may end up modifying valid memory and silently generating
> wrong code or causing sigsegvs (which will be difficult to track back this
> error). So I don't think doing nothing is an option here. If we're not going
> to add this assert, we should initialize the uninitialized data in such a
> way that we are guaranteed to detect the error. The scheme I proposed above
> would take care of that. Should I implement that instead?

No, instead the check below should catch the error much earlier.

>> Likewise the verify_dominators call is too expensive and misplaced.
>>
>> If then the call belongs in the dom_computed[] == DOM_OK early-out
>> in calculate_dominance_info
>
>
> OK, like this:
> ...
> diff --git a/gcc/dominance.c b/gcc/dominance.c
> index a9e042e..1827eda9 100644
> --- a/gcc/dominance.c
> +++ b/gcc/dominance.c
> @@ -646,7 +646,12 @@ calculate_dominance_info (enum cdi_direction dir)
>    bool reverse = (dir == CDI_POST_DOMINATORS) ? true : false;
>
>    if (dom_computed[dir_index] == DOM_OK)
> -    return;
> +    {
> +#if ENABLE_CHECKING
> +      verify_dominators (CDI_DOMINATORS);
> +#endif
> +      return;
> +    }
>
>    timevar_push (TV_DOMINANCE);
>    if (!dom_info_available_p (dir))
> ...

Yes.

> I didn't fully understand your comment, do you want me to test this?

Sure, it should catch the error.

Richard.

> Thanks,
> - Tom
>
>
>> (eventually also for the case where we
>> end up only computing the fast-query stuff).
>
>

Re: [PATCH] Check dominator info in compute_dominance_frontiers

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 4195 bytes --]

On 22/06/15 13:47, Richard Biener wrote:
> On Mon, Jun 22, 2015 at 1:33 PM, Tom de Vries <Tom_deVries@mentor.com> wrote:
>> On 22/06/15 12:14, Richard Biener wrote:
>>>
>>> On Mon, Jun 22, 2015 at 10:04 AM, Tom de Vries <Tom_deVries@mentor.com>
>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> during development of a patch I ran into a case where
>>>> compute_dominance_frontiers was called with incorrect dominance info.
>>>>
>>>> The result was a segmentation violation somewhere in the bitmap code
>>>> while
>>>> executing this bitmap_set_bit in compute_dominance_frontiers_1:
>>>> ...
>>>>                     if (!bitmap_set_bit (&frontiers[runner->index],
>>>>                                          b->index))
>>>>                       break;
>>>> ...
>>>>
>>>> The segmentation violation happens because runner->index is 0, and
>>>> frontiers[0] is uninitialized.
>>>>
>>>> [ The initialization in update_ssa looks like this:
>>>> ...
>>>>        dfs = XNEWVEC (bitmap_head, last_basic_block_for_fn (cfun));
>>>>         FOR_EACH_BB_FN (bb, cfun)
>>>>           bitmap_initialize (&dfs[bb->index], &bitmap_default_obstack);
>>>>         compute_dominance_frontiers (dfs);
>>>> ...
>>>>
>>>> FOR_EACH_BB_FN skips over the entry-block and the exit-block, so dfs[0]
>>>> (frontiers[0] in compute_dominance_frontiers_1) is not initialized.
>>>>
>>>> We could add initialization by making the entry/exit-block bitmap_heads
>>>> empty and setting the obstack to a reserved obstack bitmap_no_obstack for
>>>> which allocation results in an assert. ]
>>>>
>>>> AFAIU, the immediate problem is not that frontiers[0] is uninitialized,
>>>> but
>>>> that the loop reaches the state of runner->index == 0, due to the
>>>> incorrect
>>>> dominance info.
>>>>
>>>> The patch adds an assert to the loop in compute_dominance_frontiers_1, to
>>>> make the failure mode cleaner and easier to understand.
>>>>
>>>> I think we wouldn't catch all errors in dominance info with this assert.
>>>> So
>>>> the patch also contains an ENABLE_CHECKING-enabled verify_dominators call
>>>> at
>>>> the start of compute_dominance_frontiers. I'm not sure if:
>>>> - adding the verify_dominators call is too costly in runtime.
>>>> - the verify_dominators call should be inside or outside the
>>>>     TV_DOM_FRONTIERS measurement.
>>>> - there is a level of ENABLE_CHECKING that is more appropriate for the
>>>>     verify_dominators call.
>>>>
>>>> Is this ok for trunk if bootstrap and reg-test on x86_64 succeeds?
>>>
>>>
>>> I don't think these kind of asserts are good.  A segfault is good by
>>> itself
>>> (so you can just add the comment if you like).
>>>
>>
>> The segfault is not guaranteed to trigger, because it works on uninitialized
>> data. Instead, we may end up modifying valid memory and silently generating
>> wrong code or causing sigsegvs (which will be difficult to track back this
>> error). So I don't think doing nothing is an option here. If we're not going
>> to add this assert, we should initialize the uninitialized data in such a
>> way that we are guaranteed to detect the error. The scheme I proposed above
>> would take care of that. Should I implement that instead?
>
> No, instead the check below should catch the error much earlier.
>
>>> Likewise the verify_dominators call is too expensive and misplaced.
>>>
>>> If then the call belongs in the dom_computed[] == DOM_OK early-out
>>> in calculate_dominance_info
>>
>>
>> OK, like this:
>> ...
>> diff --git a/gcc/dominance.c b/gcc/dominance.c
>> index a9e042e..1827eda9 100644
>> --- a/gcc/dominance.c
>> +++ b/gcc/dominance.c
>> @@ -646,7 +646,12 @@ calculate_dominance_info (enum cdi_direction dir)
>>     bool reverse = (dir == CDI_POST_DOMINATORS) ? true : false;
>>
>>     if (dom_computed[dir_index] == DOM_OK)
>> -    return;
>> +    {
>> +#if ENABLE_CHECKING
>> +      verify_dominators (CDI_DOMINATORS);
>> +#endif
>> +      return;
>> +    }
>>
>>     timevar_push (TV_DOMINANCE);
>>     if (!dom_info_available_p (dir))
>> ...
>
> Yes.
>
>> I didn't fully understand your comment, do you want me to test this?
>
> Sure, it should catch the error.
>

Bootstrapped and reg-tested on x86_64. Committed as attached.

Thanks,
- Tom

[-- Attachment #2: 0005-Verify-dominators-in-early-out-calculate_dominance_i.patch --]
[-- Type: text/x-patch, Size: 759 bytes --]

Verify dominators in early-out calculate_dominance_info

2015-06-22  Tom de Vries  <tom@codesourcery.com>

	* dominance.c (calculate_dominance_info): Verify dominators if
	early-out.
---
 gcc/dominance.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gcc/dominance.c b/gcc/dominance.c
index a9e042e..9c66ca2 100644
--- a/gcc/dominance.c
+++ b/gcc/dominance.c
@@ -646,7 +646,12 @@ calculate_dominance_info (enum cdi_direction dir)
   bool reverse = (dir == CDI_POST_DOMINATORS) ? true : false;
 
   if (dom_computed[dir_index] == DOM_OK)
-    return;
+    {
+#if ENABLE_CHECKING
+      verify_dominators (CDI_DOMINATORS);
+#endif
+      return;
+    }
 
   timevar_push (TV_DOMINANCE);
   if (!dom_info_available_p (dir))
-- 
1.9.1

Re: [PATCH] Check dominator info in compute_dominance_frontiers

[Tom de Vries]

On 22/06/15 13:47, Richard Biener wrote:
>> (eventually also for the case where we
>> >>end up only computing the fast-query stuff).
>>

Like this?
...
diff --git a/gcc/dominance.c b/gcc/dominance.c
index 9c66ca2..58fc6fd 100644
--- a/gcc/dominance.c
+++ b/gcc/dominance.c
@@ -679,6 +679,12 @@ calculate_dominance_info (enum cdi_direction dir)
        free_dom_info (&di);
        dom_computed[dir_index] = DOM_NO_FAST_QUERY;
      }
+  else
+    {
+#if ENABLE_CHECKING
+      verify_dominators (CDI_DOMINATORS);
+#endif
+    }

    compute_dom_fast_query (dir);

...

Thanks,
- Tom

Re: [PATCH] Check dominator info in compute_dominance_frontiers

[Richard Biener]

On Mon, Jun 22, 2015 at 7:10 PM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 22/06/15 13:47, Richard Biener wrote:
>>>
>>> (eventually also for the case where we
>>> >>end up only computing the fast-query stuff).
>>>
>
> Like this?
> ...
> diff --git a/gcc/dominance.c b/gcc/dominance.c
> index 9c66ca2..58fc6fd 100644
> --- a/gcc/dominance.c
> +++ b/gcc/dominance.c
> @@ -679,6 +679,12 @@ calculate_dominance_info (enum cdi_direction dir)
>        free_dom_info (&di);
>        dom_computed[dir_index] = DOM_NO_FAST_QUERY;
>      }
> +  else
> +    {
> +#if ENABLE_CHECKING
> +      verify_dominators (CDI_DOMINATORS);
> +#endif
> +    }
>
>    compute_dom_fast_query (dir);

Yeah.

Richard.

> ...
>
> Thanks,
> - Tom

Re: [PATCH] Check dominator info in compute_dominance_frontiers

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 976 bytes --]

On 23/06/15 11:31, Richard Biener wrote:
> On Mon, Jun 22, 2015 at 7:10 PM, Tom de Vries <Tom_deVries@mentor.com> wrote:
>> On 22/06/15 13:47, Richard Biener wrote:
>>>>
>>>> (eventually also for the case where we
>>>>>> end up only computing the fast-query stuff).
>>>>
>>
>> Like this?
>> ...
>> diff --git a/gcc/dominance.c b/gcc/dominance.c
>> index 9c66ca2..58fc6fd 100644
>> --- a/gcc/dominance.c
>> +++ b/gcc/dominance.c
>> @@ -679,6 +679,12 @@ calculate_dominance_info (enum cdi_direction dir)
>>         free_dom_info (&di);
>>         dom_computed[dir_index] = DOM_NO_FAST_QUERY;
>>       }
>> +  else
>> +    {
>> +#if ENABLE_CHECKING
>> +      verify_dominators (CDI_DOMINATORS);
>> +#endif
>> +    }
>>
>>     compute_dom_fast_query (dir);
>
> Yeah.
>

I realized we actually want to verify 'dir' rather than 
'CDI_DOMINATORS'. The patch also fixes this for the early-out verification.

Bootstrapped and reg-tested on x86_64.

Committed to trunk.

Thanks,
- Tom


[-- Attachment #2: 0001-Verify-reused-dominators-info-in-calculate_dominance.patch --]
[-- Type: text/x-patch, Size: 980 bytes --]

Verify reused dominators info in calculate_dominance_info

2015-06-24  Tom de Vries  <tom@codesourcery.com>

	* dominance.c (calculate_dominance_info): Fix verify_dominators call
	argument.  Call verify_dominator when reusing dominator info.
---
 gcc/dominance.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gcc/dominance.c b/gcc/dominance.c
index 9c66ca2..fb61596 100644
--- a/gcc/dominance.c
+++ b/gcc/dominance.c
@@ -648,7 +648,7 @@ calculate_dominance_info (enum cdi_direction dir)
   if (dom_computed[dir_index] == DOM_OK)
     {
 #if ENABLE_CHECKING
-      verify_dominators (CDI_DOMINATORS);
+      verify_dominators (dir);
 #endif
       return;
     }
@@ -679,6 +679,12 @@ calculate_dominance_info (enum cdi_direction dir)
       free_dom_info (&di);
       dom_computed[dir_index] = DOM_NO_FAST_QUERY;
     }
+  else
+    {
+#if ENABLE_CHECKING
+      verify_dominators (dir);
+#endif
+    }
 
   compute_dom_fast_query (dir);
 
-- 
1.9.1