By the way, if that fixes things for you please upload a patch. Be sure to add Giacomo Travaglini as a reviewer since I think he wrote the function in question. Looking at the CL that introduced parseParam, I see that the old code did things more or less like what I'm suggesting, and it would be good to get some insight into why it was changed.
Gabe On Mon, Jan 14, 2019 at 4:45 PM Gabe Black <gabebl...@google.com> wrote: > That parseParam function looks odd. It's updating the "value" variable > later in the function, but for some reason is using it to initialize > "storage" also. "storage" is probably overwritten in to_number (except > maybe if there's an error?), but g++ is correct that, at least temporarily, > the uninitialized value in "value" (regardless of the fact that it's a > BitUnion) is being assigned to storage. > > I suspect the idea was to be able to use "auto" to declare storage, but I > think you should just be able to use BitUnionBaseType<T> as the type and > not look at the value of "value". Or in other words: > > auto storage = static_cast<BitUnionBaseType<T>>(value); > > would become: > > BitUnionBaseType<T> storage; > > I *think* there are also special default initializers that even work with, > say, int types which would let you initialize storage with a known (but > worthless) value so that an uninitialized value doesn't leak from to_number > even if there's an error. I think the syntax for that is just: > > BitUnionBaseType<T> storage(); > > Gabe > > On Mon, Jan 14, 2019 at 10:59 AM Daniel Carvalho <oda...@yahoo.com.br> > wrote: > >> Hello all, >> Compiling a fresh version of Gem5 (master, >> ad9a233ff9fdaadbbdebef32af242c8e3c05bfd4) generates the following >> compilation error when doing a cross compilation from Ubuntu x86 to Debian >> x86 (Jessie) (tl;dr variable "__storage" is not initialized before used): >> >> schroot -c jessie -- scons -j8 build/ARM/gem5.opt >> >> In file included from build/ARM/sim/eventq.hh:53:0, >> from build/ARM/sim/sim_object.hh:57, >> from build/ARM/cpu/intr_control.hh:39, >> from build/ARM/dev/arm/gic_v2.hh:55, >> from build/ARM/dev/arm/gic_v2.cc:44: >> build/ARM/sim/serialize.hh: In function 'void arrayParamIn(CheckpointIn&, >> const string&, T*, unsigned int) [with T = >> BitfieldBackend::BitUnionOperators<GicV2::BitfieldUnderlyingClassesCTLR>; >> std::string = std::basic_string<char>]': >> build/ARM/sim/serialize.hh:292:58: error: >> 'scalar_value.BitfieldBackend::BitUnionOperators<GicV2::BitfieldUnderlyingClassesCTLR>::<anonymous>.GicV2::BitfieldUnderlyingClassesCTLR::<anonymous>.GicV2::BitfieldUnderlyingClassesCTLR::<anonymous >> union>::__storage' may be used uninitialized in this function >> [-Werror=maybe-uninitialized] >> auto storage = static_cast<BitUnionBaseType<T>>(value); >> ^ >> build/ARM/sim/serialize.hh:507:11: note: >> 'scalar_value.BitfieldBackend::BitUnionOperators<GicV2::BitfieldUnderlyingClassesCTLR>::<anonymous>.GicV2::BitfieldUnderlyingClassesCTLR::<anonymous>.GicV2::BitfieldUnderlyingClassesCTLR::<anonymous >> union>::__storage' was declared here >> T scalar_value; >> >> >> schroot's "g++ -v": >> >> COLLECT_GCC=g++ >> COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper >> Target: x86_64-linux-gnu >> Configured with: ../src/configure -v --with-pkgversion='Debian >> 4.9.2-10+deb8u1' --with-bugurl=file:///usr/share/doc/gcc-4.9/README.Bugs >> --enable-languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr >> --program-suffix=-4.9 --enable-shared --enable-linker-build-id >> --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix >> --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib --enable-nls >> --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug >> --enable-libstdcxx-time=yes --enable-gnu-unique-object >> --disable-vtable-verify --enable-plugin --with-system-zlib >> --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo >> --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64/jre >> --enable-java-home >> --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64 >> --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.9-amd64 >> --with-arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar >> --enable-objc-gc --enable-multiarch --with-arch-32=i586 --with-abi=m64 >> --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic >> --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu >> --target=x86_64-linux-gnu >> Thread model: posix >> gcc version 4.9.2 (Debian 4.9.2-10+deb8u1) >> >> >> I tried fixing within the desirable file (i.e., bitunion.hh), but the >> code there is confusing, so I couldn't make it work there. As I don't want >> to add a dirty/broken fix (i.e., add a call to value.setter in parseParam >> before the static cast; it seems to work, but I don't want to risk it since >> I don't know if the param can be of BitfieldROType at that point), I would >> like to ask those who are more familiar with this part of the code to take >> a look at it. >> Regards,Daniel >> _______________________________________________ >> gem5-dev mailing list >> gem5-dev@gem5.org >> http://m5sim.org/mailman/listinfo/gem5-dev > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
