Integer overflow in gerber export and 3d viewer

Bug #1661705 reported by Rene Poeschl
32
This bug affects 5 people
Affects Status Importance Assigned to Milestone
KiCad
Fix Released
Unknown

Bug Description

A user on the info forum reported a strange bug where his gerbers had strange lines in them.

Investigating the file lead to the discovery that having a text on b.cu at y position -2147.483648 leads to an integer overflow when creating the gerbers or when viewing in the 3d viewer. (-2147483648 is the smallest re-presentable 32 bit integer)

KiCad info forums post:
https://forum.kicad.info/t/extraneous-traces-on-3d-view-gerber-but-not-on-pcbnew/5155/

The attached file creates an assertion error when opening in the 3d viewer because the board size in y direction is negative. (ignoring it enables one to see what is created when exporting to gerber)

It affects at least the version 4.0.4 (version of the original poster)
And my nightly build version: (6 month old fedora build.)
Application: pcbnew
Version: no-vcs-found-product, debug build
Libraries: wxWidgets 3.0.2
           libcurl/7.43.0 NSS/3.26 zlib/1.2.8 libidn/1.33 libssh2/1.6.0 nghttp2/1.7.1
Platform: Linux 4.8.13-100.fc23.x86_64 x86_64, 64 bit, Little endian, wxGTK
- Build Info -
wxWidgets: 3.0.2 (wchar_t,wx containers,compatible with 2.8)
Boost: 1.58.0
Curl: 7.43.0
KiCad - Compiler: GCC 5.3.1 with C++ ABI 1009
        Settings: USE_WX_GRAPHICS_CONTEXT=OFF
                  USE_WX_OVERLAY=OFF
                  KICAD_SCRIPTING=ON
                  KICAD_SCRIPTING_MODULES=ON
                  KICAD_SCRIPTING_WXPYTHON=ON
                  USE_FP_LIB_TABLE=HARD_CODED_ON
                  BUILD_GITHUB_PLUGIN=ON

Tags: gerber pcbnew
Revision history for this message
Rene Poeschl (poeschlr) wrote :
Revision history for this message
Rene Poeschl (poeschlr) wrote :

I should have looked. The original poster also made a bug report here: #1661674

Revision history for this message
David Pearce (halzia) wrote :

Also replicated under 4.0.5 Stable for Windows 64 bit

Revision history for this message
David Pearce (halzia) wrote :

And after a reload in 4.0.5 no longer visible. Very odd

Revision history for this message
David Pearce (halzia) wrote :

My mistake, spurious lines are present when the the file is used to create Gerber plots under 4.0.5 Win 64 and 4.0.5 Ubuntu 64 bit stable from the PPA

Revision history for this message
Nick Østergaard (nickoe) wrote :

I have marked #1661674 as a duplicte of this since this bug contains more information.

tags: added: gerber pcbnew
Changed in kicad:
status: New → Confirmed
Revision history for this message
Rene Poeschl (poeschlr) wrote :

Using the "4.5 (unit mm)" gerber format does not create the same effect. There the half of the text that generates the overflow is not plotted.

Jeff Young (jeyjey)
Changed in kicad:
importance: Undecided → Medium
Changed in kicad:
milestone: none → 5.0.0-rc2
Revision history for this message
Jon Evans (craftyjon) wrote :

Hmm, how should we fix this for Gerber export? Warn the user that their board does not fit within the maximum dimensions of a Gerber file?

Revision history for this message
Jeff Young (jeyjey) wrote :

I'd be more inclined to error-out. If the board can't be represented in Gerber, then there's no point saving a Gerber.

