> On Nov. 15, 2016, 3:16 p.m., Andreas Sandberg wrote: > > Thanks for fixing this. The only technical issue I have with this patch is > > that it uses bit fields, which have slightly undefined semantics. I would > > like to avoid that if possible. I think it would be fine to just add a > > uitn8_t array instead after se. > > > > A non-technical issue is that we should make sure this is included in the > > upstream library and then pulled into ext. Could you post a pull request to > > https://github.com/andysan/libfputils ? > > Tony Gutierrez wrote: > Sure. Are you asking me to not push this into gem5 though?
I would prefer if we could push it to the external library first and then pull in that revision into ext. This makes it easier to track the upstream revision in the change log. - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3690/#review9072 ----------------------------------------------------------- On Nov. 15, 2016, 5:18 p.m., Tony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3690/ > ----------------------------------------------------------- > > (Updated Nov. 15, 2016, 5:18 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11894:c211fc37d7a1 > --------------------------- > x86, ext: fix buf overflow in fp80 ops; pad fp80_t in fputils > > the compiler seems to align the fp80_t data struct, so here we add > explicit padding to avoid confusion. > > storeFloat80() will try to write all 16B of the fp80_t to the bits[] array > of the calling instruction. this happens because storeFloat80() points its > local fp80_t* to the memory the caller allocated for bits[], which is only > 10B, thus we get an overflow that is flagged by clang's asan. here we > get the fp80 value first, the memcpy() the bits[] of fp80_t to the mem > allocated by the caller. > > some of the x86 FP ops also use char to represent 8b types, while the fp80 > struct uses uint8_t, so here we make the x86 ops use uint8_t as well. > > > Diffs > ----- > > src/arch/x86/utility.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > ext/fputils/fpbits.h c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > ext/fputils/include/fputils/fptypes.h > c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/arch/x86/isa/microops/fpop.isa c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > > Diff: http://reviews.gem5.org/r/3690/diff/ > > > Testing > ------- > > > Thanks, > > Tony Gutierrez > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
