> On Nov. 15, 2016, 7:16 a.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 ?

Sure. Are you asking me to not push this into gem5 though?


- Tony


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3690/#review9072
-----------------------------------------------------------


On Nov. 1, 2016, 12:36 p.m., Tony Gutierrez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3690/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2016, 12:36 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 11888:ed39f10e4ff3
> ---------------------------
> 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.
> 
> 
> Diffs
> -----
> 
>   ext/fputils/fpbits.h c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   ext/fputils/include/fputils/fptypes.h 
> c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/arch/x86/isa/microops/fpop.isa c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/arch/x86/utility.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
> 
> Diff: http://reviews.gem5.org/r/3690/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Tony Gutierrez
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to