(The nicer solution would be to clip, but that's a lot more work.)

Revision history for this message
Seth Hillbrand (sethh) wrote :

Why do you think it is an issue in the file? I suspect that this might just be in the viewer. Gerber _should_ handle positions as text with 4.6 format, so up to 9999.999999 mm/in.

Or is there something that prevents that in the spec? I might have missed it somewhere.

Revision history for this message
jean-pierre charras (jp-charras) wrote :

No so simple.
Gerber can handle 9999.99999 mm (internal unit = 10 nanometers)

But Pcbnew cannot handle this size.
It is limited to roughly -1m to +1m

this board file has a text located to INT_MIN on the y axis.
I do not know how it is possible, but this is outside the Pcbnew capacity.
see the line:
  (gr_text Reset (at 129.54 -2147.483648 180) (layer B.Cu)

that locate a text at y = -2m and this is not possible with Pcbnew.

Therefore the .gbr file cannot be correctly created by Pcbnew (integer overflow) and I think the board file is broken.

Revision history for this message
Wayne Stambaugh (stambaughw) wrote : Re: [Bug 1661705] Re: Integer overflow in gerber export and 3d viewer

On 3/8/2018 10:31 AM, jean-pierre charras wrote:
> No so simple.
> Gerber can handle 9999.99999 mm (internal unit = 10 nanometers)
>
> But Pcbnew cannot handle this size.
> It is limited to roughly -1m to +1m

Do you mean the drawing area of pcbnew is limited to -1m to 1m? I
thought the internal units of pcbnew was 1 nanometer. If I am not
mistaken, the internal units (coordinates) are held by 32 bit integers.
If my math is correct, the board editor should be able to support a
drawing area from -2147.483648mm to 2147.483648mm. That should fit
comfortably into the gerber file limits.

>
> this board file has a text located to INT_MIN on the y axis.
> I do not know how it is possible, but this is outside the Pcbnew capacity.
> see the line:
> (gr_text Reset (at 129.54 -2147.483648 180) (layer B.Cu)

My guess is that this was edited by hand which would cause the text it
extend past the allowable drawing area triggering an integer overflow
but -2147.483648 should be a valid coordinate as long as nothing
extended beyond this limit.

>
> that locate a text at y = -2m and this is not possible with Pcbnew.
>
> Therefore the .gbr file cannot be correctly created by Pcbnew (integer
> overflow) and I think the board file is broken.
>

Revision history for this message
jean-pierre charras (jp-charras) wrote :

1 - The coordinates are from -2147.483648mm to 2147.483648mm.
But I talked about a board, not coordinates.
This is a bit different.
For instance in some calculations, we calculate absolute values, therefore 0 to 2147.483648
and a diagonal distance limits coordinates to 0 to 2147.483648/1.414 = 1.6m

This is the reason I am thinking a board is (roughly) limited to 1m x 1m

2 - In this case, a text located at -2147.483648 mm cannot be correctly draw because segments drawn to create the text shape can have smaller y coordinates.
We need a margin around the x and y text coordinates

I also am thinking this is a board file modified by hand.

Revision history for this message
Wayne Stambaugh (stambaughw) wrote :

On 3/8/2018 11:56 AM, jean-pierre charras wrote:
> 1 - The coordinates are from -2147.483648mm to 2147.483648mm.
> But I talked about a board, not coordinates.
> This is a bit different.
> For instance in some calculations, we calculate absolute values, therefore 0 to 2147.483648
> and a diagonal distance limits coordinates to 0 to 2147.483648/1.414 = 1.6m

The question is, should we be doing this or should we be working in 64
bits and then checking for overflows to 32 bits? Either that or we
clamp the drawing area to (2^32)/4 to leave enough room for 32-bit
calculations. We should take a look at this in v6 since technically
this is a bug.

>
> This is the reason I am thinking a board is (roughly) limited to 1m x 1m
>
> 2 - In this case, a text located at -2147.483648 mm cannot be correctly draw because segments drawn to create the text shape can have smaller y coordinates.
> We need a margin around the x and y text coordinates
>
> I also am thinking this is a board file modified by hand.
>

Revision history for this message
jean-pierre charras (jp-charras) wrote :

I agree we should take a look at this in v6 about this.
However:
- Many calculations are already made in 64 bits.
- More or less the working area is already limited.
It is still too big in GAL mode, but it is limited.
But this is also a proof this offending file was modified outside Pcbnew.

Anyway, if a file is modified with stupid values, we will have problems.
Sometimes I understand why binary file formats exist.

Revision history for this message
Jon Evans (craftyjon) wrote :

So should we remove this from 5.0RC2 milestone then?

Revision history for this message
Jeff Young (jeyjey) wrote :

Sounds like it to me....

Revision history for this message
Seth Hillbrand (sethh) wrote :

I think that there's still a bug here for 5.

In pcbnew, if I place text at the edge, either by placing it or moving it to the maximum value, I get this issue.

Steps:
1) Place a text on F.Cu
2) Select and choose Move Relative
3) Input -2148, -2148 for the X/Y coordinates. Click OK
4) Save
5) plot to gerber.

pcbnew will write the file like this:

  (gr_text Test (at -2147.483648 -2147.483648) (layer F.Cu)
    (effects (font (size 1.5 1.5) (thickness 0.3)))

The gerbview output looks like the OP's image. Just a straight line, top to bottom.

Not really a realistic board but nevertheless I think the bug could be that we don't limit placing/moving items to the 1m limit.

Revision history for this message
Wayne Stambaugh (stambaughw) wrote :

On 3/15/2018 5:32 PM, Seth Hillbrand wrote:
> I think that there's still a bug here for 5.
>
> In pcbnew, if I place text at the edge, either by placing it or moving
> it to the maximum value, I get this issue.
>
> Steps:
> 1) Place a text on F.Cu
> 2) Select and choose Move Relative
> 3) Input -2148, -2148 for the X/Y coordinates. Click OK
> 4) Save
> 5) plot to gerber.
>
> pcbnew will write the file like this:
>
> (gr_text Test (at -2147.483648 -2147.483648) (layer F.Cu)
> (effects (font (size 1.5 1.5) (thickness 0.3)))
>
> The gerbview output looks like the OP's image. Just a straight line,
> top to bottom.
>
> Not really a realistic board but nevertheless I think the bug could be
> that we don't limit placing/moving items to the 1m limit.
>

@Seth, I'm fine with limiting the drawing area to half of the maximum
viewing area but that does not address existing boards that contain
objects outside this area. I know this is rare but it's happened at
least once. Otherwise this bug report wouldn't exist. I don't know
that there is an easy solution to fix this. It may be worth waiting
until we can address with a better solution in v6.

Revision history for this message
Jon Evans (craftyjon) wrote :

I'm with Wayne here, I think that since a real fix will involve some kind of warning/migration for existing boards that are too big, and because you have to go out of your way to trigger this bug, I don't think it's critical to 5.0 release.

Revision history for this message
Wayne Stambaugh (stambaughw) wrote :

I changed the release version to v6 so we can do a proper evaluation of the problem.

Changed in kicad:
milestone: 5.0.0-rc2 → 6.0.0-rc1
Revision history for this message
KiCad Janitor (kicad-janitor) wrote :

KiCad bug tracker has moved to Gitlab. This report is now available here: https://gitlab.com/kicad/code/kicad/-/issues/1766

Changed in kicad:
status: Confirmed → Expired
Changed in kicad:
importance: Medium → Unknown
status: Expired → New
Changed in kicad:
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.