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

On Thu, Aug 13, 2026 at 1:51 PM Jeffrey Law
<[email protected]> wrote:
>
>
>
> 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.

if there is no call to promote_ssa_mode, then the 2 modes won't be
different and unsignedp won't be used.
So it does the solve issue there too.

>
>
>
> jeff