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

Joel Brobecker
2020-11-24T02:39:42Z

> > 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.

My pleasure. I was relieved to find that the main issue was in
the unittest itself! There's still the question of protecting
ourselves from this kind of issue, just in case, but at least
there we know what to be careful of.

I just pushed the patch to master, and will work on catching
those issues over the weekend.

gdb/ChangeLog:

* unittests/gmp-utils-selftests.c (write_fp_test): Use mpq_set_si
instead of mpq_set_ui to initialize our GMP rational.

--
Joel