Richard Biener <richard.guent...@gmail.com> writes:
> On Mon, May 5, 2014 at 4:51 PM, Richard Sandiford
> <rdsandif...@googlemail.com> wrote:
>> Richard Biener <richard.guent...@gmail.com> writes:
>>> On Mon, May 5, 2014 at 12:54 PM, Richard Sandiford
>>> <rdsandif...@googlemail.com> wrote:
>>>> Richard Biener <richard.guent...@gmail.com> writes:
>>>>> On Fri, May 2, 2014 at 9:19 PM, Richard Sandiford
>>>>> <rdsandif...@googlemail.com> wrote:
>>>>>> I'd hoped the days of zero-precision INTEGER_CSTs were behind us after
>>>>>> Richard's patch to remove min amd max values from zero-width bitfields,
>>>>>> but a boostrap-ubsan showed otherwise. One source is in:
>>>>>>
>>>>>> null_pointer_node = build_int_cst (build_pointer_type
>>>>>> (void_type_node), 0);
>>>>>>
>>>>>> if no_target, since the precision of the type comes from ptr_mode,
>>>>>> which remains the default VOIDmode. Maybe that's a bug, but setting
>>>>>> it to an arbitrary nonzero value would also be dangerous.
>>>>>
>>>>> Can you explain what 'no_target' should be? ptr_mode should never be
>>>>> VOIDmode.
>>>>
>>>> Sorry, meant "no_backend" rather than "no_target". See do_compile.
>>>
>>> Ok. So we do
>>>
>>> /* This must be run always, because it is needed to compute the FP
>>> predefined macros, such as __LDBL_MAX__, for targets using non
>>> default FP formats. */
>>> init_adjust_machine_modes ();
>>>
>>> /* Set up the back-end if requested. */
>>> if (!no_backend)
>>> backend_init ();
>>>
>>> where I think that init_adjust_machine_modes should initialize the
>>> {byte,word,ptr}_mode globals. Move from init_emit_once.
init_adjust_machine_modes is an auto-generated function so I ended up
using a new function instead. Tested on x86_64-linux-gnu. OK to install?
>>>>> So I don't think we want this patch. Instead stor-layout should
>>>>> ICE on zero-precision integer/pointer types.
>>>>
>>>> What should happen for void_zero_node?
>>>
>>> Not sure what that beast is supposed to be or why it should be
>>> of INTEGER_CST kind (it's not even initialized in any meaningful
>>> way).
>>>
>>> That said, the wide-int code shouldn't be slowed down by
>>> precision == 0 checks. We should never ever reach wide-int
>>> with such a constant.
>>
>> void_zero_node is used for ubsan too, and survives into gimple.
>> I did hit this in real tests, it wasn't just theoretical.
>
> Ugh - for what does it use that ... :/
>
> Please remember how to trigger those issues and I'll happily have
> a look after the merge.
At the time it was just a normal bootstrap-ubsan, but that was
before the zero-precision patch. Probably the best way of
checking for zero-precision tree constants is to put an assert
for nonzero precisions in:
inline unsigned int
wi::int_traits <const_tree>::get_precision (const_tree tcst)
{
return TYPE_PRECISION (TREE_TYPE (tcst));
}
and:
template <int N>
inline wi::extended_tree <N>::extended_tree (const_tree t)
: m_t (t)
{
gcc_checking_assert (TYPE_PRECISION (TREE_TYPE (t)) <= N);
}
and then bootstrap-ubsan. But like Marek says, the ubsan code uses
void_zero_node by name.
Thanks,
Richard
gcc/
* emit-rtl.c (init_derived_machine_modes): New functionm, split
out from...
(init_emit_once): ...here.
* rtl.h (init_derived_machine_modes): Declare.
* toplev.c (do_compile): Call it even if no_backend.
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c 2014-05-03 20:18:49.157107743 +0100
+++ gcc/emit-rtl.c 2014-05-05 17:44:53.579038259 +0100
@@ -5620,6 +5620,30 @@ init_emit_regs (void)
}
}
+/* Initialize global machine_mode variables. */
+
+void
+init_derived_machine_modes (void)
+{
+ byte_mode = VOIDmode;
+ word_mode = VOIDmode;
+
+ for (enum machine_mode mode = GET_CLASS_NARROWEST_MODE (MODE_INT);
+ mode != VOIDmode;
+ mode = GET_MODE_WIDER_MODE (mode))
+ {
+ if (GET_MODE_BITSIZE (mode) == BITS_PER_UNIT
+ && byte_mode == VOIDmode)
+ byte_mode = mode;
+
+ if (GET_MODE_BITSIZE (mode) == BITS_PER_WORD
+ && word_mode == VOIDmode)
+ word_mode = mode;
+ }
+
+ ptr_mode = mode_for_size (POINTER_SIZE, GET_MODE_CLASS (Pmode), 0);
+}
+
/* Create some permanent unique rtl objects shared between all functions. */
void
@@ -5643,36 +5667,6 @@ init_emit_once (void)
reg_attrs_htab = htab_create_ggc (37, reg_attrs_htab_hash,
reg_attrs_htab_eq, NULL);
- /* Compute the word and byte modes. */
-
- byte_mode = VOIDmode;
- word_mode = VOIDmode;
- double_mode = VOIDmode;
-
- for (mode = GET_CLASS_NARROWEST_MODE (MODE_INT);
- mode != VOIDmode;
- mode = GET_MODE_WIDER_MODE (mode))
- {
- if (GET_MODE_BITSIZE (mode) == BITS_PER_UNIT
- && byte_mode == VOIDmode)
- byte_mode = mode;
-
- if (GET_MODE_BITSIZE (mode) == BITS_PER_WORD
- && word_mode == VOIDmode)
- word_mode = mode;
- }
-
- for (mode = GET_CLASS_NARROWEST_MODE (MODE_FLOAT);
- mode != VOIDmode;
- mode = GET_MODE_WIDER_MODE (mode))
- {
- if (GET_MODE_BITSIZE (mode) == DOUBLE_TYPE_SIZE
- && double_mode == VOIDmode)
- double_mode = mode;
- }
-
- ptr_mode = mode_for_size (POINTER_SIZE, GET_MODE_CLASS (Pmode), 0);
-
#ifdef INIT_EXPANDERS
/* This is to initialize {init|mark|free}_machine_status before the first
call to push_function_context_to. This is needed by the Chill front
@@ -5695,6 +5689,8 @@ init_emit_once (void)
else
const_true_rtx = gen_rtx_CONST_INT (VOIDmode, STORE_FLAG_VALUE);
+ double_mode = mode_for_size (DOUBLE_TYPE_SIZE, MODE_FLOAT, 0);
+
REAL_VALUE_FROM_INT (dconst0, 0, 0, double_mode);
REAL_VALUE_FROM_INT (dconst1, 1, 0, double_mode);
REAL_VALUE_FROM_INT (dconst2, 2, 0, double_mode);
Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h 2014-05-03 20:18:49.157107743 +0100
+++ gcc/rtl.h 2014-05-05 17:40:26.035785011 +0100
@@ -2545,6 +2545,7 @@ extern int get_max_insn_count (void);
extern int in_sequence_p (void);
extern void init_emit (void);
extern void init_emit_regs (void);
+extern void init_derived_machine_modes (void);
extern void init_emit_once (void);
extern void push_topmost_sequence (void);
extern void pop_topmost_sequence (void);
Index: gcc/toplev.c
===================================================================
--- gcc/toplev.c 2014-04-26 16:05:43.076775028 +0100
+++ gcc/toplev.c 2014-05-05 17:41:00.168079259 +0100
@@ -1891,6 +1891,7 @@ do_compile (void)
predefined macros, such as __LDBL_MAX__, for targets using non
default FP formats. */
init_adjust_machine_modes ();
+ init_derived_machine_modes ();
/* Set up the back-end if requested. */
if (!no_backend)
