out_dxf: EED groups 1070 and 1071 are signed — do not zero-extend them - #1392
Open
tuxiasumari wants to merge 1 commit into
Open
out_dxf: EED groups 1070 and 1071 are signed — do not zero-extend them#1392tuxiasumari wants to merge 1 commit into
tuxiasumari wants to merge 1 commit into
Conversation
`Dwg_Eed_Data` stores the 1070 short in a `BITCODE_RS` and the 1071 long in a `BITCODE_RL`, both unsigned, and the DXF writer handed those fields straight to `VALUE_RS`/`VALUE_RL`. The value was therefore zero-extended before formatting and every negative came back as its unsigned complement: `1070 -6700` re-exported as `1070 58836`, `1070 -1` as `65535`, `1071 -70000` as `4294897296`. `dxf_format()` already documents the right signedness for both codes, so the fix is to hand it a signed value; the signed macros `VALUE_RSd` / `VALUE_RLd` are what the neighbouring resbuf path already uses for the same group codes. While here, `dxf_format(1071)` returned `"%9li"`, which reads 64 bits of vararg. Every caller passes a 32-bit value (this EED field and resbuf's `i32`), so it is now `"%9i"` — the mismatch was undefined behaviour on every LP64 target and is why casting the field alone was not enough. The DWG side was never wrong: a DWG written here is byte-identical to one ODA File Converter writes from the same DXF, and reading ODA's own DWG reproduced the same corrupt DXF, so this is purely the export path. Verified against ODA File Converter 25.6 on a drawing carrying 1070/1071 values at both ends of the range (-32768, -1, 32767, -70000): after the fix the emitted DXF matches ODA's value for value. Regression on 200 real drawings, stock against patched: 2595 values change and every one of them is exactly the two's-complement reinterpretation (1014 in 1070, 1581 in 1071); no other line of any file differs, and `make check` stays at 254/254.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Dwg_Eed_Datastores the 1070 short in aBITCODE_RSand the 1071 long in aBITCODE_RL— both unsigned — and the DXF writer handed those fields straight toVALUE_RS/VALUE_RL. The value is therefore zero-extended before formatting, soevery negative comes back as its unsigned complement.
XDATA is where other applications keep their data, so this silently rewrites values
LibreDWG is only meant to carry across.
Reproducer
A DXF with one LINE carrying XDATA at both ends of the range, through
dxf2dwg --as r2000thendwg2dxf:1070 -67001070 588361070 -67001070 -11070 655351070 -11070 -327681070 327681070 -327681071 -700001071 42948972961071 -700001070 6700,1040 -12.5,1000 "negativo"The DWG side was never wrong. The DWG this writes is byte-identical in its EED to
the one ODA writes from the same DXF, and reading ODA's own DWG with
dwgread -O jsonreproduces the same
58836. The corruption is purely in the export path.Cause
src/out_dxf.c, indxf_write_eed():dxf_format()already documents the correct signedness for both codes ("%6i","%9li"), and the resbuf path a few lines below already uses the signed macrosVALUE_RSd/VALUE_RLdfor the same group codes. This makes the EED path agreewith both.
A second, latent bug found on the way
dxf_format(1071)returned"%9li", which reads 64 bits of vararg. Every callerpasses a 32-bit value (this EED field, and resbuf's
i32), so the mismatch isundefined behaviour on every LP64 target — and it is why casting the field alone was
not enough: 1070 was fixed but 1071 still printed
4294897296. It is now"%9i".Verification
exactly the two's-complement reinterpretation (1014 in 1070, 1581 in 1071).
No other line of any file differs — 146 of the 200 drawings carry at least one
corrupted value, so this affects roughly three quarters of real-world files.
make check: 254/254, unchanged.compiler warnings, and fixes the reproducer there on its own.
Found with a round-trip fuzz harness (ezdxf →
dxf2dwg→dwg2dxf→ compare) afterwidening it to carry XDATA.