Change type of newprec from uint8_t to int

[randy3k]

Our production environment compiles packages with a bunch of sanitizers. And we got the following error while running the ggplot2 4.0 tests.

.../svglite/src/SvgStream.h:21:21: runtime error: 307 is outside the range of representable values of type 'unsigned char'
    #0 0x7b6786a41fb2 in write_double<std::__u::basic_ofstream<char, std::__u::char_traits<char> > > .../svglite/src/SvgStream.h:21:21
    #1 0x7b6786a41fb2 in SvgStreamFile::write(double) .../svglite/src/SvgStream.h:110:37

SUMMARY: UndefinedBehaviorSanitizer: float-cast-overflow ../svglite/src/SvgStream.h:21:21  

Gemini suggested this change. And here is the Gemini output.

Okay, I see the issue. The runtime error float-cast-overflow is happening in the write_double function in SvgStream.h.

Specifically, this line is the problem:

uint8_t newprec = std::fabs(data) >= 1 || data == 0. ? prec : std::ceil(-std::log10(std::fabs(data))) + 1;

When the absolute value of data is very small (e.g., close to zero, like 1e-300), -std::log10(std::fabs(data)) becomes a large positive number. Adding 1 to it can result in a value greater than 255. Since newprec is declared as uint8_t, this large value overflows the representable range of uint8_t, causing the float-cast-overflow error.

The fix is to change the type of newprec to something that can hold larger values, such as int. The std::setprecision function accepts an int argument, so this change is safe.

I changed the type of newprec from uint8_t to int to prevent the overflow when calculating the precision for very small double values

[MichaelChirico]

Is std::ceil() guaranteed to be in uint8_t range? If so maybe maybe something like this is preferable?

Suggested change

	int newprec = std::fabs(data) >= 1 || data == 0. ? prec : std::ceil(-std::log10(std::fabs(data))) + 1;
	uint8_t newprec = std::fabs(data) >= 1 || data == 0. ? prec : std::ceil(-std::log10(std::fabs(data)));
	newprec = newprec == UINT8_MAX ? UINT8_MAX : newprec - 1;

[randy3k]

I believe std::ceil returns double for double input. For double data > 0, the smallest value is about 2.2 x 10^(-308), which gives std::ceil(-std::log10(std::fabs(data))) equals to 308.

https://g.co/gemini/share/691c650e3e82

On the other hand, std::setprecision takes int input.
I think the patch by Gemini should be sufficient in this case.

[MichaelChirico]

True, matching the std::setprecision() parameter sounds like the best choice to me

https://en.cppreference.com/w/cpp/io/manip/setprecision

[MichaelChirico]

For the record this is the test that triggers the error from {svglite}:

https://github.com/tidyverse/ggplot2/blob/961b194941380faa77addafd9d9f46b2d105ba8e/tests/testthat/test-geom-polygon.R#L28