[Bug c++/94939] New: [9.2.1] invalid code generation in ternary op with static class member (undefined behaviour nearby?)

[Bug c++/94939] New: [9.2.1] invalid code generation in ternary op with static class member (undefined behaviour nearby?)

[tobias.pankrath@ssw-trading.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94939

            Bug ID: 94939
           Summary: [9.2.1] invalid code generation in ternary op with
                    static class member (undefined behaviour nearby?)
           Product: gcc
           Version: 9.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tobias.pankrath@ssw-trading.com
  Target Milestone: ---

I've got the following classes, which are using the type_safe library
(https://github.com/foonathan/type_safe).

struct Old
{
    inline constexpr Old();
    constexpr Old(std::int64_t v)
        : storage(v)
    {}
    std::int64_t storage = -1;
    static const Old magic;
};

constexpr Old::Old() { storage = 0; }

constexpr Old Old::magic = Old(12);

struct OldDerived : Old
{
    constexpr OldDerived(Old old)
        : Old(old)
    {}
    constexpr OldDerived(std::int64_t v)
        : Old(v)
    {}
    auto get() { return storage; }
    static const OldDerived magic;
};

constexpr OldDerived OldDerived::magic = OldDerived(Old::magic);

struct New : public Old
{
    New() = default;
};

template<typename Strongtypedef, typename OldType_>
struct conversions
{
    using OldType = OldType_;
    const OldType& toOld() const
    {
        return static_cast<const OldType&>(
            static_cast<const Old&>(type_safe::get(static_cast<const
Strongtypedef&>(*this))));
    }
};

template<typename ST, typename OT_>
struct one_more_type : conversions<ST, OT_>
{};

struct NewDerived
    : type_safe::strong_typedef<NewDerived, New>
    , one_more_type<NewDerived, OldDerived>
{
    NewDerived() = default;
    NewDerived(std::int64_t value)
        : strong_typedef(New{{value}})
    {}
};

As a colleague already pointed out, conversions::toOld() might have
an illegal cast in it and maybe that is all to this. What I observe is the
following code prints 'VALUE IS 0' (in production: VALUE IS <GARBAGE>).

OldDerived
callee(bool b)
{
    NewDerived a = NewDerived{11};
    return b ? a.toOld() : OldDerived::magic;
}

__attribute_noinline__ void
caller_magic(bool b)
{
    OldDerived value = callee(b);
    std::cerr << "VALUE IS " << value.storage << std::endl;
}

void main()
{
    caller_magic(true);
}

And if I do not use OldDerived::magic, the following code prints correctly
'VALUE IS 11'.

OldDerived
callee_lit(bool b)
{
    NewDerived a = NewDerived{11};
    return b ? a.toOld() : OldDerived(12);
}

__attribute_noinline__ void
caller_lit(bool b)
{
    OldDerived value = callee_lit(b);
    std::cerr << "VALUE IS " << value.storage << std::endl;
}

void main()
{
    caller_lit(true);
}


Here is the code, including the parts from type_safe) inside the compiler
explorer: https://godbolt.org/z/TGB8Jg


Is it enough to fix the cast to make sure the code does what one expects?

[Bug c++/94939] [9.2.1] invalid code generation in ternary op with static class member (undefined behaviour nearby?)

[tobias.pankrath@ssw-trading.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94939

Tobias Pankrath <tobias.pankrath@ssw-trading.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |x86_64
               Host|                            |Ubuntu 19.10
                URL|                            |https://godbolt.org/z/TGB8J
                   |                            |g

--- Comment #1 from Tobias Pankrath <tobias.pankrath@ssw-trading.com> ---
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/9/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:hsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 9.2.1-9ubuntu2'
--with-bugurl=file:///usr/share/doc/gcc-9/README.Bugs
--enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++,gm2 --prefix=/usr
--with-gcc-major-version-only --program-suffix=-9
--program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id
--libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
--libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object
--disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib
--with-target-system-zlib=auto --enable-multiarch --disable-werror
--with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32
--enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none,hsa
--without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 9.2.1 20191008 (Ubuntu 9.2.1-9ubuntu2)

[Bug c++/94939] [9.2.1] invalid code generation in ternary op with static class member (undefined behaviour nearby?)

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94939

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-05-04

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
I would guess the type_safe libraries doing for
type_safe::get(static_cast<const Strongtypedef&>(*this)) is "wrong" (whatever
type_safe is guaranteeing).

Please provide a complete preprocessed testcase that can be compiled.

[Bug c++/94939] [9.2.1] invalid code generation in ternary op with static class member (undefined behaviour nearby?)

[tobias.pankrath@ssw-trading.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94939

--- Comment #3 from Tobias Pankrath <tobias.pankrath@ssw-trading.com> ---
That was quick :-) Thank you for looking into it.

Is this enough? I pasted all type_safe code that is used for the example
directly in there: https://godbolt.org/z/TGB8Jg

[Bug c++/94939] [9.2.1] invalid code generation in ternary op with static class member (undefined behaviour nearby?)

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94939

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
t567.cc: In function ‘OldDerived callee(bool)’:
t567.cc:187:14: warning:
‘a.NewDerived::<anonymous>.type_safe::strong_typedef<NewDerived,
New>::value_.New::<anonymous>.Old::storage’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
     return b ? a.toOld() : OldDerived::magic;
            ~~^~~~~~~~~~~~~~~~~~~~~~~~
t567.cc: In function ‘void caller_magic(bool)’:
t567.cc:187:14: warning: ‘a’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
     return b ? a.toOld() : OldDerived::magic;
            ~~^~~~~~~~~~~~~~~~~~~~~~~~


  MEM[(struct  &)&a] ={v} {CLOBBER};
  MEM[(struct strong_typedef *)&a] = 11;
  if (b_5(D) != 0)
    goto <bb 3>; [0.00%]
  else
    goto <bb 4>; [0.00%]

  <bb 3> [100.00%]:
  _7 = &MEM[(const struct New *)&a].D.43195;
  _13 = _7;
  iftmp.0_8 = _13;
  goto <bb 5>; [0.00%]

  <bb 4> [0.00%]:
  iftmp.0_6 = &magic;

  <bb 5> [0.00%]:
  # iftmp.0_1 = PHI <iftmp.0_8(3), iftmp.0_6(4)>
  D.48802 = *iftmp.0_1;
  a ={v} {CLOBBER};


Yes I think there is some aliasing issues in fact using -fno-strict-aliasing
"fixes" the warning and even gives what you expect.


And yes the code is undefined and violates aliasing.

NewDerived has no field or inheritance from OldDerived so mixing those together
like you with the casts is what causes the aliasing violations.

[Bug c++/94939] [9.2.1] invalid code generation in ternary op with static class member (undefined behaviour nearby?)

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94939

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |INVALID

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Invalid as this is aliasing violation.