Re: [PATCH] tree-outof-ssa: fix unsignedp uninitialized in insert_value_copy_on_edge

On 8/13/2026 2:45 PM, Andrea Pinski wrote:
> On Thu, Aug 13, 2026 at 1:39 PM Jeffrey Law
> <[email protected]> wrote:
>>
>>
>> On 8/12/2026 10:27 PM, Sam Price wrote:
>>> insert_value_copy_on_edge declares 'unsignedp' uninitialized and sets it
>>> only as a side effect of an argument to gcc_assert.  When src_mode and
>>> dest_mode differ it is then passed to convert_modes, which uses it to
>>> choose between zero and sign extension.
>>>
>>> gcc_assert does not always evaluate its argument.  With GCC_VERSION
>>> < 4005 and without assert checking, gcc/system.h defines
>>>
>>>     #define gcc_assert(EXPR) ((void)(0 && (EXPR)))
>>>
>>> so promote_ssa_mode is never called and unsignedp is passed to
>>> convert_modes indeterminate, selecting zero or sign extension at random.
>>> GCC_VERSION is __GNUC__ * 1000 + __GNUC_MINOR__, so any host compiler
>>> reporting less than 4.5 takes this branch.
>>>
>>> Relying on a side effect in gcc_assert is against the coding
>>> conventions regardless of which expansion is in use.  Call
>>> promote_ssa_mode outside the assert, as the same file already does in
>>> get_temp_reg, and keep the assert for the invariant alone.  unsignedp is
>>> initialized for the !REG_P path, where promote_ssa_mode is not called.
>>>
>>> Signed-off-by: Sam Price<[email protected]>
>>> Assisted-by: Claude (Anthropic)
>>> ---
>>>
>>> Found while getting gcc.c-torture running against a MicroBlaze port to
>>> LLVM, comparing the two compilers over the suite.
>> So I don't think is necessarily as simple as it might appear.  In
>> particular what is the right value for the !REG case.    I would suggest
>> one of two paths to make sure we're safe WRT the unsignedp initialization.
>>
>> First, make the call to promote_ssa_mode unconditionally.  The worry
>> here is that it may not be ready to handle non-REG cases.
>>
>> Second, in the src_mode != dest_mode code, assert REG_P (dest).
>>
>> Given the change affects generic code it would need to be bootstrapped
>> and regression tested on one of the primary targets (x86, aarch64 linux
>> being the most common).
> So the problem will only show up with --disable-checking and a non GCC build.
> Since GCC before 5.4.0 can't build GCC at this point. So maybe the
> best way forward is either removing assert checking or just changing
> the non-GCC gcc_assert case into the assert checking case.
> I filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=126838 and
> Richard B. sugguests we remove --disable-checking. I tend to agree
> with him.
I could live with that as well.   But I don't think that resolves the 
question of what value to use for unsignedp unless you pair the removal 
with making the call to promote_ssa_mode unconditional.



jeff