Re: pushed: Add support for DWARF-based fixed point types

Simon Marchi
2020-11-23T16:12:30Z

On 2020-11-22 11:27 p.m., Joel Brobecker wrote:
>> We pass mpz_export a buffer of 8 bytes (statically allocated in
>> write_fp_test), but GMP decides it needs to write 16 bytes, hence the
>> overflow.
>>
>> I tried to read the GMP doc, but I am familiar with its concepts, so I
>> don't really understand if we are using the API correctly or not.
>
> I found the source of the problem, which was in a way subtle-enough
> that you really have to pay attention to these details (which,
> luckily, are handled automatically thanks to our minor C++-ification
> of GMP in gmp-utils), and yet so obvious once you find it.
> Attached is the patch that I will push later today (need to run RSN,
> and don't want to make a mistake because I'm rushing).
>
> I think this error might be highlighting a weakness, though. I need
> to investigate more, but I'm thinking it might be wise to add some
> checks during export that the buffer size is large enough to fit
> the value. In other words, I'm thinking of having our own
> safe_mpz_export which double-checks the size of the buffer according
> to the formula given by the documentation, and raises an error if
> too small.
>
> The fact that GMP happily goes beyond the end of the buffer is
> a bit unexpected, still. Maybe something to report to the GMP team.

Ah, nice! The explanation in the commit message makes sense, thanks.

And yes, please go wild adding assertions!

Simon